From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>
Cc: dri-devel@lists.freedesktop.org, chris@chris-wilson.co.uk,
daniel.vetter@ffwll.ch, sumit.semwal@linaro.org,
linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org
Subject: Re: [PATCH 07/10] dma-buf/resv: add new fences container implementation
Date: Wed, 21 Aug 2019 18:04:54 +0200 [thread overview]
Message-ID: <20190821160454.GN11147@phenom.ffwll.local> (raw)
In-Reply-To: <20190821123147.110736-8-christian.koenig@amd.com>
On Wed, Aug 21, 2019 at 02:31:44PM +0200, Christian König wrote:
> Add a new container for fences which internally uses
> dma_fence_array's to store the fences.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/dma-buf/dma-resv.c | 221 +++++++++++++++++++++++++++++++++++++
> include/linux/dma-resv.h | 49 ++++++++
> 2 files changed, 270 insertions(+)
>
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index d3a9a3bb15f0..83033b3e8521 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -33,6 +33,7 @@
> */
>
> #include <linux/dma-resv.h>
> +#include <linux/dma-fence-array.h>
> #include <linux/export.h>
>
> /**
> @@ -55,6 +56,226 @@ EXPORT_SYMBOL(reservation_seqcount_class);
> const char reservation_seqcount_string[] = "reservation_seqcount";
> EXPORT_SYMBOL(reservation_seqcount_string);
>
> +static void dma_resv_fences_init(struct dma_resv_fences *fences)
> +{
> + RCU_INIT_POINTER(fences->fence, NULL);
> + fences->staged = NULL;
> +}
> +
> +static void dma_resv_fences_fini(struct dma_resv_fences *fences)
> +{
> + /*
> + * This object should be dead and all references must have
> + * been released to it, so no need to be protected with rcu.
> + */
> + dma_fence_put(rcu_dereference_protected(fences->fence, true));
> + dma_fence_array_free(fences->staged);
> +}
> +
> +/**
> + * dma_resv_fences_reserve - allocate fence slots
> + * @fences: fences object where we need slots
> + * @num_fences: number of fence slots we need
> + *
> + * Make sure that we have at least @num_fences + all the existing ones free
> + * slots in the staged dma_fence_array.
> + *
> + * Returns -ENOMEM on allocation failure, 0 otherwise.
> + */
> +int dma_resv_fences_reserve(struct dma_resv *obj,
> + struct dma_resv_fences *fences,
> + unsigned int num_fences)
> +{
> + struct dma_fence *fence = dma_resv_fences_deref(obj, fences);
> + struct dma_fence_array *staged, *array;
> + unsigned int i;
> +
> + array = fences->staged;
> + if (!array)
> + array = to_dma_fence_array(fence);
> +
> + if (array)
> + num_fences += array->num_fences;
> + else if (fence)
> + num_fences += 1;
> +
> + staged = fences->staged;
> + if (staged && dma_fence_array_max_fences(staged) >= num_fences)
> + return 0;
> +
> + staged = dma_fence_array_alloc(num_fences, NULL);
> + if (!staged)
> + return -ENOMEM;
> +
> + /* Copy over all fences from the old object */
> + if (array) {
> + for (i = 0; i < array->num_fences; ++i) {
> + struct dma_fence *f = array->fences[i];
> +
> + staged->fences[i] = dma_fence_get(f);
> + }
> + staged->num_fences = array->num_fences;
> +
> + } else if (fence) {
> + staged->fences[0] = dma_fence_get(fence);
> + staged->num_fences = 1;
> +
> + } else {
> + staged->num_fences = 0;
> + }
> +
> + dma_fence_array_free(fences->staged);
> + fences->staged = staged;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dma_resv_fences_reserve);
> +
> +/**
> + * dma_resv_fences_assign - set the singleton fence
> + * @fences: fences object where to set the fence
> + * @fence: singleton fence for the object
> + *
> + * Internal helper to assign the signleton fence without grapping a reference.
> + * If the old fence is a dma_fence_array try to recycle it.
> + */
> +static void dma_resv_fences_assign(struct dma_resv *obj,
> + struct dma_resv_fences *fences,
> + struct dma_fence *fence)
> +{
> + struct dma_fence_array *array, *staged;
> + unsigned int num_fences, i;
> + struct dma_fence *old;
> +
> + old = dma_resv_fences_deref(obj, fences);
> + rcu_assign_pointer(fences->fence, fence);
> +
> + dma_fence_array_free(fences->staged);
> + fences->staged = NULL;
> +
> + /* Try to recycle the old fence array */
> + staged = to_dma_fence_array(old);
> + if (!staged)
> + goto drop_old;
> +
> + array = to_dma_fence_array(fence);
> + if (array)
> + num_fences = array->num_fences;
> + else
> + num_fences = fence ? 1 : 0;
> +
> + if (dma_fence_array_max_fences(staged) < num_fences)
> + goto drop_old;
> +
> + /* Try to drop the last reference */
> + if (!dma_fence_array_recycle(staged))
Without an rcu barrier here you're not syncing to new clients at all.
I don't think this works, and I expect that once you've readded all the
barriers and retry loops we're back to seqlocks.
-Daniel
> + return;
> +
> + /* Make sure the staged array has the latest fences */
> + if (array) {
> + for (i = 0; i < array->num_fences; ++i) {
> + struct dma_fence *f = array->fences[i];
> +
> + if (f == staged->fences[i])
> + continue;
> +
> + dma_fence_put(staged->fences[i]);
> + staged->fences[i] = dma_fence_get(f);
> + }
> + for (;i < staged->num_fences; ++i)
> + dma_fence_put(staged->fences[i]);
> + staged->num_fences = array->num_fences;
> +
> + } else if (fence) {
> + for (i = 0; i < staged->num_fences; ++i)
> + dma_fence_put(staged->fences[i]);
> + staged->fences[0] = dma_fence_get(fence);
> + staged->num_fences = 1;
> + } else {
> + for (i = 0; i < staged->num_fences; ++i)
> + dma_fence_put(staged->fences[i]);
> + staged->num_fences = 0;
> + }
> +
> + fences->staged = staged;
> + return;
> +
> +drop_old:
> + dma_fence_put(old);
> +}
> +
> +/**
> + * dma_resv_fences_set - set the singleton fence
> + * @fences: fences object where to set the fence
> + * @fence: singleton fence for the object
> + *
> + * Grabs a reference to the new fence and replaces the current singleton fence
> + * with a new one. If the old fence is a dma_fence_array try to recycle it.
> + */
> +void dma_resv_fences_set(struct dma_resv *obj,
> + struct dma_resv_fences *fences,
> + struct dma_fence *fence)
> +{
> + dma_fence_get(fence);
> + dma_resv_fences_assign(obj, fences, fence);
> +}
> +EXPORT_SYMBOL(dma_resv_fences_set);
> +
> +/**
> + * dma_resv_fences_add - add a fence to the staged fence_array
> + * @fences: fences object where to add the fence to
> + * @fence: fence to add
> + *
> + * Add a new fence to the staged fence_array.
> + */
> +void dma_resv_fences_add(struct dma_resv_fences *fences,
> + struct dma_fence *fence)
> +{
> + struct dma_fence_array *staged = fences->staged;
> + struct dma_fence *old;
> + unsigned int i;
> +
> +#ifndef CONFIG_DEBUG_MUTEXES
> + for (i = 0; i < staged->num_fences; ++i) {
> + old = staged->fences[i];
> +
> + if (old->context == fence->context ||
> + dma_fence_is_signaled(old)) {
> + dma_fence_put(old);
> + goto replace;
> + }
> + }
> +#endif
> +
> + BUG_ON(staged->num_fences >= dma_fence_array_max_fences(staged));
> + i = staged->num_fences++;
> +
> +replace:
> + staged->fences[i] = dma_fence_get(fence);
> +}
> +EXPORT_SYMBOL(dma_resv_fences_add);
> +
> +/**
> + * dma_resv_fences_commit - commit the staged dma_fence_array
> + * @fences: fences object where the commit should happen
> + *
> + * Commit the fences staged in the dma_fence_array and make them visible to
> + * other threads.
> + */
> +void dma_resv_fences_commit(struct dma_resv *obj,
> + struct dma_resv_fences *fences)
> +{
> + struct dma_fence_array *staged = fences->staged;
> +
> + if (!staged || !staged->num_fences)
> + return;
> +
> + fences->staged = NULL;
> + dma_fence_array_init(staged, dma_fence_context_alloc(1), 1, false);
> + dma_resv_fences_assign(obj, fences, &staged->base);
> +}
> +EXPORT_SYMBOL(dma_resv_fences_commit);
> +
> /**
> * dma_resv_list_alloc - allocate fence list
> * @shared_max: number of fences we need space for
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index 03b0f95682b0..c70f13fa6789 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -45,10 +45,33 @@
> #include <linux/seqlock.h>
> #include <linux/rcupdate.h>
>
> +struct dma_resv;
> +
> extern struct ww_class reservation_ww_class;
> extern struct lock_class_key reservation_seqcount_class;
> extern const char reservation_seqcount_string[];
>
> +/**
> + * struct dma_resv_fences - fences inside a reservation object
> + * @fence: the current RCU protected singleton fence
> + * @staged: optional staged dma_fence_array to replace @fence
> + */
> +struct dma_resv_fences {
> + struct dma_fence __rcu *fence;
> + struct dma_fence_array *staged;
> +};
> +
> +int dma_resv_fences_reserve(struct dma_resv *obj,
> + struct dma_resv_fences *fences,
> + unsigned int num_fences);
> +void dma_resv_fences_set(struct dma_resv *obj,
> + struct dma_resv_fences *fences,
> + struct dma_fence *fence);
> +void dma_resv_fences_add(struct dma_resv_fences *fences,
> + struct dma_fence *fence);
> +void dma_resv_fences_commit(struct dma_resv *obj,
> + struct dma_resv_fences *fences);
> +
> /**
> * struct dma_resv_list - a list of shared fences
> * @rcu: for internal use
> @@ -80,6 +103,32 @@ struct dma_resv {
> #define dma_resv_held(obj) lockdep_is_held(&(obj)->lock.base)
> #define dma_resv_assert_held(obj) lockdep_assert_held(&(obj)->lock.base)
>
> +/**
> + * dma_resv_fences_deref - get singleton fence
> + * @obj: the reservation object
> + * @fences: the fences object
> + *
> + * Returns the singleton fence from a resv_fences object.
> + */
> +static inline struct dma_fence *
> +dma_resv_fences_deref(struct dma_resv *obj, struct dma_resv_fences *fences)
> +{
> + return rcu_dereference_protected(fences->fence,
> + dma_resv_held(obj));
> +}
> +
> +/**
> + * dma_resv_fences_get_rcu - RCU get single fence
> + * @fences: fences structure where we need to get a reference for
> + *
> + * Get a reference to the single fence representing the synchronization.
> + */
> +static inline struct dma_fence *
> +dma_resv_fences_get_rcu(struct dma_resv_fences *fences)
> +{
> + return dma_fence_get_rcu_safe(&fences->fence);
> +}
> +
> /**
> * dma_resv_get_list - get the reservation object's
> * shared fence list, with update-side lock held
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
next prev parent reply other threads:[~2019-08-21 16:05 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-21 12:31 [RFC] replacing dma_resv API Christian König
2019-08-21 12:31 ` [PATCH 01/10] dma-buf: make to_dma_fence_array NULL safe Christian König
2019-08-21 12:31 ` [PATCH 02/10] dma-buf: add dma_fence_array_alloc/free Christian König
2019-08-21 12:31 ` [PATCH 03/10] dma-buf: add dma_fence_array_recycle Christian König
2019-08-21 16:24 ` Chris Wilson
2019-08-22 8:38 ` Christian König
2019-08-21 12:31 ` [PATCH 04/10] dma-buf: add dma_fence_array_for_each Christian König
2019-08-21 12:31 ` [PATCH 05/10] dma-buf/resv: add dma_resv_prune_fences Christian König
2019-08-21 14:55 ` Chris Wilson
2019-08-21 14:56 ` Chris Wilson
2019-08-21 12:31 ` [PATCH 06/10] dma-buf/resv: stop pruning shared fences when exclusive is added Christian König
2019-08-21 12:31 ` [PATCH 07/10] dma-buf/resv: add new fences container implementation Christian König
2019-08-21 16:04 ` Daniel Vetter [this message]
2019-08-22 8:23 ` Christian König
2019-08-22 13:02 ` Daniel Vetter
2019-08-22 13:53 ` Koenig, Christian
2019-08-21 12:31 ` [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container Christian König
2019-08-21 15:24 ` Chris Wilson
2019-08-21 17:35 ` Chris Wilson
2019-08-22 8:37 ` Christian König
2019-08-22 9:16 ` Christian König
2019-08-21 16:21 ` Chris Wilson
2019-08-24 13:22 ` Chris Wilson
2019-08-21 12:31 ` [PATCH 09/10] dma-buf/resv: replace exclusive " Christian König
2019-08-21 12:31 ` [PATCH 10/10] dma-buf/resv: add other operations Christian König
2019-08-22 12:28 ` Ville Syrjälä
2019-08-21 16:13 ` [RFC] replacing dma_resv API Daniel Vetter
2019-08-21 20:05 ` Daniel Vetter
2019-08-22 9:27 ` Christian König
2019-08-21 20:11 ` Chris Wilson
2019-08-21 20:22 ` Daniel Vetter
2019-08-22 9:14 ` Christian König
2019-08-22 10:00 ` 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=20190821160454.GN11147@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=chris@chris-wilson.co.uk \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-media@vger.kernel.org \
--cc=sumit.semwal@linaro.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;
as well as URLs for NNTP newsgroup(s).