Re: dma_fence: force users to take the lock manually

[Boris Brezillon <boris.brezillon@collabora.com>]

Hi,

On Thu, 5 Mar 2026 14:59:02 +0100
Christian König <christian.koenig@amd.com> wrote:

> On 3/5/26 14:54, Philipp Stanner wrote:
> > Yo Christian,
> > 
> > a while ago we were discussing this problem
> > 
> > dma_fence_set_error(f, -ECANCELED);

If you really have two concurrent threads setting the error, this part
is racy, though I can't think of any situation where concurrent
signaling of a set of fences wouldn't be protected by another external
lock.

> > dma_fence_signal(f); // racy!

This is not racy because dma_fence_signal() takes/releases the
lock internally. Besides, calling dma_fence_signal() on an already
signaled fence is considered an invalid pattern if I trust the -EINVAL
returned here[1].

> > 
> > 
> > I think you mentioned that you are considering to redesign the
> > dma_fence API so that users have to take the lock themselves to touch
> > the fence:
> > 
> > dma_fence_lock(f);
> > dma_fence_set_error(f, -ECANCELED);
> > dma_fence_signal(f);

I guess you mean dma_fence_signal_locked().

> > dme_fence_unlock(f);
> > 
> > 
> > Is that still up to date? Is there work in progress about that?  
> 
> It's on my "maybe if I ever have time for that" list, but yeah I think it would be really nice to have and a great cleanup.
> 
> We have a bunch of different functions which provide both a _locked() and _unlocked() variant just because callers where to lazy to lock the fence.
> 
> Especially the dma_fence_signal function is overloaded 4 (!) times with locked/unlocked and with and without timestamp functions.
> 
> > I discovered that I might need / want that for the Rust abstractions.  
> 
> Well my educated guess is for Rust you only want the locked function and never allow callers to be lazy.

I don't think we have an immediate need for manual locking in rust
drivers (no signaling done under an already dma_fence-locked section
that I can think of), especially after the inline_lock you've
introduced. Now, I don't think it matters if only the _locked() variant
is exposed and the rust code is expected to acquire/release the lock
manually, all I'm saying is that we probably don't need that in drivers
(might be different if we start implementing fence containers like
arrays and chain in rust, but I don't think we have an immediate need
for that).

Regards,

Boris

[1]https://elixir.bootlin.com/linux/v6.19.3/source/drivers/dma-buf/dma-fence.c#L375