[PATCH] dma-buf: make returning the exclusive fence optional

[PATCH] dma-buf: make returning the exclusive fence optional

[Christian König]

Change reservation_object_get_fences_rcu to make the exclusive fence
pointer optional.

If not specified the exclusive fence is put into the fence array as
well.

This is helpful for a couple of cases where we need all fences in a
single array.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b759a569b7b8..461afa9febd4 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
  * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
  * the required size, and must be freed by caller)
  *
- * RETURNS
- * Zero or -errno
+ * Retrieve all fences from the reservation object. If the pointer for the
+ * exclusive fence is not specified the fence is put into the array of the
+ * shared fences as well. Returns either zero or -ENOMEM.
  */
 int reservation_object_get_fences_rcu(struct reservation_object *obj,
 				      struct dma_fence **pfence_excl,
@@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
 
 	do {
 		struct reservation_object_list *fobj;
-		unsigned seq;
-		unsigned int i;
+		unsigned int i, seq;
+		size_t sz = 0;
 
 		shared_count = i = 0;
 
@@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
 			goto unlock;
 
 		fobj = rcu_dereference(obj->fence);
-		if (fobj) {
+		if (fobj)
+			sz += sizeof(*shared) * fobj->shared_max;
+
+		if (!pfence_excl && fence_excl)
+			sz += sizeof(*shared);
+
+		if (sz) {
 			struct dma_fence **nshared;
-			size_t sz = sizeof(*shared) * fobj->shared_max;
 
 			nshared = krealloc(shared, sz,
 					   GFP_NOWAIT | __GFP_NOWARN);
@@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
 				break;
 			}
 			shared = nshared;
-			shared_count = fobj->shared_count;
-
+			shared_count = fobj ? fobj->shared_count : 0;
 			for (i = 0; i < shared_count; ++i) {
 				shared[i] = rcu_dereference(fobj->shared[i]);
 				if (!dma_fence_get_rcu(shared[i]))
 					break;
 			}
+
+			if (!pfence_excl && fence_excl) {
+				shared[i] = fence_excl;
+				fence_excl = NULL;
+				++i;
+				++shared_count;
+			}
 		}
 
 		if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
@@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
 
 	*pshared_count = shared_count;
 	*pshared = shared;
-	*pfence_excl = fence_excl;
+	if (pfence_excl)
+		*pfence_excl = fence_excl;
 
 	return ret;
 }
-- 
2.14.1

Re: [Linaro-mm-sig] [PATCH] dma-buf: make returning the exclusive fence optional

[Daniel Vetter]

On Wed, Jan 10, 2018 at 01:53:41PM +0100, Christian König wrote:
> Change reservation_object_get_fences_rcu to make the exclusive fence
> pointer optional.
> 
> If not specified the exclusive fence is put into the fence array as
> well.
> 
> This is helpful for a couple of cases where we need all fences in a
> single array.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Seeing the use-case for this would be a lot more interesting ...
-Daniel

> ---
>  drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index b759a569b7b8..461afa9febd4 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
>   * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
>   * the required size, and must be freed by caller)
>   *
> - * RETURNS
> - * Zero or -errno
> + * Retrieve all fences from the reservation object. If the pointer for the
> + * exclusive fence is not specified the fence is put into the array of the
> + * shared fences as well. Returns either zero or -ENOMEM.
>   */
>  int reservation_object_get_fences_rcu(struct reservation_object *obj,
>  				      struct dma_fence **pfence_excl,
> @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>  
>  	do {
>  		struct reservation_object_list *fobj;
> -		unsigned seq;
> -		unsigned int i;
> +		unsigned int i, seq;
> +		size_t sz = 0;
>  
>  		shared_count = i = 0;
>  
> @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>  			goto unlock;
>  
>  		fobj = rcu_dereference(obj->fence);
> -		if (fobj) {
> +		if (fobj)
> +			sz += sizeof(*shared) * fobj->shared_max;
> +
> +		if (!pfence_excl && fence_excl)
> +			sz += sizeof(*shared);
> +
> +		if (sz) {
>  			struct dma_fence **nshared;
> -			size_t sz = sizeof(*shared) * fobj->shared_max;
>  
>  			nshared = krealloc(shared, sz,
>  					   GFP_NOWAIT | __GFP_NOWARN);
> @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>  				break;
>  			}
>  			shared = nshared;
> -			shared_count = fobj->shared_count;
> -
> +			shared_count = fobj ? fobj->shared_count : 0;
>  			for (i = 0; i < shared_count; ++i) {
>  				shared[i] = rcu_dereference(fobj->shared[i]);
>  				if (!dma_fence_get_rcu(shared[i]))
>  					break;
>  			}
> +
> +			if (!pfence_excl && fence_excl) {
> +				shared[i] = fence_excl;
> +				fence_excl = NULL;
> +				++i;
> +				++shared_count;
> +			}
>  		}
>  
>  		if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
> @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>  
>  	*pshared_count = shared_count;
>  	*pshared = shared;
> -	*pfence_excl = fence_excl;
> +	if (pfence_excl)
> +		*pfence_excl = fence_excl;
>  
>  	return ret;
>  }
> -- 
> 2.14.1
> 
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Linaro-mm-sig] [PATCH] dma-buf: make returning the exclusive fence optional

[Christian König]

Am 10.01.2018 um 14:21 schrieb Daniel Vetter:
> On Wed, Jan 10, 2018 at 01:53:41PM +0100, Christian König wrote:
>> Change reservation_object_get_fences_rcu to make the exclusive fence
>> pointer optional.
>>
>> If not specified the exclusive fence is put into the fence array as
>> well.
>>
>> This is helpful for a couple of cases where we need all fences in a
>> single array.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> Seeing the use-case for this would be a lot more interesting ...

Yeah, sorry the use case is a 20 patches set on amd-gfx.

Didn't wanted to post all those here as well.

Christian.

> -Daniel
>
>> ---
>>   drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++---------
>>   1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
>> index b759a569b7b8..461afa9febd4 100644
>> --- a/drivers/dma-buf/reservation.c
>> +++ b/drivers/dma-buf/reservation.c
>> @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
>>    * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
>>    * the required size, and must be freed by caller)
>>    *
>> - * RETURNS
>> - * Zero or -errno
>> + * Retrieve all fences from the reservation object. If the pointer for the
>> + * exclusive fence is not specified the fence is put into the array of the
>> + * shared fences as well. Returns either zero or -ENOMEM.
>>    */
>>   int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>   				      struct dma_fence **pfence_excl,
>> @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>   
>>   	do {
>>   		struct reservation_object_list *fobj;
>> -		unsigned seq;
>> -		unsigned int i;
>> +		unsigned int i, seq;
>> +		size_t sz = 0;
>>   
>>   		shared_count = i = 0;
>>   
>> @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>   			goto unlock;
>>   
>>   		fobj = rcu_dereference(obj->fence);
>> -		if (fobj) {
>> +		if (fobj)
>> +			sz += sizeof(*shared) * fobj->shared_max;
>> +
>> +		if (!pfence_excl && fence_excl)
>> +			sz += sizeof(*shared);
>> +
>> +		if (sz) {
>>   			struct dma_fence **nshared;
>> -			size_t sz = sizeof(*shared) * fobj->shared_max;
>>   
>>   			nshared = krealloc(shared, sz,
>>   					   GFP_NOWAIT | __GFP_NOWARN);
>> @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>   				break;
>>   			}
>>   			shared = nshared;
>> -			shared_count = fobj->shared_count;
>> -
>> +			shared_count = fobj ? fobj->shared_count : 0;
>>   			for (i = 0; i < shared_count; ++i) {
>>   				shared[i] = rcu_dereference(fobj->shared[i]);
>>   				if (!dma_fence_get_rcu(shared[i]))
>>   					break;
>>   			}
>> +
>> +			if (!pfence_excl && fence_excl) {
>> +				shared[i] = fence_excl;
>> +				fence_excl = NULL;
>> +				++i;
>> +				++shared_count;
>> +			}
>>   		}
>>   
>>   		if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
>> @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
>>   
>>   	*pshared_count = shared_count;
>>   	*pshared = shared;
>> -	*pfence_excl = fence_excl;
>> +	if (pfence_excl)
>> +		*pfence_excl = fence_excl;
>>   
>>   	return ret;
>>   }
>> -- 
>> 2.14.1
>>
>> _______________________________________________
>> Linaro-mm-sig mailing list
>> Linaro-mm-sig@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

Re: [Linaro-mm-sig] [PATCH] dma-buf: make returning the exclusive fence optional

[Daniel Vetter]

On Wed, Jan 10, 2018 at 02:46:32PM +0100, Christian König wrote:
> Am 10.01.2018 um 14:21 schrieb Daniel Vetter:
> > On Wed, Jan 10, 2018 at 01:53:41PM +0100, Christian König wrote:
> > > Change reservation_object_get_fences_rcu to make the exclusive fence
> > > pointer optional.
> > > 
> > > If not specified the exclusive fence is put into the fence array as
> > > well.
> > > 
> > > This is helpful for a couple of cases where we need all fences in a
> > > single array.
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > Seeing the use-case for this would be a lot more interesting ...
> 
> Yeah, sorry the use case is a 20 patches set on amd-gfx.
> 
> Didn't wanted to post all those here as well.

Imo better to spam more lists instead of splitting up discussions ... It's
at least what we tend to do for i915 stuff, and no one seems to complain.
-Daniel

> 
> Christian.
> 
> > -Daniel
> > 
> > > ---
> > >   drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++---------
> > >   1 file changed, 22 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> > > index b759a569b7b8..461afa9febd4 100644
> > > --- a/drivers/dma-buf/reservation.c
> > > +++ b/drivers/dma-buf/reservation.c
> > > @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences);
> > >    * @pshared: the array of shared fence ptrs returned (array is krealloc'd to
> > >    * the required size, and must be freed by caller)
> > >    *
> > > - * RETURNS
> > > - * Zero or -errno
> > > + * Retrieve all fences from the reservation object. If the pointer for the
> > > + * exclusive fence is not specified the fence is put into the array of the
> > > + * shared fences as well. Returns either zero or -ENOMEM.
> > >    */
> > >   int reservation_object_get_fences_rcu(struct reservation_object *obj,
> > >   				      struct dma_fence **pfence_excl,
> > > @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
> > >   	do {
> > >   		struct reservation_object_list *fobj;
> > > -		unsigned seq;
> > > -		unsigned int i;
> > > +		unsigned int i, seq;
> > > +		size_t sz = 0;
> > >   		shared_count = i = 0;
> > > @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
> > >   			goto unlock;
> > >   		fobj = rcu_dereference(obj->fence);
> > > -		if (fobj) {
> > > +		if (fobj)
> > > +			sz += sizeof(*shared) * fobj->shared_max;
> > > +
> > > +		if (!pfence_excl && fence_excl)
> > > +			sz += sizeof(*shared);
> > > +
> > > +		if (sz) {
> > >   			struct dma_fence **nshared;
> > > -			size_t sz = sizeof(*shared) * fobj->shared_max;
> > >   			nshared = krealloc(shared, sz,
> > >   					   GFP_NOWAIT | __GFP_NOWARN);
> > > @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
> > >   				break;
> > >   			}
> > >   			shared = nshared;
> > > -			shared_count = fobj->shared_count;
> > > -
> > > +			shared_count = fobj ? fobj->shared_count : 0;
> > >   			for (i = 0; i < shared_count; ++i) {
> > >   				shared[i] = rcu_dereference(fobj->shared[i]);
> > >   				if (!dma_fence_get_rcu(shared[i]))
> > >   					break;
> > >   			}
> > > +
> > > +			if (!pfence_excl && fence_excl) {
> > > +				shared[i] = fence_excl;
> > > +				fence_excl = NULL;
> > > +				++i;
> > > +				++shared_count;
> > > +			}
> > >   		}
> > >   		if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
> > > @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj,
> > >   	*pshared_count = shared_count;
> > >   	*pshared = shared;
> > > -	*pfence_excl = fence_excl;
> > > +	if (pfence_excl)
> > > +		*pfence_excl = fence_excl;
> > >   	return ret;
> > >   }
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > Linaro-mm-sig mailing list
> > > Linaro-mm-sig@lists.linaro.org
> > > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch