[01/11] drm/ttm: Make LRU removal optional.

Submitted by Christian König on May 14, 2019, 12:31 p.m.

Details

Message ID 20190514123127.1650-1-christian.koenig@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 7 6 5 4 3 2 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Christian König May 14, 2019, 12:31 p.m.
We are already doing this for DMA-buf imports and also for
amdgpu VM BOs for quite a while now.

If this doesn't run into any problems we are probably going
to stop removing BOs from the LRU altogether.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  9 +++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c       |  4 ++--
 drivers/gpu/drm/qxl/qxl_release.c             |  2 +-
 drivers/gpu/drm/radeon/radeon_gem.c           |  2 +-
 drivers/gpu/drm/radeon/radeon_object.c        |  2 +-
 drivers/gpu/drm/ttm/ttm_execbuf_util.c        | 20 +++++++++++--------
 drivers/gpu/drm/virtio/virtgpu_ioctl.c        |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c      |  3 ++-
 drivers/gpu/drm/vmwgfx/vmwgfx_validation.h    |  2 +-
 include/drm/ttm/ttm_bo_driver.h               |  5 ++++-
 include/drm/ttm/ttm_execbuf_util.h            |  3 ++-
 13 files changed, 34 insertions(+), 24 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e1cae4a37113..647e18f9e136 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -574,7 +574,7 @@  static int reserve_bo_and_vm(struct kgd_mem *mem,
 	amdgpu_vm_get_pd_bo(vm, &ctx->list, &ctx->vm_pd[0]);
 
 	ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-				     false, &ctx->duplicates);
+				     false, &ctx->duplicates, true);
 	if (!ret)
 		ctx->reserved = true;
 	else {
@@ -647,7 +647,7 @@  static int reserve_bo_and_cond_vms(struct kgd_mem *mem,
 	}
 
 	ret = ttm_eu_reserve_buffers(&ctx->ticket, &ctx->list,
-				     false, &ctx->duplicates);
+				     false, &ctx->duplicates, true);
 	if (!ret)
 		ctx->reserved = true;
 	else
@@ -1800,7 +1800,8 @@  static int validate_invalid_user_pages(struct amdkfd_process_info *process_info)
 	}
 
 	/* Reserve all BOs and page tables for validation */
-	ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates);
+	ret = ttm_eu_reserve_buffers(&ticket, &resv_list, false, &duplicates,
+				     true);
 	WARN(!list_empty(&duplicates), "Duplicates should be empty");
 	if (ret)
 		goto out_free;
@@ -2006,7 +2007,7 @@  int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef)
 	}
 
 	ret = ttm_eu_reserve_buffers(&ctx.ticket, &ctx.list,
-				     false, &duplicate_save);
+				     false, &duplicate_save, true);
 	if (ret) {
 		pr_debug("Memory eviction: TTM Reserve Failed. Try again\n");
 		goto ttm_reserve_fail;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d72cc583ebd1..fff558cf385b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@  static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	}
 
 	r = ttm_eu_reserve_buffers(&p->ticket, &p->validated, true,
-				   &duplicates);
+				   &duplicates, true);
 	if (unlikely(r != 0)) {
 		if (r != -ERESTARTSYS)
 			DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
index 54dd02a898b9..06f83cac0d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
@@ -79,7 +79,7 @@  int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	list_add(&csa_tv.head, &list);
 	amdgpu_vm_get_pd_bo(vm, &list, &pd);
 
-	r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
+	r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true);
 	if (r) {
 		DRM_ERROR("failed to reserve CSA,PD BOs: err=%d\n", r);
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 7b840367004c..d513a5ad03dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -171,7 +171,7 @@  void amdgpu_gem_object_close(struct drm_gem_object *obj,
 
 	amdgpu_vm_get_pd_bo(vm, &list, &vm_pd);
 
-	r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
+	r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates, true);
 	if (r) {
 		dev_err(adev->dev, "leaking bo va because "
 			"we fail to reserve bo (%d)\n", r);
@@ -608,7 +608,7 @@  int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 
 	amdgpu_vm_get_pd_bo(&fpriv->vm, &list, &vm_pd);
 
-	r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates);
+	r = ttm_eu_reserve_buffers(&ticket, &list, true, &duplicates, true);
 	if (r)
 		goto error_unref;
 
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 30f85f0130cb..49f9a9385393 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -256,7 +256,7 @@  int qxl_release_reserve_list(struct qxl_release *release, bool no_intr)
 		return 0;
 
 	ret = ttm_eu_reserve_buffers(&release->ticket, &release->bos,
-				     !no_intr, NULL);
+				     !no_intr, NULL, true);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 44617dec8183..7411e69e2712 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -559,7 +559,7 @@  static void radeon_gem_va_update_vm(struct radeon_device *rdev,
 	if (!vm_bos)
 		return;
 
-	r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL);
+	r = ttm_eu_reserve_buffers(&ticket, &list, true, NULL, true);
 	if (r)
 		goto error_free;
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 833e909706a9..36683de0300b 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -539,7 +539,7 @@  int radeon_bo_list_validate(struct radeon_device *rdev,
 	u64 bytes_moved_threshold = radeon_bo_get_threshold_for_moves(rdev);
 
 	INIT_LIST_HEAD(&duplicates);
-	r = ttm_eu_reserve_buffers(ticket, head, true, &duplicates);
+	r = ttm_eu_reserve_buffers(ticket, head, true, &duplicates, true);
 	if (unlikely(r != 0)) {
 		return r;
 	}
diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 0075eb9a0b52..957ec375a4ba 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -69,7 +69,8 @@  void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket,
 	list_for_each_entry(entry, list, head) {
 		struct ttm_buffer_object *bo = entry->bo;
 
-		ttm_bo_add_to_lru(bo);
+		if (list_empty(&bo->lru))
+			ttm_bo_add_to_lru(bo);
 		reservation_object_unlock(bo->resv);
 	}
 	spin_unlock(&glob->lru_lock);
@@ -93,7 +94,7 @@  EXPORT_SYMBOL(ttm_eu_backoff_reservation);
 
 int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 			   struct list_head *list, bool intr,
-			   struct list_head *dups)
+			   struct list_head *dups, bool del_lru)
 {
 	struct ttm_bo_global *glob;
 	struct ttm_validate_buffer *entry;
@@ -172,11 +173,11 @@  int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 		list_add(&entry->head, list);
 	}
 
-	if (ticket)
-		ww_acquire_done(ticket);
-	spin_lock(&glob->lru_lock);
-	ttm_eu_del_from_lru_locked(list);
-	spin_unlock(&glob->lru_lock);
+	if (del_lru) {
+		spin_lock(&glob->lru_lock);
+		ttm_eu_del_from_lru_locked(list);
+		spin_unlock(&glob->lru_lock);
+	}
 	return 0;
 }
 EXPORT_SYMBOL(ttm_eu_reserve_buffers);
@@ -203,7 +204,10 @@  void ttm_eu_fence_buffer_objects(struct ww_acquire_ctx *ticket,
 			reservation_object_add_shared_fence(bo->resv, fence);
 		else
 			reservation_object_add_excl_fence(bo->resv, fence);
-		ttm_bo_add_to_lru(bo);
+		if (list_empty(&bo->lru))
+			ttm_bo_add_to_lru(bo);
+		else
+			ttm_bo_move_to_lru_tail(bo, NULL);
 		reservation_object_unlock(bo->resv);
 	}
 	spin_unlock(&glob->lru_lock);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 161b80fee492..5cffaa24259f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -63,7 +63,7 @@  static int virtio_gpu_object_list_validate(struct ww_acquire_ctx *ticket,
 	struct virtio_gpu_object *qobj;
 	int ret;
 
-	ret = ttm_eu_reserve_buffers(ticket, head, true, NULL);
+	ret = ttm_eu_reserve_buffers(ticket, head, true, NULL, true);
 	if (ret != 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index a7c30e567f09..d28cbedba0b5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -465,7 +465,8 @@  vmw_resource_check_buffer(struct ww_acquire_ctx *ticket,
 	val_buf->bo = &res->backup->base;
 	val_buf->num_shared = 0;
 	list_add_tail(&val_buf->head, &val_list);
-	ret = ttm_eu_reserve_buffers(ticket, &val_list, interruptible, NULL);
+	ret = ttm_eu_reserve_buffers(ticket, &val_list, interruptible, NULL,
+				     true);
 	if (unlikely(ret != 0))
 		goto out_no_reserve;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
index 3b396fea40d7..ac435b51f4eb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
@@ -165,7 +165,7 @@  vmw_validation_bo_reserve(struct vmw_validation_context *ctx,
 			  bool intr)
 {
 	return ttm_eu_reserve_buffers(&ctx->ticket, &ctx->bo_list, intr,
-				      NULL);
+				      NULL, true);
 }
 
 /**
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index c008346c2401..fc0d995ac90d 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -769,7 +769,10 @@  static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
 {
 	if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
 		spin_lock(&bo->bdev->glob->lru_lock);
-		ttm_bo_add_to_lru(bo);
+		if (list_empty(&bo->lru))
+			ttm_bo_add_to_lru(bo);
+		else
+			ttm_bo_move_to_lru_tail(bo, NULL);
 		spin_unlock(&bo->bdev->glob->lru_lock);
 	}
 	reservation_object_unlock(bo->resv);
diff --git a/include/drm/ttm/ttm_execbuf_util.h b/include/drm/ttm/ttm_execbuf_util.h
index 621615fa7728..7e46cc678e7e 100644
--- a/include/drm/ttm/ttm_execbuf_util.h
+++ b/include/drm/ttm/ttm_execbuf_util.h
@@ -70,6 +70,7 @@  extern void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket,
  * @list:    thread private list of ttm_validate_buffer structs.
  * @intr:    should the wait be interruptible
  * @dups:    [out] optional list of duplicates.
+ * @del_lru: true if BOs should be removed from the LRU.
  *
  * Tries to reserve bos pointed to by the list entries for validation.
  * If the function returns 0, all buffers are marked as "unfenced",
@@ -98,7 +99,7 @@  extern void ttm_eu_backoff_reservation(struct ww_acquire_ctx *ticket,
 
 extern int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
 				  struct list_head *list, bool intr,
-				  struct list_head *dups);
+				  struct list_head *dups, bool del_lru);
 
 /**
  * function ttm_eu_fence_buffer_objects.

Comments

> -----Original Message-----

> From: Christian König <ckoenig.leichtzumerken@gmail.com>

> Sent: Tuesday, May 14, 2019 8:31 PM

> To: Olsak, Marek <Marek.Olsak@amd.com>; Zhou, David(ChunMing)

> <David1.Zhou@amd.com>; Liang, Prike <Prike.Liang@amd.com>; dri-

> devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org

> Subject: [PATCH 01/11] drm/ttm: Make LRU removal optional.

> 

> [CAUTION: External Email]

> 

> We are already doing this for DMA-buf imports and also for amdgpu VM BOs

> for quite a while now.

> 

> If this doesn't run into any problems we are probably going to stop removing

> BOs from the LRU altogether.

> 

> Signed-off-by: Christian König <christian.koenig@amd.com>

> ---

[snip]
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c

> b/drivers/gpu/drm/ttm/ttm_execbuf_util.c

> index 0075eb9a0b52..957ec375a4ba 100644

> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c

> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c

> @@ -69,7 +69,8 @@ void ttm_eu_backoff_reservation(struct

> ww_acquire_ctx *ticket,

>         list_for_each_entry(entry, list, head) {

>                 struct ttm_buffer_object *bo = entry->bo;

> 

> -               ttm_bo_add_to_lru(bo);

> +               if (list_empty(&bo->lru))

> +                       ttm_bo_add_to_lru(bo);

>                 reservation_object_unlock(bo->resv);

>         }

>         spin_unlock(&glob->lru_lock);

> @@ -93,7 +94,7 @@ EXPORT_SYMBOL(ttm_eu_backoff_reservation);

> 

>  int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,

>                            struct list_head *list, bool intr,

> -                          struct list_head *dups)

> +                          struct list_head *dups, bool del_lru)

>  {

>         struct ttm_bo_global *glob;

>         struct ttm_validate_buffer *entry; @@ -172,11 +173,11 @@ int

> ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,

>                 list_add(&entry->head, list);

>         }

> 

> -       if (ticket)

> -               ww_acquire_done(ticket);

> -       spin_lock(&glob->lru_lock);

> -       ttm_eu_del_from_lru_locked(list);

> -       spin_unlock(&glob->lru_lock);

> +       if (del_lru) {

> +               spin_lock(&glob->lru_lock);

> +               ttm_eu_del_from_lru_locked(list);

> +               spin_unlock(&glob->lru_lock);

> +       }


Can you make bo to lru tail here when del_lru is false?

Busy iteration in evict_first will try other process Bos first, which could save loop time.

>         return 0;

>  }

>  EXPORT_SYMBOL(ttm_eu_reserve_buffers);

> @@ -203,7 +204,10 @@ void ttm_eu_fence_buffer_objects(struct

> ww_acquire_ctx *ticket,

>                         reservation_object_add_shared_fence(bo->resv, fence);

>                 else

>                         reservation_object_add_excl_fence(bo->resv, fence);

> -               ttm_bo_add_to_lru(bo);

> +               if (list_empty(&bo->lru))

> +                       ttm_bo_add_to_lru(bo);

> +               else

> +                       ttm_bo_move_to_lru_tail(bo, NULL);


If this line is done in above, then we don't need this here.

-David
>                 reservation_object_unlock(bo->resv);

>         }

>         spin_unlock(&glob->lru_lock);

> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c

> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c

> index 161b80fee492..5cffaa24259f 100644

> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c

> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c

> @@ -63,7 +63,7 @@ static int virtio_gpu_object_list_validate(struct

> ww_acquire_ctx *ticket,

>         struct virtio_gpu_object *qobj;

>         int ret;

> 

> -       ret = ttm_eu_reserve_buffers(ticket, head, true, NULL);

> +       ret = ttm_eu_reserve_buffers(ticket, head, true, NULL, true);

>         if (ret != 0)

>                 return ret;

> 

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c

> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c

> index a7c30e567f09..d28cbedba0b5 100644

> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c

> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c

> @@ -465,7 +465,8 @@ vmw_resource_check_buffer(struct ww_acquire_ctx

> *ticket,

>         val_buf->bo = &res->backup->base;

>         val_buf->num_shared = 0;

>         list_add_tail(&val_buf->head, &val_list);

> -       ret = ttm_eu_reserve_buffers(ticket, &val_list, interruptible, NULL);

> +       ret = ttm_eu_reserve_buffers(ticket, &val_list, interruptible, NULL,

> +                                    true);

>         if (unlikely(ret != 0))

>                 goto out_no_reserve;

> 

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h

> b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h

> index 3b396fea40d7..ac435b51f4eb 100644

> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h

> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h

> @@ -165,7 +165,7 @@ vmw_validation_bo_reserve(struct

> vmw_validation_context *ctx,

>                           bool intr)

>  {

>         return ttm_eu_reserve_buffers(&ctx->ticket, &ctx->bo_list, intr,

> -                                     NULL);

> +                                     NULL, true);

>  }

> 

>  /**

> diff --git a/include/drm/ttm/ttm_bo_driver.h

> b/include/drm/ttm/ttm_bo_driver.h index c008346c2401..fc0d995ac90d

> 100644

> --- a/include/drm/ttm/ttm_bo_driver.h

> +++ b/include/drm/ttm/ttm_bo_driver.h

> @@ -769,7 +769,10 @@ static inline void ttm_bo_unreserve(struct

> ttm_buffer_object *bo)  {

>         if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {

>                 spin_lock(&bo->bdev->glob->lru_lock);

> -               ttm_bo_add_to_lru(bo);

> +               if (list_empty(&bo->lru))

> +                       ttm_bo_add_to_lru(bo);

> +               else

> +                       ttm_bo_move_to_lru_tail(bo, NULL);

>                 spin_unlock(&bo->bdev->glob->lru_lock);

>         }

>         reservation_object_unlock(bo->resv);

> diff --git a/include/drm/ttm/ttm_execbuf_util.h

> b/include/drm/ttm/ttm_execbuf_util.h

> index 621615fa7728..7e46cc678e7e 100644

> --- a/include/drm/ttm/ttm_execbuf_util.h

> +++ b/include/drm/ttm/ttm_execbuf_util.h

> @@ -70,6 +70,7 @@ extern void ttm_eu_backoff_reservation(struct

> ww_acquire_ctx *ticket,

>   * @list:    thread private list of ttm_validate_buffer structs.

>   * @intr:    should the wait be interruptible

>   * @dups:    [out] optional list of duplicates.

> + * @del_lru: true if BOs should be removed from the LRU.

>   *

>   * Tries to reserve bos pointed to by the list entries for validation.

>   * If the function returns 0, all buffers are marked as "unfenced", @@ -98,7

> +99,7 @@ extern void ttm_eu_backoff_reservation(struct ww_acquire_ctx

> *ticket,

> 

>  extern int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,

>                                   struct list_head *list, bool intr,

> -                                 struct list_head *dups);

> +                                 struct list_head *dups, bool del_lru);

> 

>  /**

>   * function ttm_eu_fence_buffer_objects.

> --

> 2.17.1