Re: [PATCH 1/2] dma-buf: make reservation_object_copy_fences rcu save

[Christian König <deathsimple@xxxxxxxxxxx>]

Ping? David can you take a look?

Alex is on vacation and that is a rather important bug fix.

Thanks,
Christian.

Am 04.09.2017 um 21:02 schrieb Christian König:
> From: Christian König <christian.koenig@xxxxxxx>
>
> Stop requiring that the src reservation object is locked for this operation.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
>   drivers/dma-buf/reservation.c | 56 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 42 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index dec3a81..b44d9d7 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -266,8 +266,7 @@ EXPORT_SYMBOL(reservation_object_add_excl_fence);
>   * @dst: the destination reservation object
>   * @src: the source reservation object
>   *
> -* Copy all fences from src to dst. Both src->lock as well as dst-lock must be
> -* held.
> +* Copy all fences from src to dst. dst-lock must be held.
>   */
>   int reservation_object_copy_fences(struct reservation_object *dst,
>   				   struct reservation_object *src)
> @@ -277,33 +276,62 @@ int reservation_object_copy_fences(struct reservation_object *dst,
>   	size_t size;
>   	unsigned i;
>   
> -	src_list = reservation_object_get_list(src);
> +	rcu_read_lock();
> +	src_list = rcu_dereference(src->fence);
>   
> +retry:
>   	if (src_list) {
> -		size = offsetof(typeof(*src_list),
> -				shared[src_list->shared_count]);
> +		unsigned shared_count = src_list->shared_count;
> +
> +		size = offsetof(typeof(*src_list), shared[shared_count]);
> +		rcu_read_unlock();
> +
>   		dst_list = kmalloc(size, GFP_KERNEL);
>   		if (!dst_list)
>   			return -ENOMEM;
>   
> -		dst_list->shared_count = src_list->shared_count;
> -		dst_list->shared_max = src_list->shared_count;
> -		for (i = 0; i < src_list->shared_count; ++i)
> -			dst_list->shared[i] =
> -				dma_fence_get(src_list->shared[i]);
> +		rcu_read_lock();
> +		src_list = rcu_dereference(src->fence);
> +		if (!src_list || src_list->shared_count > shared_count) {
> +			kfree(dst_list);
> +			goto retry;
> +		}
> +
> +		dst_list->shared_count = 0;
> +		dst_list->shared_max = shared_count;
> +		for (i = 0; i < src_list->shared_count; ++i) {
> +			struct dma_fence *fence;
> +
> +			fence = rcu_dereference(src_list->shared[i]);
> +			if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> +				     &fence->flags))
> +				continue;
> +
> +			if (!dma_fence_get_rcu(fence)) {
> +				kfree(dst_list);
> +				src_list = rcu_dereference(src->fence);
> +				goto retry;
> +			}
> +
> +			if (dma_fence_is_signaled(fence)) {
> +				dma_fence_put(fence);
> +				continue;
> +			}
> +
> +			dst_list->shared[dst_list->shared_count++] = fence;
> +		}
>   	} else {
>   		dst_list = NULL;
>   	}
>   
> +	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);
> -	new = reservation_object_get_excl(src);
> -
> -	dma_fence_get(new);
>   
>   	preempt_disable();
>   	write_seqcount_begin(&dst->seq);


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel