Re: [PATCH 35/59] drm/ttm: make TTM responsible for cleaning system only.

From: Ben Skeggs <skeggsb@gmail.com>
To: Dave Airlie <airlied@gmail.com>
Cc: sroland@vmware.com,
	"ML dri-devel" <dri-devel@lists.freedesktop.org>,
	linux-graphics-maintainer@vmware.com,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Ben Skeggs" <bskeggs@redhat.com>
Subject: Re: [PATCH 35/59] drm/ttm: make TTM responsible for cleaning system only.
Date: Wed, 5 Aug 2020 15:46:04 +1000	[thread overview]
Message-ID: <CACAvsv5n1Ck53SUw40EeFFYu5fQ0y5mDFOPxVq9uZnvEFs0cxw@mail.gmail.com> (raw)
In-Reply-To: <20200804025632.3868079-36-airlied@gmail.com>

On Tue, 4 Aug 2020 at 12:58, Dave Airlie <airlied@gmail.com> wrote:
>
> From: Dave Airlie <airlied@redhat.com>
>
> Drivers should all be cleaning up their memory managers
> themselves now, so let the core just clean the system one up.
>
> Remove the legacy cleaning interface.
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Ben Skeggs <bskeggs@redhat.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c    | 54 +++------------------------------
>  include/drm/ttm/ttm_bo_api.h    | 28 -----------------
>  include/drm/ttm/ttm_bo_driver.h | 10 ------
>  3 files changed, 4 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index a45038c74de6..ebecb796dd49 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1452,42 +1452,6 @@ int ttm_mem_type_manager_force_list_clean(struct ttm_bo_device *bdev,
>  }
>  EXPORT_SYMBOL(ttm_mem_type_manager_force_list_clean);
>
> -int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)
> -{
> -       struct ttm_mem_type_manager *man;
> -       int ret = -EINVAL;
> -
> -       if (mem_type >= TTM_NUM_MEM_TYPES) {
> -               pr_err("Illegal memory type %d\n", mem_type);
> -               return ret;
> -       }
> -       man = &bdev->man[mem_type];
> -
> -       if (!man->has_type) {
> -               pr_err("Trying to take down uninitialized memory manager type %u\n",
> -                      mem_type);
> -               return ret;
> -       }
> -
> -       ttm_mem_type_manager_disable(man);
> -
> -       ret = 0;
> -       if (mem_type > 0) {
> -               ret = ttm_mem_type_manager_force_list_clean(bdev, man);
> -               if (ret) {
> -                       pr_err("Cleanup eviction failed\n");
> -                       return ret;
> -               }
> -
> -               if (man->func->takedown)
> -                       ret = (*man->func->takedown)(man);
> -       }
> -
> -       ttm_mem_type_manager_cleanup(man);
> -
> -       return ret;
> -}
> -EXPORT_SYMBOL(ttm_bo_clean_mm);
>
>  int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type)
>  {
> @@ -1591,21 +1555,11 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
>  {
>         struct ttm_bo_global *glob = &ttm_bo_glob;
>         int ret = 0;
> -       unsigned i = TTM_NUM_MEM_TYPES;
> +       unsigned i;
>         struct ttm_mem_type_manager *man;
>
> -       while (i--) {
> -               man = &bdev->man[i];
> -               if (man->has_type) {
> -                       man->use_type = false;
> -                       if ((i != TTM_PL_SYSTEM) && ttm_bo_clean_mm(bdev, i)) {
> -                               ret = -EBUSY;
> -                               pr_err("DRM memory manager type %d is not clean\n",
> -                                      i);
> -                       }
> -                       man->has_type = false;
> -               }
> -       }
> +       man = &bdev->man[TTM_PL_SYSTEM];
> +       ttm_mem_type_manager_disable(man);
>
>         mutex_lock(&ttm_global_mutex);
>         list_del(&bdev->device_list);
> @@ -1618,7 +1572,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
>
>         spin_lock(&glob->lru_lock);
>         for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> -               if (list_empty(&bdev->man[0].lru[0]))
> +               if (list_empty(&man->lru[0]))
>                         pr_debug("Swap list %d was clean\n", i);
>         spin_unlock(&glob->lru_lock);
>
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 2c84622faa44..9c55eafd0e7d 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -546,34 +546,6 @@ void ttm_mem_type_manager_init(struct ttm_bo_device *bdev,
>                                struct ttm_mem_type_manager *man,
>                                unsigned long p_size);
>
> -/**
> - * ttm_bo_clean_mm
> - *
> - * @bdev: Pointer to a ttm_bo_device struct.
> - * @mem_type: The memory type.
> - *
> - * Take down a manager for a given memory type after first walking
> - * the LRU list to evict any buffers left alive.
> - *
> - * Normally, this function is part of lastclose() or unload(), and at that
> - * point there shouldn't be any buffers left created by user-space, since
> - * there should've been removed by the file descriptor release() method.
> - * However, before this function is run, make sure to signal all sync objects,
> - * and verify that the delayed delete queue is empty. The driver must also
> - * make sure that there are no NO_EVICT buffers present in this memory type
> - * when the call is made.
> - *
> - * If this function is part of a VT switch, the caller must make sure that
> - * there are no appications currently validating buffers before this
> - * function is called. The caller can do that by first taking the
> - * struct ttm_bo_device::ttm_lock in write mode.
> - *
> - * Returns:
> - * -EINVAL: invalid or uninitialized memory type.
> - * -EBUSY: There are still buffers left in this memory type.
> - */
> -int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type);
> -
>  /**
>   * ttm_bo_evict_mm
>   *
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 9b4c22abc22c..8cc39cd55a14 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -48,16 +48,6 @@
>  struct ttm_mem_type_manager;
>
>  struct ttm_mem_type_manager_func {
> -       /**
> -        * struct ttm_mem_type_manager member takedown
> -        *
> -        * @man: Pointer to a memory type manager.
> -        *
> -        * Called to undo the setup done in init. All allocated resources
> -        * should be freed.
> -        */
> -       int  (*takedown)(struct ttm_mem_type_manager *man);
> -
>         /**
>          * struct ttm_mem_type_manager member get_node
>          *
> --
> 2.26.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel