[PATCH 1/2] dma-buf: keep only not signaled fence in reservation_object_add_shared_replace

[Chunming Zhou]

On 2017年10月24日 21:55, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> The amdgpu issue to also need signaled fences in the reservation objects
> should be fixed by now.
>
> Optimize the list by keeping only the not signaled yet fences around.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/dma-buf/reservation.c | 31 +++++++++++++++++++++----------
>   1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index b44d9d7db347..4ede77d1bb31 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -145,8 +145,8 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   				      struct reservation_object_list *fobj,
>   				      struct dma_fence *fence)
>   {
> -	unsigned i;
>   	struct dma_fence *old_fence = NULL;
> +	unsigned i, j, k;
>   
>   	dma_fence_get(fence);
>   
> @@ -162,9 +162,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   	 * references from the old struct are carried over to
>   	 * the new.
>   	 */
> -	fobj->shared_count = old->shared_count;
> -
> -	for (i = 0; i < old->shared_count; ++i) {
> +	for (i = 0, j = 0, k = old->shared_count; i < old->shared_count; ++i) {
>   		struct dma_fence *check;
>   
>   		check = rcu_dereference_protected(old->shared[i],
> @@ -172,10 +170,14 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   
>   		if (!old_fence && check->context == fence->context) {
>   			old_fence = check;
> -			RCU_INIT_POINTER(fobj->shared[i], fence);
> -		} else
> -			RCU_INIT_POINTER(fobj->shared[i], check);
> +			RCU_INIT_POINTER(fobj->shared[j++], fence);
> +		} else if (!dma_fence_is_signaled(check)) {
> +			RCU_INIT_POINTER(fobj->shared[j++], check);
> +		} else {
> +			RCU_INIT_POINTER(fobj->shared[--k], check);
> +		}
>   	}
> +	fobj->shared_count = j;
>   	if (!old_fence) {
>   		RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
Here there is a memory leak for signaled fence slots, since you re-order 
the slots, the j'th slot is storing signaled fence, there is no place to 
put it when you assign to new one.
you cam move it to end or put it here first.

Regards,
David Zhou
>   		fobj->shared_count++;
> @@ -192,10 +194,19 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   	write_seqcount_end(&obj->seq);
>   	preempt_enable();
>   
> -	if (old)
> -		kfree_rcu(old, rcu);
> -
>   	dma_fence_put(old_fence);
> +
> +	if (!old)
> +		return;
> +
> +	for (i = fobj->shared_count; i < old->shared_count; ++i) {
> +		struct dma_fence *f;
> +
> +		f = rcu_dereference_protected(fobj->shared[i],
> +					      reservation_object_held(obj));
> +		dma_fence_put(f);
> +	}
> +	kfree_rcu(old, rcu);
>   }
>   
>   /**