Re: [RFC PATCH v2 0/3] Solve dma_fence's problems by locking

[Philipp Stanner]

On Tue, 2026-10-20 at 14:02 +0200, Christian König wrote:
> On 10/20/26 12:00, Philipp Stanner wrote:
> > In our Rust code bases, I'm currently experiencing what we also
> > experience in C at many places: that the dma_fence contract's RCU
> > requirements make the API more fragile and put an unnecessary burden on
> > the API user. Most notably, in C the API behaving correctly now depends
> > on the API *user* waiting before free()s, waiting before unloads – all
> > because dma_fence is not properly synchronized.
> > 
> > The patches commit messages give more details about where I want to go
> > with this particular RFC.
> > 
> > My rough "vision" for dma_fence, where I think it should develop to, can
> > be described like this:
> > 
> > 1. All users should use inline-lock.
> > 2. The fence-signaled state should be our decoupling point.
> > 3. The fence state must be consistently locked.
> 
> 
> > 4. This means all callbacks are only invoked for unsignaled fences, and
> >    the fence cannot signal while a callback runs.
> 
> First of all that prevents concurrent execution of the callbacks
> which for a lot of use cases is a must have.

Currently only set_deadline() can execute in parallel.

IIUC in our past discussions, your wish has been that the driver can
take its own locks with free locking-order in the callbacks.

Thus, the callback is invoked under lock in any case, just a few
instructions earlier or later. Thus, no callback ever truly executes
concurrently.

> 
> Then as I pointed out before the second problem is that this again
> makes the existing lock inversion between the signaling by polling
> and signaling by interrupts even worse than it already is.

We asked for examples in our last discussions, and the only one ever
presented was the list-access requiring the fctx-lock in
nouveau_fence_no_signaling(). Nouveau is often not a good example.

Can you provide examples in the kernel where deadlock would occur?

Note that I pointed out that all callbacks are already invoked with the
lock taken, which proofs that it's doable. Future users with inline-
lock could employ various techniques for dealing with the lock-order.

Only exception currently is set_deadline(). The only user, MSM,
acquires the lock manually and then nevertheless triggers work on a
kthread, examplifying just one technique.

> 
> What we could use is an SRCU, but a spinlock or any kind of lock is
> clearly the wrong approach.
> 
> So as far as I can see that is a big no-go and we should really just
> use RCU for everything and remove leaking the fence look into the
> callbacks like I outlined before.

Look at patch 3. We might want to add some legacy support for shared-
lock, but it is evident how much more simple the code gets.

Also:

> > 
> > Point №6 means that this:
> > 
> > dma_fence_lock_irqsave(…);

I even forgot that you also need a

if (!dma_fence_is_signaled_locked(…))

here

> > dma_fence_set_error(f, err);
> > dma_fence_signal_locked(f);
> > dma_fence_unlock_irqrestore(…);
> > synchronize_rcu();
> > kfree(driver_foo);
> > 
> > should simply become this:
> > 
> > dma_fence_signal(f, err);
> > kfree(driver_foo);


:)