From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: Marek.Olsak@amd.com, David1.Zhou@amd.com, Prike.Liang@amd.com,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/4] drm/ttm: Make LRU removal optional.
Date: Fri, 10 May 2019 16:22:14 +0200 [thread overview]
Message-ID: <7a9ee0fe-5cc0-bd32-ff1e-7e7f3705976b@gmail.com> (raw)
In-Reply-To: <20190510141316.1746-1-christian.koenig@amd.com>
Stupid me, patch #3 has a rebase error so please ignore this version.
@Marek: This set is fixing the GDS problem for my constructed test case,
you probably try it also in a real world scenario when it lands.
Christian.
Am 10.05.19 um 16:13 schrieb Christian König:
> 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(-)
>
> 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 d0e221c8d940..a1d6a0721e53 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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
prev parent reply other threads:[~2019-05-10 14:22 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-10 14:13 [PATCH 1/4] drm/ttm: Make LRU removal optional Christian König
2019-05-10 14:13 ` [PATCH 2/4] drm/ttm: fix busy memory to fail other user v7 Christian König
2019-05-10 14:13 ` [PATCH 3/4] drm/amd/display: use ttm_eu_reserve_buffers instead of amdgpu_bo_reserve Christian König
2019-05-10 14:13 ` [PATCH 4/4] drm/amdgpu: stop removing BOs from the LRU during CS Christian König
2019-05-11 1:08 ` Marek Olšák
2019-05-13 14:01 ` Liang, Prike
2019-05-13 14:17 ` Christian König
2019-05-10 14:22 ` Christian König [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7a9ee0fe-5cc0-bd32-ff1e-7e7f3705976b@gmail.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=David1.Zhou@amd.com \
--cc=Marek.Olsak@amd.com \
--cc=Prike.Liang@amd.com \
--cc=dri-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox