Re: [PATCH v3 1/2] dma-buf: Split out dma fence array create into alloc and arm functions

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Aug 23, 2024 at 08:37:30AM +0200, Christian König wrote:
> Am 23.08.24 um 06:54 schrieb Matthew Brost:
> > Useful to preallocate dma fence array and then arm in path of reclaim or
> > a dma fence.
> 
> Exactly that was rejected before because it allows to create circle
> dependencies.
> 

Can you explain or do you have link to that discussion? Trying to think
how this would be problematic and failing to see how it is. 

> You would need a really really good argument why that is necessary.
>

It seems quite useful when you have a code path in which you know N fences
will be generated, prealloc a dma fence array, then populate at
later time ensuring no failures points (malloc), and then finally
install dma fence array in timeline sync obj (chain fences not allowed).

It fits nicely for VM bind operations in which a device has multple
TLBs and the TLB invalidation completion is a fence. I suspect Intel
can't be the only device out their with multiple TLBs, does VM bind, and
use timeline sync obj.

Matt

> Regards,
> Christian.
> 
> > 
> > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > Cc: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > ---
> >   drivers/dma-buf/dma-fence-array.c | 81 ++++++++++++++++++++++---------
> >   include/linux/dma-fence-array.h   |  7 +++
> >   2 files changed, 66 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c
> > index c74ac197d5fe..b03e0a87a5cd 100644
> > --- a/drivers/dma-buf/dma-fence-array.c
> > +++ b/drivers/dma-buf/dma-fence-array.c
> > @@ -144,36 +144,38 @@ const struct dma_fence_ops dma_fence_array_ops = {
> >   EXPORT_SYMBOL(dma_fence_array_ops);
> >   /**
> > - * dma_fence_array_create - Create a custom fence array
> > + * dma_fence_array_alloc - Allocate a custom fence array
> > + * @num_fences:		[in]	number of fences to add in the array
> > + *
> > + * Return dma fence array on success, NULL on failure
> > + */
> > +struct dma_fence_array *dma_fence_array_alloc(int num_fences)
> > +{
> > +	struct dma_fence_array *array;
> > +
> > +	return kzalloc(struct_size(array, callbacks, num_fences), GFP_KERNEL);
> > +}
> > +EXPORT_SYMBOL(dma_fence_array_alloc);
> > +
> > +/**
> > + * dma_fence_array_arm - Arm a custom fence array
> > + * @array:		[in]	dma fence array to arm
> >    * @num_fences:		[in]	number of fences to add in the array
> >    * @fences:		[in]	array containing the fences
> >    * @context:		[in]	fence context to use
> >    * @seqno:		[in]	sequence number to use
> >    * @signal_on_any:	[in]	signal on any fence in the array
> >    *
> > - * Allocate a dma_fence_array object and initialize the base fence with
> > - * dma_fence_init().
> > - * In case of error it returns NULL.
> > - *
> > - * The caller should allocate the fences array with num_fences size
> > - * and fill it with the fences it wants to add to the object. Ownership of this
> > - * array is taken and dma_fence_put() is used on each fence on release.
> > - *
> > - * If @signal_on_any is true the fence array signals if any fence in the array
> > - * signals, otherwise it signals when all fences in the array signal.
> > + * Implementation of @dma_fence_array_create without allocation. Useful to arm a
> > + * preallocated dma fence fence in the path of reclaim or dma fence signaling.
> >    */
> > -struct dma_fence_array *dma_fence_array_create(int num_fences,
> > -					       struct dma_fence **fences,
> > -					       u64 context, unsigned seqno,
> > -					       bool signal_on_any)
> > +void dma_fence_array_arm(struct dma_fence_array *array,
> > +			 int num_fences,
> > +			 struct dma_fence **fences,
> > +			 u64 context, unsigned seqno,
> > +			 bool signal_on_any)
> >   {
> > -	struct dma_fence_array *array;
> > -
> > -	WARN_ON(!num_fences || !fences);
> > -
> > -	array = kzalloc(struct_size(array, callbacks, num_fences), GFP_KERNEL);
> > -	if (!array)
> > -		return NULL;
> > +	WARN_ON(!array || !num_fences || !fences);
> >   	array->num_fences = num_fences;
> > @@ -200,6 +202,41 @@ struct dma_fence_array *dma_fence_array_create(int num_fences,
> >   	 */
> >   	while (num_fences--)
> >   		WARN_ON(dma_fence_is_container(fences[num_fences]));
> > +}
> > +EXPORT_SYMBOL(dma_fence_array_arm);
> > +
> > +/**
> > + * dma_fence_array_create - Create a custom fence array
> > + * @num_fences:		[in]	number of fences to add in the array
> > + * @fences:		[in]	array containing the fences
> > + * @context:		[in]	fence context to use
> > + * @seqno:		[in]	sequence number to use
> > + * @signal_on_any:	[in]	signal on any fence in the array
> > + *
> > + * Allocate a dma_fence_array object and initialize the base fence with
> > + * dma_fence_init().
> > + * In case of error it returns NULL.
> > + *
> > + * The caller should allocate the fences array with num_fences size
> > + * and fill it with the fences it wants to add to the object. Ownership of this
> > + * array is taken and dma_fence_put() is used on each fence on release.
> > + *
> > + * If @signal_on_any is true the fence array signals if any fence in the array
> > + * signals, otherwise it signals when all fences in the array signal.
> > + */
> > +struct dma_fence_array *dma_fence_array_create(int num_fences,
> > +					       struct dma_fence **fences,
> > +					       u64 context, unsigned seqno,
> > +					       bool signal_on_any)
> > +{
> > +	struct dma_fence_array *array;
> > +
> > +	array = dma_fence_array_alloc(num_fences);
> > +	if (!array)
> > +		return NULL;
> > +
> > +	dma_fence_array_arm(array, num_fences, fences,
> > +			    context, seqno, signal_on_any);
> >   	return array;
> >   }
> > diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h
> > index 29c5650c1038..3466ffc4b803 100644
> > --- a/include/linux/dma-fence-array.h
> > +++ b/include/linux/dma-fence-array.h
> > @@ -79,6 +79,13 @@ to_dma_fence_array(struct dma_fence *fence)
> >   	for (index = 0, fence = dma_fence_array_first(head); fence;	\
> >   	     ++(index), fence = dma_fence_array_next(head, index))
> > +struct dma_fence_array *dma_fence_array_alloc(int num_fences);
> > +void dma_fence_array_arm(struct dma_fence_array *array,
> > +			 int num_fences,
> > +			 struct dma_fence **fences,
> > +			 u64 context, unsigned seqno,
> > +			 bool signal_on_any);
> > +
> >   struct dma_fence_array *dma_fence_array_create(int num_fences,
> >   					       struct dma_fence **fences,
> >   					       u64 context, unsigned seqno,
>