[PATCH 1/8] dma-buf: remove shared fence staging in reservation object

Christian König ckoenig.leichtzumerken at gmail.com
Fri Oct 12 08:22:00 UTC 2018


Ping! Adding a few people directly and more mailing lists.

Can I get an acked-by/rb for this? It's only a cleanup and not much 
functional change.

Regards,
Christian.

Am 04.10.2018 um 15:12 schrieb Christian König:
> No need for that any more. Just replace the list when there isn't enough
> room any more for the additional fence.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/dma-buf/reservation.c | 178 ++++++++++++++----------------------------
>   include/linux/reservation.h   |   4 -
>   2 files changed, 58 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 6c95f61a32e7..5825fc336a13 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -68,105 +68,23 @@ EXPORT_SYMBOL(reservation_seqcount_string);
>    */
>   int reservation_object_reserve_shared(struct reservation_object *obj)
>   {
> -	struct reservation_object_list *fobj, *old;
> -	u32 max;
> +	struct reservation_object_list *old, *new;
> +	unsigned int i, j, k, max;
>   
>   	old = reservation_object_get_list(obj);
>   
>   	if (old && old->shared_max) {
> -		if (old->shared_count < old->shared_max) {
> -			/* perform an in-place update */
> -			kfree(obj->staged);
> -			obj->staged = NULL;
> +		if (old->shared_count < old->shared_max)
>   			return 0;
> -		} else
> +		else
>   			max = old->shared_max * 2;
> -	} else
> -		max = 4;
> -
> -	/*
> -	 * resize obj->staged or allocate if it doesn't exist,
> -	 * noop if already correct size
> -	 */
> -	fobj = krealloc(obj->staged, offsetof(typeof(*fobj), shared[max]),
> -			GFP_KERNEL);
> -	if (!fobj)
> -		return -ENOMEM;
> -
> -	obj->staged = fobj;
> -	fobj->shared_max = max;
> -	return 0;
> -}
> -EXPORT_SYMBOL(reservation_object_reserve_shared);
> -
> -static void
> -reservation_object_add_shared_inplace(struct reservation_object *obj,
> -				      struct reservation_object_list *fobj,
> -				      struct dma_fence *fence)
> -{
> -	struct dma_fence *signaled = NULL;
> -	u32 i, signaled_idx;
> -
> -	dma_fence_get(fence);
> -
> -	preempt_disable();
> -	write_seqcount_begin(&obj->seq);
> -
> -	for (i = 0; i < fobj->shared_count; ++i) {
> -		struct dma_fence *old_fence;
> -
> -		old_fence = rcu_dereference_protected(fobj->shared[i],
> -						reservation_object_held(obj));
> -
> -		if (old_fence->context == fence->context) {
> -			/* memory barrier is added by write_seqcount_begin */
> -			RCU_INIT_POINTER(fobj->shared[i], fence);
> -			write_seqcount_end(&obj->seq);
> -			preempt_enable();
> -
> -			dma_fence_put(old_fence);
> -			return;
> -		}
> -
> -		if (!signaled && dma_fence_is_signaled(old_fence)) {
> -			signaled = old_fence;
> -			signaled_idx = i;
> -		}
> -	}
> -
> -	/*
> -	 * memory barrier is added by write_seqcount_begin,
> -	 * fobj->shared_count is protected by this lock too
> -	 */
> -	if (signaled) {
> -		RCU_INIT_POINTER(fobj->shared[signaled_idx], fence);
>   	} else {
> -		BUG_ON(fobj->shared_count >= fobj->shared_max);
> -		RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
> -		fobj->shared_count++;
> +		max = 4;
>   	}
>   
> -	write_seqcount_end(&obj->seq);
> -	preempt_enable();
> -
> -	dma_fence_put(signaled);
> -}
> -
> -static void
> -reservation_object_add_shared_replace(struct reservation_object *obj,
> -				      struct reservation_object_list *old,
> -				      struct reservation_object_list *fobj,
> -				      struct dma_fence *fence)
> -{
> -	unsigned i, j, k;
> -
> -	dma_fence_get(fence);
> -
> -	if (!old) {
> -		RCU_INIT_POINTER(fobj->shared[0], fence);
> -		fobj->shared_count = 1;
> -		goto done;
> -	}
> +	new = kmalloc(offsetof(typeof(*new), shared[max]), GFP_KERNEL);
> +	if (!new)
> +		return -ENOMEM;
>   
>   	/*
>   	 * no need to bump fence refcounts, rcu_read access
> @@ -174,46 +92,45 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   	 * references from the old struct are carried over to
>   	 * the new.
>   	 */
> -	for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) {
> -		struct dma_fence *check;
> +	for (i = 0, j = 0, k = max; i < (old ? old->shared_count : 0); ++i) {
> +		struct dma_fence *fence;
>   
> -		check = rcu_dereference_protected(old->shared[i],
> -						reservation_object_held(obj));
> -
> -		if (check->context == fence->context ||
> -		    dma_fence_is_signaled(check))
> -			RCU_INIT_POINTER(fobj->shared[--k], check);
> +		fence = rcu_dereference_protected(old->shared[i],
> +						  reservation_object_held(obj));
> +		if (dma_fence_is_signaled(fence))
> +			RCU_INIT_POINTER(new->shared[--k], fence);
>   		else
> -			RCU_INIT_POINTER(fobj->shared[j++], check);
> +			RCU_INIT_POINTER(new->shared[j++], fence);
>   	}
> -	fobj->shared_count = j;
> -	RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
> -	fobj->shared_count++;
> +	new->shared_count = j;
> +	new->shared_max = max;
>   
> -done:
>   	preempt_disable();
>   	write_seqcount_begin(&obj->seq);
>   	/*
>   	 * RCU_INIT_POINTER can be used here,
>   	 * seqcount provides the necessary barriers
>   	 */
> -	RCU_INIT_POINTER(obj->fence, fobj);
> +	RCU_INIT_POINTER(obj->fence, new);
>   	write_seqcount_end(&obj->seq);
>   	preempt_enable();
>   
>   	if (!old)
> -		return;
> +		return 0;
>   
>   	/* Drop the references to the signaled fences */
> -	for (i = k; i < fobj->shared_max; ++i) {
> -		struct dma_fence *f;
> +	for (i = k; i < new->shared_max; ++i) {
> +		struct dma_fence *fence;
>   
> -		f = rcu_dereference_protected(fobj->shared[i],
> -					      reservation_object_held(obj));
> -		dma_fence_put(f);
> +		fence = rcu_dereference_protected(new->shared[i],
> +						  reservation_object_held(obj));
> +		dma_fence_put(fence);
>   	}
>   	kfree_rcu(old, rcu);
> +
> +	return 0;
>   }
> +EXPORT_SYMBOL(reservation_object_reserve_shared);
>   
>   /**
>    * reservation_object_add_shared_fence - Add a fence to a shared slot
> @@ -226,15 +143,39 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   void reservation_object_add_shared_fence(struct reservation_object *obj,
>   					 struct dma_fence *fence)
>   {
> -	struct reservation_object_list *old, *fobj = obj->staged;
> +	struct reservation_object_list *fobj;
> +	unsigned int i;
>   
> -	old = reservation_object_get_list(obj);
> -	obj->staged = NULL;
> +	dma_fence_get(fence);
> +
> +	fobj = reservation_object_get_list(obj);
>   
> -	if (!fobj)
> -		reservation_object_add_shared_inplace(obj, old, fence);
> -	else
> -		reservation_object_add_shared_replace(obj, old, fobj, fence);
> +	preempt_disable();
> +	write_seqcount_begin(&obj->seq);
> +
> +	for (i = 0; i < fobj->shared_count; ++i) {
> +		struct dma_fence *old_fence;
> +
> +		old_fence = rcu_dereference_protected(fobj->shared[i],
> +						      reservation_object_held(obj));
> +		if (old_fence->context == fence->context ||
> +		    dma_fence_is_signaled(old_fence)) {
> +			dma_fence_put(old_fence);
> +			goto replace;
> +		}
> +	}
> +
> +	BUG_ON(fobj->shared_count >= fobj->shared_max);
> +	fobj->shared_count++;
> +
> +replace:
> +	/*
> +	 * memory barrier is added by write_seqcount_begin,
> +	 * fobj->shared_count is protected by this lock too
> +	 */
> +	RCU_INIT_POINTER(fobj->shared[i], fence);
> +	write_seqcount_end(&obj->seq);
> +	preempt_enable();
>   }
>   EXPORT_SYMBOL(reservation_object_add_shared_fence);
>   
> @@ -343,9 +284,6 @@ int reservation_object_copy_fences(struct reservation_object *dst,
>   	new = dma_fence_get_rcu_safe(&src->fence_excl);
>   	rcu_read_unlock();
>   
> -	kfree(dst->staged);
> -	dst->staged = NULL;
> -
>   	src_list = reservation_object_get_list(dst);
>   	old = reservation_object_get_excl(dst);
>   
> diff --git a/include/linux/reservation.h b/include/linux/reservation.h
> index 02166e815afb..54cf6773a14c 100644
> --- a/include/linux/reservation.h
> +++ b/include/linux/reservation.h
> @@ -68,7 +68,6 @@ struct reservation_object_list {
>    * @seq: sequence count for managing RCU read-side synchronization
>    * @fence_excl: the exclusive fence, if there is one currently
>    * @fence: list of current shared fences
> - * @staged: staged copy of shared fences for RCU updates
>    */
>   struct reservation_object {
>   	struct ww_mutex lock;
> @@ -76,7 +75,6 @@ struct reservation_object {
>   
>   	struct dma_fence __rcu *fence_excl;
>   	struct reservation_object_list __rcu *fence;
> -	struct reservation_object_list *staged;
>   };
>   
>   #define reservation_object_held(obj) lockdep_is_held(&(obj)->lock.base)
> @@ -95,7 +93,6 @@ reservation_object_init(struct reservation_object *obj)
>   	__seqcount_init(&obj->seq, reservation_seqcount_string, &reservation_seqcount_class);
>   	RCU_INIT_POINTER(obj->fence, NULL);
>   	RCU_INIT_POINTER(obj->fence_excl, NULL);
> -	obj->staged = NULL;
>   }
>   
>   /**
> @@ -124,7 +121,6 @@ reservation_object_fini(struct reservation_object *obj)
>   
>   		kfree(fobj);
>   	}
> -	kfree(obj->staged);
>   
>   	ww_mutex_destroy(&obj->lock);
>   }



More information about the amd-gfx mailing list