[PATCH 2/9] drm/ttm: move the LRU into resource handling v3

[Matthew Auld]

On Wed, 9 Feb 2022 at 08:41, Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> This way we finally fix the problem that new resource are
> not immediately evict-able after allocation.
>
> That has caused numerous problems including OOM on GDS handling
> and not being able to use TTM as general resource manager.
>
> v2: stop assuming in ttm_resource_fini that res->bo is still valid.
> v3: cleanup kerneldoc, add more lockdep annotation
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> Tested-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |   8 +-
>  drivers/gpu/drm/i915/gem/i915_gem_ttm.c |   2 +-
>  drivers/gpu/drm/ttm/ttm_bo.c            | 115 ++--------------------
>  drivers/gpu/drm/ttm/ttm_bo_util.c       |   1 -
>  drivers/gpu/drm/ttm/ttm_device.c        |  64 ++++++-------
>  drivers/gpu/drm/ttm/ttm_resource.c      | 122 +++++++++++++++++++++++-
>  include/drm/ttm/ttm_bo_api.h            |  16 ----
>  include/drm/ttm/ttm_bo_driver.h         |  29 +-----
>  include/drm/ttm/ttm_resource.h          |  35 +++++++
>  9 files changed, 197 insertions(+), 195 deletions(-)

<snip>

>  /**
>   * ttm_resource_init - resource object constructure
>   * @bo: buffer object this resources is allocated for
> @@ -52,10 +156,12 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
>         res->bus.is_iomem = false;
>         res->bus.caching = ttm_cached;
>         res->bo = bo;
> +       INIT_LIST_HEAD(&res->lru);
>
>         man = ttm_manager_type(bo->bdev, place->mem_type);
>         spin_lock(&bo->bdev->lru_lock);
>         man->usage += bo->base.size;
> +       ttm_resource_move_to_lru_tail(res, NULL);
>         spin_unlock(&bo->bdev->lru_lock);
>  }
>  EXPORT_SYMBOL(ttm_resource_init);
> @@ -66,15 +172,21 @@ EXPORT_SYMBOL(ttm_resource_init);
>   * @res: the resource to clean up
>   *
>   * Should be used by resource manager backends to clean up the TTM resource
> - * objects before freeing the underlying structure. Counterpart of
> - * &ttm_resource_init
> + * objects before freeing the underlying structure. Makes sure the resource is
> + * removed from the LRU before destruction.
> + * Counterpart of &ttm_resource_init.
>   */
>  void ttm_resource_fini(struct ttm_resource_manager *man,
>                        struct ttm_resource *res)
>  {
> -       spin_lock(&man->bdev->lru_lock);
> -       man->usage -= res->bo->base.size;
> -       spin_unlock(&man->bdev->lru_lock);
> +       struct ttm_device *bdev = man->bdev;
> +
> +       spin_lock(&bdev->lru_lock);
> +       list_del_init(&res->lru);
> +       if (res->bo && bdev->funcs->del_from_lru_notify)
> +               bdev->funcs->del_from_lru_notify(res->bo);
> +       man->usage -= res->num_pages << PAGE_SHIFT;

Above we are using the bo->base.size for tracking usage, but here we
are using num_pages. Is it guaranteed that bo->base.size is always
page aligned? Do we need some kind of WARN_ON()? Perhaps also sanity
checking that usage == 0 when tearing down the man?