On Sun, Aug 11, 2019 at 10:15:23AM +0100, Chris Wilson wrote:
> Now that dma_fence_signal always takes the spinlock to flush the
> cb_list, simply take the spinlock and call dma_fence_signal_locked() to
> avoid code repetition.
> 
> Suggested-by: Christian König <christian.koe...@amd.com>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Christian König <christian.koe...@amd.com>

Hm, I think this largely defeats the point of having the lockless signal
enabling trickery in dma_fence. Maybe that part isn't needed by anyone,
but feels like a thing that needs a notch more thought. And if we need it,
maybe a bit more cleanup.

I guess with the addition of more fancy atomic BITs we could avoid this
too ... but no idea whether that's worth the effort.
-Daniel

> ---
>  drivers/dma-buf/dma-fence.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index ab4a456bba04..367b71084d34 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -122,26 +122,18 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
>   */
>  int dma_fence_signal_locked(struct dma_fence *fence)
>  {
> -     int ret = 0;
> -
> -     lockdep_assert_held(fence->lock);
> -
>       if (WARN_ON(!fence))
>               return -EINVAL;
>  
> -     if (!__dma_fence_signal(fence)) {
> -             ret = -EINVAL;
> +     lockdep_assert_held(fence->lock);
>  
> -             /*
> -              * we might have raced with the unlocked dma_fence_signal,
> -              * still run through all callbacks
> -              */
> -     } else {
> -             __dma_fence_signal__timestamp(fence, ktime_get());
> -     }
> +     if (!__dma_fence_signal(fence))
> +             return -EINVAL;
>  
> +     __dma_fence_signal__timestamp(fence, ktime_get());
>       __dma_fence_signal__notify(fence);
> -     return ret;
> +
> +     return 0;
>  }
>  EXPORT_SYMBOL(dma_fence_signal_locked);
>  
> @@ -161,19 +153,19 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
>  int dma_fence_signal(struct dma_fence *fence)
>  {
>       unsigned long flags;
> +     int ret;
>  
>       if (!fence)
>               return -EINVAL;
>  
> -     if (!__dma_fence_signal(fence))
> -             return -EINVAL;
> -
> -     __dma_fence_signal__timestamp(fence, ktime_get());
> +     if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +             return 0;
>  
>       spin_lock_irqsave(fence->lock, flags);
> -     __dma_fence_signal__notify(fence);
> +     ret = dma_fence_signal_locked(fence);
>       spin_unlock_irqrestore(fence->lock, flags);
> -     return 0;
> +
> +     return ret;
>  }
>  EXPORT_SYMBOL(dma_fence_signal);
>  
> -- 
> 2.23.0.rc1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to