Re: [PATCH] dma-buf/reservation: should keep later one in add fence(v3)

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

And still a NAK, the prerequisite for adding a shared fence is that it 
is later than any existing fence in that context.

In other words reservation_object_add_shared_fence() is always called 
with a new fence, never with some old one.

So the whole checking here is completely superfluous,
Christian.

Am 06.03.2018 um 08:00 schrieb Monk Liu:
> v2:
> still check context first to avoid warning from dma_fence_is_later
> apply this fix in add_shared_replace as well
>
> v3:
> use a bool falg to indict if fence is need to insert to new slot
> and ignore it if it is an eld fence compared with the one with
> the same context in old->shared
>
> Change-Id: If6a979ba9fd6c923b82212f35f07a9ff31c86767
> Signed-off-by: Monk Liu <Monk.Liu@xxxxxxx>
> ---
>   drivers/dma-buf/reservation.c | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
> index 314eb10..a7d0598 100644
> --- a/drivers/dma-buf/reservation.c
> +++ b/drivers/dma-buf/reservation.c
> @@ -118,7 +118,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj,
>   		old_fence = rcu_dereference_protected(fobj->shared[i],
>   						reservation_object_held(obj));
>   
> -		if (old_fence->context == fence->context) {
> +		if (old_fence->context == fence->context &&
> +			dma_fence_is_later(fence, old_fence)) {
>   			/* memory barrier is added by write_seqcount_begin */
>   			RCU_INIT_POINTER(fobj->shared[i], fence);
>   			write_seqcount_end(&obj->seq);
> @@ -158,6 +159,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   				      struct dma_fence *fence)
>   {
>   	unsigned i, j, k;
> +	bool wrong_fence = false;
>   
>   	dma_fence_get(fence);
>   
> @@ -179,15 +181,29 @@ reservation_object_add_shared_replace(struct reservation_object *obj,
>   		check = rcu_dereference_protected(old->shared[i],
>   						reservation_object_held(obj));
>   
> -		if (check->context == fence->context ||
> -		    dma_fence_is_signaled(check))
> +		if (dma_fence_is_signaled(check)) {
> +			/* put check to tail of fobj if signaled */
>   			RCU_INIT_POINTER(fobj->shared[--k], check);
> -		else
> +		} else if (check->context == fence->context) {
> +			if (dma_fence_is_later(fence, check)) {
> +				/* put check to tail of fobj if it is deprecated */
> +				RCU_INIT_POINTER(fobj->shared[--k], check);
> +			} else {
> +				/* this is a wrong operation that add an eld fence */
> +				wrong_fence = true;
> +				RCU_INIT_POINTER(fobj->shared[j++], check);
> +			}
> +		} else {
> +			/* add fence to new slot */
>   			RCU_INIT_POINTER(fobj->shared[j++], check);
> +		}
>   	}
> +
>   	fobj->shared_count = j;
> -	RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
> -	fobj->shared_count++;
> +	if (!wrong_fence) {
> +		RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence);
> +		fobj->shared_count++;
> +	}
>   
>   done:
>   	preempt_disable();

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