On Thu, Nov 27, 2025 at 11:16:26AM +0100, Philipp Stanner wrote:
> On Thu, 2025-11-27 at 11:01 +0100, Christian König wrote:
> > On 11/27/25 10:16, Philipp Stanner wrote:
> > > On Thu, 2025-11-27 at 09:11 +0100, Christian König wrote:
> > > > On 11/26/25 17:55, Matthew Brost wrote:
> > > > > On Wed, Nov 26, 2025 at 08:41:27AM -0800, Matthew Brost wrote:
> > > > > > On Wed, Nov 26, 2025 at 02:19:10PM +0100, Philipp Stanner wrote:
> > > > > > > The dma_fence framework checks at many places whether the 
> > > > > > > signaled flag
> > > > > > > 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_test_signaled_flag(), which only checks whether a 
> > > > > > > fence is
> > > > > > > signaled. Use it internally.
> > > > > > > 
> > > > > > > Suggested-by: Tvrtko Ursulin <[email protected]>
> > > > > > > Signed-off-by: Philipp Stanner <[email protected]>
> > > > > > 
> > > > > > This is a nice cleanp:
> > > > > > Reviewed-by: Matthew Brost <[email protected]>
> > > > > > 
> > > > > > > ---
> > > > > > >  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..25117a906846 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)))
> > > > > 
> > > > > I need to read a little better, I think this change isn't quite right.
> > > > > The original code is test and set, the updated code is test only 
> > > > > (i.e.,
> > > > > you are missing the set step). So maybe just leave this line as is.
> > > > 
> > > > Oh, good point! I've totally missed that as well.
> > > 
> > > Oh dear; I also just saw it when opening the mail client ._.
> > > 
> > > > 
> > > > But that means that this patch set hasn't even been smoke tested.
> > > 
> > > I've built it and did some basic testing with my Nouveau system. Any
> > > suggestions? Do you have a CI that one can trigger?

Intel's Xe list will trigger CI - you are authorized to trigger that
flow. Navigate to Intel's list patchworks [1], find your series and see
the results. It is noisy, so also certainly to say failure but I let you
know if anything is actually a problem.

Matt

[1] 
https://patchwork.freedesktop.org/project/intel-xe/series/?ordering=-last_updated

> > 
> > DMA-buf has CONFIG_DMABUF_SELFTESTS which should be able to catch things 
> > like that.
> > 
> > But even running Nouveau should have found this since basically no fence at 
> > would signal any more.
> 
> They will signal and execute their callbacks, but all is_signaled()
> checks are then broken.
> 
> Thx for the feedback, I'll be more careful with changes like those and
> test more extensively.
> 
> P.

Reply via email to