Message-ID: <20260210101525.7fb85f25@fedora>
Date: Tue, 10 Feb 2026 10:15:25 +0100
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: "Christian König" <christian.koenig@....com>, Philipp
 Stanner <phasta@...lbox.org>, phasta@...nel.org, Danilo Krummrich
 <dakr@...nel.org>, David Airlie <airlied@...il.com>, Simona Vetter
 <simona@...ll.ch>, Gary Guo <gary@...yguo.net>, Benno Lossin
 <lossin@...nel.org>, Daniel Almeida <daniel.almeida@...labora.com>, Joel
 Fernandes <joelagnelf@...dia.com>, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, rust-for-linux@...r.kernel.org
Subject: Re: [RFC PATCH 2/4] rust: sync: Add dma_fence abstractions

On Tue, 10 Feb 2026 08:38:00 +0000
Alice Ryhl <aliceryhl@...gle.com> wrote:

> On Tue, Feb 10, 2026 at 09:16:34AM +0100, Christian König wrote:
> > On 2/9/26 15:58, Boris Brezillon wrote:  
> > > On Mon, 09 Feb 2026 09:19:46 +0100
> > > Philipp Stanner <phasta@...lbox.org> wrote:
> > >   
> > >> On Fri, 2026-02-06 at 11:23 +0100, Danilo Krummrich wrote:  
> > >>> On Thu Feb 5, 2026 at 9:57 AM CET, Boris Brezillon wrote:    
> > >>>> On Tue,  3 Feb 2026 09:14:01 +0100
> > >>>> Philipp Stanner <phasta@...nel.org> wrote:
> > >>>> Unfortunately, I don't know how to translate that in rust, but we
> > >>>> need a way to check if any path code path does a DmaFence.signal(),
> > >>>> go back to the entry point (for a WorkItem, that would be
> > >>>> WorkItem::run() for instance), and make it a DmaFenceSignallingPath.
> > >>>> Not only that, but we need to know all the deps that make it so
> > >>>> this path can be called (if I take the WorkItem example, that would
> > >>>> be the path that leads to the WorkItem being scheduled).    
> > >>>
> > >>> I think we need a guard object for this that is not Send, just like for any
> > >>> other lock.
> > >>>
> > >>> Internally, those markers rely on lockdep, i.e. they just acquire and release a
> > >>> "fake" lock.    
> > >>
> > >> The guard object would be created through fence.begin_signalling(), wouldn't it?  
> > > 
> > > It shouldn't be a (&self)-method, because at the start of a DMA
> > > signaling path, you don't necessarily know which fence you're going to
> > > signal (you might actually signal several of them).
> > >   
> > >> And when it drops you call dma_fence_end_signalling()?  
> > > 
> > > Yep, dma_fence_end_signalling() should be called when the guard is
> > > dropped.
> > >   
> > >>
> > >> How would that ensure that the driver actually marks the signalling region correctly?  
> > > 
> > > Nothing, and that's a problem we have in C: you have no way of telling
> > > which code section is going to be a DMA-signaling path. I can't think
> > > of any way to make that safer in rust, unfortunately. The best I can
> > > think of would be to
> > > 
> > > - Have a special DmaFenceSignalWorkItem (wrapper a WorkItem with extra
> > >   constraints) that's designed for DMA-fence signaling, and that takes
> > >   the DmaSignaling guard around the ::run() call.
> > > - We would then need to ensure that any code path scheduling this work
> > >   item is also in a DMA-signaling path by taking a ref to the
> > >   DmaSignalingGuard. This of course doesn't guarantee that the section
> > >   is wide enough to prevent any non-authorized operations in any path
> > >   leading to this WorkItem scheduling, but it would at least force the
> > >   caller to consider the problem.  
> > 
> > On the C side I have a patch set which does something very similar.
> > 
> > It's basically a WARN_ON_ONCE() which triggers as soon as you try to
> > signal a DMA fence from an IOCTL, or more specific process context.
> > 
> > Signaling a DMA fence from interrupt context, a work item or kernel
> > thread is still allowed, there is just the hole that you can schedule
> > a work item from process context as well.
> > 
> > The major problem with that patch set is that we have tons of very
> > hacky signaling paths in drivers already because we initially didn't
> > knew how much trouble getting this wrong causes.
> > 
> > I'm strongly in favor of getting this right for the rust side from the
> > beginning and enforcing strict rules for every code trying to
> > implement a DMA fence.  
> 
> Hmm. Could you say a bit more about what the rules are? I just re-read
> the comments in dma-fence.c, but I have some questions.
> 
> First, how does the signalling annotation work when the signalling path
> crosses thread boundaries?

It's not supposed to cross the thread boundary at all. The annotation
is per-thread, and it that sense, it matches the lock guard model
perfectly.

> For example, let's say I call an ioctl to
> perform an async VM_BIND, then the dma fence signalling critical path
> starts in the ioctl, but then it moves into a workqueue and finishes
> there, right?

It's a bit trickier. The fence signalling path usually doesn't exist in
the submitting ioctl until the submission becomes effective and the
emitted fences are exposed to the outside world. That is, when:
- syncobjs are updated to point to this new fence
- fencefd pointing to this new fence is returned
- fence is added to the dma_resvs inside the gem/dma_buf objects
- ... (there might be other cases I forgot about)

In the submission path, what's important is that no blocking allocation
is done between the moment the fence is exposed, and the moment it's
queued. In practice what happens is that the job this fence is bound to
is queued even before the fences are exposed, so if anything, what we
should ensure is the ordering, and having a guarantee that a job being
queued means it's going to be dequeued and executed soon enough.

The second DMA signaling path exists in the context of the
workqueue/item dequeuing a job from the JobQueue (or drm_sched) and
pushing it to the HW. Then there's the IRQ handler being called to
inform the GPU is done executing this job, which might in some cases
lead to another work item being queued for further processing from
which the dma_fence is signaled. In other cases, the dma_fence is
signaled directly from the IRQ handler. All of these contexts are
considered being part of the DMA-signaling path. But it's not like the
fence signaling annotation is passed around, because the cookies
returned by dma_fence_begin_signalling() are only valid in a single
thread context, IIRC.

> 
> Second, it looks like we have the same challenge as with irq locks where
> you must properly nest dma_fence_begin_signalling() regions, and can't
> e.g. do this:
> 
> c1 = dma_fence_begin_signalling()
> c2 = dma_fence_begin_signalling()
> dma_fence_end_signalling(c1)
> dma_fence_end_signalling(c2)

I think that's the case yes, you have to end in reverse being order.

Powered by blists - more mailing lists