| Message ID | 20190514123127.1650-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] drm/ttm: Make LRU removal optional. | expand |
> -----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
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.
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(-)