From: Daniel Vetter <daniel@ffwll.ch> To: "Christian König" <ckoenig.leichtzumerken@gmail.com> Cc: daniel@ffwll.ch, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org Subject: Re: [PATCH 01/24] dma-buf: add dma_resv_replace_fences Date: Wed, 22 Dec 2021 22:05:45 +0100 [thread overview] Message-ID: <YcOTKYkEcu7MG2sY@phenom.ffwll.local> (raw) In-Reply-To: <20211207123411.167006-2-christian.koenig@amd.com> On Tue, Dec 07, 2021 at 01:33:48PM +0100, Christian König wrote: > This function allows to replace fences from the shared fence list when > we can gurantee that the operation represented by the original fence has > finished or no accesses to the resources protected by the dma_resv > object any more when the new fence finishes. > > Then use this function in the amdkfd code when BOs are unmapped from the > process. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-resv.c | 43 ++++++++++++++++ > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 49 +++---------------- > include/linux/dma-resv.h | 2 + > 3 files changed, 52 insertions(+), 42 deletions(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 4deea75c0b9c..a688dbded3d3 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -284,6 +284,49 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) > } > EXPORT_SYMBOL(dma_resv_add_shared_fence); > > +/** > + * dma_resv_replace_fences - replace fences in the dma_resv obj > + * @obj: the reservation object > + * @context: the context of the fences to replace > + * @replacement: the new fence to use instead > + * > + * Replace fences with a specified context with a new fence. Only valid if the > + * operation represented by the original fences is completed or has no longer > + * access to the resources protected by the dma_resv object when the new fence > + * completes. > + */ > +void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, > + struct dma_fence *replacement) > +{ > + struct dma_resv_list *list; > + struct dma_fence *old; > + unsigned int i; > + > + dma_resv_assert_held(obj); > + > + write_seqcount_begin(&obj->seq); > + > + old = dma_resv_excl_fence(obj); > + if (old->context == context) { > + RCU_INIT_POINTER(obj->fence_excl, dma_fence_get(replacement)); > + dma_fence_put(old); > + } > + > + list = dma_resv_shared_list(obj); > + for (i = 0; list && i < list->shared_count; ++i) { > + old = rcu_dereference_protected(list->shared[i], > + dma_resv_held(obj)); > + if (old->context != context) > + continue; > + > + rcu_assign_pointer(list->shared[i], dma_fence_get(replacement)); > + dma_fence_put(old); Since the fences are all guaranteed to be from the same context, maybe we should have a WARN_ON(__dma_fence_is_later()); here just to be safe? With that added: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + } > + > + write_seqcount_end(&obj->seq); > +} > +EXPORT_SYMBOL(dma_resv_replace_fences); > + > /** > * dma_resv_add_excl_fence - Add an exclusive fence. > * @obj: the reservation object > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > index 71acd577803e..b558ef0f8c4a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > @@ -236,53 +236,18 @@ void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo) > static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, > struct amdgpu_amdkfd_fence *ef) > { > - struct dma_resv *resv = bo->tbo.base.resv; > - struct dma_resv_list *old, *new; > - unsigned int i, j, k; > + struct dma_fence *replacement; > > if (!ef) > return -EINVAL; > > - old = dma_resv_shared_list(resv); > - if (!old) > - return 0; > - > - new = kmalloc(struct_size(new, shared, old->shared_max), GFP_KERNEL); > - if (!new) > - return -ENOMEM; > - > - /* Go through all the shared fences in the resevation object and sort > - * the interesting ones to the end of the list. > + /* TODO: Instead of block before we should use the fence of the page > + * table update and TLB flush here directly. > */ > - for (i = 0, j = old->shared_count, k = 0; i < old->shared_count; ++i) { > - struct dma_fence *f; > - > - f = rcu_dereference_protected(old->shared[i], > - dma_resv_held(resv)); > - > - if (f->context == ef->base.context) > - RCU_INIT_POINTER(new->shared[--j], f); > - else > - RCU_INIT_POINTER(new->shared[k++], f); > - } > - new->shared_max = old->shared_max; > - new->shared_count = k; > - > - /* Install the new fence list, seqcount provides the barriers */ > - write_seqcount_begin(&resv->seq); > - RCU_INIT_POINTER(resv->fence, new); > - write_seqcount_end(&resv->seq); > - > - /* Drop the references to the removed fences or move them to ef_list */ > - for (i = j; i < old->shared_count; ++i) { > - struct dma_fence *f; > - > - f = rcu_dereference_protected(new->shared[i], > - dma_resv_held(resv)); > - dma_fence_put(f); > - } > - kfree_rcu(old, rcu); > - > + replacement = dma_fence_get_stub(); > + dma_resv_replace_fences(bo->tbo.base.resv, ef->base.context, > + replacement); > + dma_fence_put(replacement); > return 0; > } > > diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h > index eebf04325b34..e0be34265eae 100644 > --- a/include/linux/dma-resv.h > +++ b/include/linux/dma-resv.h > @@ -457,6 +457,8 @@ void dma_resv_init(struct dma_resv *obj); > void dma_resv_fini(struct dma_resv *obj); > int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences); > void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence); > +void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, > + struct dma_fence *fence); > void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence); > int dma_resv_get_fences(struct dma_resv *obj, struct dma_fence **pfence_excl, > unsigned *pshared_count, struct dma_fence ***pshared); > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch> To: "Christian König" <ckoenig.leichtzumerken@gmail.com> Cc: linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org Subject: Re: [PATCH 01/24] dma-buf: add dma_resv_replace_fences Date: Wed, 22 Dec 2021 22:05:45 +0100 [thread overview] Message-ID: <YcOTKYkEcu7MG2sY@phenom.ffwll.local> (raw) In-Reply-To: <20211207123411.167006-2-christian.koenig@amd.com> On Tue, Dec 07, 2021 at 01:33:48PM +0100, Christian König wrote: > This function allows to replace fences from the shared fence list when > we can gurantee that the operation represented by the original fence has > finished or no accesses to the resources protected by the dma_resv > object any more when the new fence finishes. > > Then use this function in the amdkfd code when BOs are unmapped from the > process. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-resv.c | 43 ++++++++++++++++ > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 49 +++---------------- > include/linux/dma-resv.h | 2 + > 3 files changed, 52 insertions(+), 42 deletions(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 4deea75c0b9c..a688dbded3d3 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -284,6 +284,49 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) > } > EXPORT_SYMBOL(dma_resv_add_shared_fence); > > +/** > + * dma_resv_replace_fences - replace fences in the dma_resv obj > + * @obj: the reservation object > + * @context: the context of the fences to replace > + * @replacement: the new fence to use instead > + * > + * Replace fences with a specified context with a new fence. Only valid if the > + * operation represented by the original fences is completed or has no longer > + * access to the resources protected by the dma_resv object when the new fence > + * completes. > + */ > +void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, > + struct dma_fence *replacement) > +{ > + struct dma_resv_list *list; > + struct dma_fence *old; > + unsigned int i; > + > + dma_resv_assert_held(obj); > + > + write_seqcount_begin(&obj->seq); > + > + old = dma_resv_excl_fence(obj); > + if (old->context == context) { > + RCU_INIT_POINTER(obj->fence_excl, dma_fence_get(replacement)); > + dma_fence_put(old); > + } > + > + list = dma_resv_shared_list(obj); > + for (i = 0; list && i < list->shared_count; ++i) { > + old = rcu_dereference_protected(list->shared[i], > + dma_resv_held(obj)); > + if (old->context != context) > + continue; > + > + rcu_assign_pointer(list->shared[i], dma_fence_get(replacement)); > + dma_fence_put(old); Since the fences are all guaranteed to be from the same context, maybe we should have a WARN_ON(__dma_fence_is_later()); here just to be safe? With that added: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + } > + > + write_seqcount_end(&obj->seq); > +} > +EXPORT_SYMBOL(dma_resv_replace_fences); > + > /** > * dma_resv_add_excl_fence - Add an exclusive fence. > * @obj: the reservation object > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > index 71acd577803e..b558ef0f8c4a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > @@ -236,53 +236,18 @@ void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo) > static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, > struct amdgpu_amdkfd_fence *ef) > { > - struct dma_resv *resv = bo->tbo.base.resv; > - struct dma_resv_list *old, *new; > - unsigned int i, j, k; > + struct dma_fence *replacement; > > if (!ef) > return -EINVAL; > > - old = dma_resv_shared_list(resv); > - if (!old) > - return 0; > - > - new = kmalloc(struct_size(new, shared, old->shared_max), GFP_KERNEL); > - if (!new) > - return -ENOMEM; > - > - /* Go through all the shared fences in the resevation object and sort > - * the interesting ones to the end of the list. > + /* TODO: Instead of block before we should use the fence of the page > + * table update and TLB flush here directly. > */ > - for (i = 0, j = old->shared_count, k = 0; i < old->shared_count; ++i) { > - struct dma_fence *f; > - > - f = rcu_dereference_protected(old->shared[i], > - dma_resv_held(resv)); > - > - if (f->context == ef->base.context) > - RCU_INIT_POINTER(new->shared[--j], f); > - else > - RCU_INIT_POINTER(new->shared[k++], f); > - } > - new->shared_max = old->shared_max; > - new->shared_count = k; > - > - /* Install the new fence list, seqcount provides the barriers */ > - write_seqcount_begin(&resv->seq); > - RCU_INIT_POINTER(resv->fence, new); > - write_seqcount_end(&resv->seq); > - > - /* Drop the references to the removed fences or move them to ef_list */ > - for (i = j; i < old->shared_count; ++i) { > - struct dma_fence *f; > - > - f = rcu_dereference_protected(new->shared[i], > - dma_resv_held(resv)); > - dma_fence_put(f); > - } > - kfree_rcu(old, rcu); > - > + replacement = dma_fence_get_stub(); > + dma_resv_replace_fences(bo->tbo.base.resv, ef->base.context, > + replacement); > + dma_fence_put(replacement); > return 0; > } > > diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h > index eebf04325b34..e0be34265eae 100644 > --- a/include/linux/dma-resv.h > +++ b/include/linux/dma-resv.h > @@ -457,6 +457,8 @@ void dma_resv_init(struct dma_resv *obj); > void dma_resv_fini(struct dma_resv *obj); > int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences); > void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence); > +void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, > + struct dma_fence *fence); > void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence); > int dma_resv_get_fences(struct dma_resv *obj, struct dma_fence **pfence_excl, > unsigned *pshared_count, struct dma_fence ***pshared); > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
next prev parent reply other threads:[~2021-12-22 21:05 UTC|newest] Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-07 12:33 completely rework the dma_resv semantic Christian König 2021-12-07 12:33 ` [PATCH 01/24] dma-buf: add dma_resv_replace_fences Christian König 2021-12-22 21:05 ` Daniel Vetter [this message] 2021-12-22 21:05 ` Daniel Vetter 2022-01-03 10:48 ` Christian König 2022-01-03 10:48 ` Christian König 2022-01-14 16:28 ` Daniel Vetter 2022-01-14 16:28 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 02/24] dma-buf: finally make the dma_resv_list private Christian König 2021-12-22 21:08 ` Daniel Vetter 2021-12-22 21:08 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 03/24] dma-buf: drop excl_fence parameter from dma_resv_get_fences Christian König 2021-12-22 21:13 ` Daniel Vetter 2021-12-22 21:13 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 04/24] dma-buf: add dma_resv_get_singleton v2 Christian König 2021-12-22 21:21 ` Daniel Vetter 2021-12-22 21:21 ` Daniel Vetter 2022-01-03 11:13 ` Christian König 2022-01-03 11:13 ` Christian König 2022-01-14 16:31 ` Daniel Vetter 2022-01-14 16:31 ` Daniel Vetter 2022-01-17 11:26 ` Christian König 2022-01-17 11:26 ` Christian König 2022-03-02 17:39 ` Jason Ekstrand 2021-12-07 12:33 ` [PATCH 05/24] RDMA: use dma_resv_wait() instead of extracting the fence Christian König 2021-12-22 21:23 ` Daniel Vetter 2021-12-22 21:23 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 06/24] drm/etnaviv: stop using dma_resv_excl_fence Christian König 2021-12-22 21:26 ` Daniel Vetter 2021-12-22 21:26 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 07/24] drm/nouveau: " Christian König 2021-12-22 21:26 ` Daniel Vetter 2021-12-22 21:26 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 08/24] drm/vmwgfx: " Christian König 2021-12-22 21:31 ` Daniel Vetter 2021-12-22 21:31 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 09/24] drm/radeon: " Christian König 2021-12-22 21:30 ` Daniel Vetter 2021-12-22 21:30 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 10/24] drm/amdgpu: remove excl as shared workarounds Christian König 2021-12-22 21:34 ` Daniel Vetter 2021-12-22 21:34 ` Daniel Vetter 2021-12-07 12:33 ` [PATCH 11/24] drm/amdgpu: use dma_resv_for_each_fence for CS workaround Christian König 2021-12-22 21:37 ` Daniel Vetter 2021-12-22 21:37 ` Daniel Vetter 2022-01-03 12:24 ` Christian König 2022-01-03 12:24 ` Christian König 2021-12-07 12:33 ` [PATCH 12/24] dma-buf: finally make dma_resv_excl_fence private Christian König 2021-12-22 21:39 ` Daniel Vetter 2021-12-22 21:39 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 13/24] dma-buf: drop the DAG approach for the dma_resv object Christian König 2021-12-22 21:43 ` Daniel Vetter 2021-12-22 21:43 ` Daniel Vetter 2022-01-04 15:08 ` Christian König 2022-01-04 15:08 ` Christian König 2022-01-14 16:33 ` Daniel Vetter 2022-01-14 16:33 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 14/24] dma-buf/drivers: make reserving a shared slot mandatory v2 Christian König 2021-12-22 21:50 ` Daniel Vetter 2021-12-22 21:50 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 15/24] drm: support more than one write fence in drm_gem_plane_helper_prepare_fb Christian König 2021-12-22 21:51 ` Daniel Vetter 2021-12-22 21:51 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 16/24] drm/nouveau: support more than one write fence in fenv50_wndw_prepare_fb Christian König 2021-12-22 21:52 ` Daniel Vetter 2021-12-22 21:52 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 17/24] drm/amdgpu: use dma_resv_get_singleton in amdgpu_pasid_free_cb Christian König 2021-12-22 21:53 ` Daniel Vetter 2021-12-22 21:53 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 18/24] dma-buf: add enum dma_resv_usage v3 Christian König 2021-12-22 22:00 ` Daniel Vetter 2021-12-22 22:00 ` Daniel Vetter 2022-03-02 17:55 ` Jason Ekstrand 2022-03-03 13:13 ` Christian König 2021-12-07 12:34 ` [PATCH 19/24] dma-buf: specify usage while adding fences to dma_resv obj v2 Christian König 2021-12-07 12:34 ` [PATCH 20/24] dma-buf: add DMA_RESV_USAGE_KERNEL Christian König 2021-12-22 22:05 ` Daniel Vetter 2021-12-22 22:05 ` Daniel Vetter 2022-03-02 18:11 ` Jason Ekstrand 2022-03-03 13:49 ` Christian König 2021-12-07 12:34 ` [PATCH 21/24] dma-buf: add DMA_RESV_USAGE_BOOKKEEP Christian König 2021-12-22 22:10 ` Daniel Vetter 2021-12-22 22:10 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 22/24] dma-buf: wait for map to complete for static attachments Christian König 2021-12-22 22:16 ` Daniel Vetter 2021-12-22 22:16 ` Daniel Vetter 2021-12-07 12:34 ` [PATCH 23/24] amdgpu: remove DMA-buf fence workaround Christian König 2021-12-07 12:34 ` [PATCH 24/24] drm/ttm: remove bo->moving Christian König 2021-12-17 14:39 ` completely rework the dma_resv semantic Christian König 2021-12-22 22:17 ` Daniel Vetter 2021-12-22 22:17 ` Daniel Vetter 2021-12-23 9:11 ` Christian König 2021-12-23 9:11 ` Christian König 2022-01-14 16:35 ` Daniel Vetter 2022-01-14 16:35 ` Daniel Vetter
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=YcOTKYkEcu7MG2sY@phenom.ffwll.local \ --to=daniel@ffwll.ch \ --cc=ckoenig.leichtzumerken@gmail.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=linaro-mm-sig@lists.linaro.org \ --cc=linux-media@vger.kernel.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.