Re: [PATCH 1/2] dma-fence: Rename dma_fence_is_signaled()

[Philipp Stanner <phasta@mailbox.org>]

On Wed, 2025-04-09 at 14:51 +0200, Philipp Stanner wrote:
> On Wed, 2025-04-09 at 14:39 +0200, Boris Brezillon wrote:
> > Hi Philipp,
> > 
> > On Wed,  9 Apr 2025 14:06:37 +0200
> > Philipp Stanner <phasta@kernel.org> wrote:
> > 
> > > dma_fence_is_signaled()'s name strongly reads as if this function
> > > were
> > > intended for checking whether a fence is already signaled. Also
> > > the
> > > boolean it returns hints at that.
> > > 
> > > The function's behavior, however, is more complex: it can check
> > > with a
> > > driver callback whether the hardware's sequence number indicates
> > > that
> > > the fence can already be treated as signaled, although the
> > > hardware's /
> > > driver's interrupt handler has not signaled it yet. If that's the
> > > case,
> > > the function also signals the fence.
> > > 
> > > (Presumably) this has caused a bug in Nouveau (unknown commit),
> > > where
> > > nouveau_fence_done() uses the function to check a fence, which
> > > causes a
> > > race.
> > > 
> > > Give the function a more obvious name.
> > 
> > This is just my personal view on this, but I find the new name just
> > as
> > confusing as the old one. It sounds like something is checked, but
> > it's
> > clear what, and then the fence is forcibly signaled like it would
> > be
> > if
> > you call drm_fence_signal(). Of course, this clarified by the doc,
> > but
> > given the goal was to make the function name clearly reflect what
> > it
> > does, I'm not convinced it's significantly better.
> > 
> > Maybe dma_fence_check_hw_state_and_propagate(), though it might be
> > too long of name. Oh well, feel free to ignore this comments if a
> > majority is fine with the new name.
> 
> Yoa, the name isn't perfect (the perfect name describing the whole
> behavior would be
> dma_fence_check_if_already_signaled_then_check_hardware_state_and_pro
> pa
> gate() ^^'
> 
> My intention here is to have the reader realize "watch out, the fence
> might get signaled here!", which is probably the most important event
> regarding fences, which can race, invoke the callbacks and so on.
> 
> For details readers will then check the documentation.
> 
> But I'm of course open to see if there's a majority for this or that
> name.

how about:

dma_fence_check_hw_and_signal() ?

P.

> 
> P.
> 
> 
> > 
> > Regards,
> > 
> > Boris
>