Re: [PATCH 07/10] dma-buf/resv: add new fences container implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
> ---
>  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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux