[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <89c04c8b3ca0fadf73452ca20ffd61cb106d762a.camel@mailbox.org>
Date: Thu, 27 Nov 2025 11:16:26 +0100
From: Philipp Stanner <phasta@...lbox.org>
To: Christian König <christian.koenig@....com>,
phasta@...nel.org, Matthew Brost <matthew.brost@...el.com>
Cc: Sumit Semwal <sumit.semwal@...aro.org>, Gustavo Padovan
<gustavo@...ovan.org>, Felix Kuehling <Felix.Kuehling@....com>, Alex
Deucher <alexander.deucher@....com>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi
<rodrigo.vivi@...el.com>, Tvrtko Ursulin <tursulin@...ulin.net>, Huang Rui
<ray.huang@....com>, Matthew Auld <matthew.auld@...el.com>, Maarten
Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, Lucas De
Marchi <lucas.demarchi@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, linux-media@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
linux-kernel@...r.kernel.org, amd-gfx@...ts.freedesktop.org,
intel-gfx@...ts.freedesktop.org, intel-xe@...ts.freedesktop.org,
rust-for-linux@...r.kernel.org, Tvrtko Ursulin <tvrtko.ursulin@...lia.com>
Subject: Re: [PATCH 1/6] dma-buf/dma-fence: Add
dma_fence_test_signaled_flag()
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 <tvrtko.ursulin@...lia.com>
> > > > > > Signed-off-by: Philipp Stanner <phasta@...nel.org>
> > > > >
> > > > > This is a nice cleanp:
> > > > > Reviewed-by: Matthew Brost <matthew.brost@...el.com>
> > > > >
> > > > > > ---
> > > > > > 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?
>
> 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.
Powered by blists - more mailing lists