Re: [PATCH 1/2] dma-buf: Add __dma_fence_is_signaled()

[Philipp Stanner]

On Mon, 2025-12-01 at 15:46 +0000, Steven Price wrote:
> On 25/11/2025 10:44, Philipp Stanner wrote:
> > The dma_fence framework checks at many places whether the signaled bit
> > of a fence is already set. The code can be simplified and made more
> > readable by providing a helper function for that.
> > 
> > Add __dma_fence_is_signaled(), which _only_ checks whether a fence is
> > signaled. Use it internally.
> > 
> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
> > Signed-off-by: Philipp Stanner <phasta@xxxxxxxxxx>
> > ---
> >  drivers/dma-buf/dma-fence.c | 19 +++++++++----------
> >  include/linux/dma-fence.h   | 24 ++++++++++++++++++++++--
> >  2 files changed, 31 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 39e6f93dc310..3a48896ded62 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -372,8 +372,7 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> >  
> >  	lockdep_assert_held(fence->lock);
> >  
> > -	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > -				      &fence->flags)))
> > +	if (unlikely(__dma_fence_is_signaled(fence)))
> 
> Here you are replacing test_and_set_bit(), but the helper just does
> test_bit() - so this is changing the behaviour (and I suspect is wrong).
> 
> Thanks,
> Steve
> 
> 

Yes, that is a bug and has already been caught by various parties – but
it's good that you're paying attention :)

We're currently in this revision, where the bug is fixed:
https://lore.kernel.org/dri-devel/20251201105011.19386-2-phasta@xxxxxxxxxx/


P.