On Tue, Feb 10, 2026 at 10:15:25AM +0100, Boris Brezillon wrote:
> On Tue, 10 Feb 2026 08:38:00 +0000
> Alice Ryhl <[email protected]> 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 <[email protected]> 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 <[email protected]> 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.
Ok I understand what's going on now.
You talk about it as-if there are two signalling paths, one in the ioctl
and another in the workqueue. But that sounds like a workaround to make
it work with how dma_fence_begin_signalling() is implemented, and not
the true situation. (Added: looks like Christian confirms this.)
One way you can see this is by looking at what we require of the
workqueue. For all this to work, it's pretty important that we never
schedule anything on the workqueue that's not signalling safe, since
otherwise you could have a deadlock where the workqueue is executes some
random job calling kmalloc(GFP_KERNEL) and then blocks on our fence,
meaning that the VM_BIND job never gets scheduled since the workqueue
is never freed up. Deadlock.
And the correct way to model the above in lockdep is to have the DMA
fence lockdep key be nested *outside* the lockdep key of the workqueue.
Because there is a step in the signalling path where you wait for other
jobs to complete before the signalling path is able to continue.
And the meaning of such a lockdep dependency is exactly that the
critical region moves from one thread to another.
Perhaps we could have an API like this:
// Split the DriverDmaFence into two separate fence concepts:
struct PrivateFence { ... }
struct PublishedFence { ... }
/// The owner of this value must ensure that this fence is signalled.
struct MustBeSignalled<'fence> { ... }
/// Proof value indicating that the fence has either already been
/// signalled, or it will be. The lifetime ensures that you cannot mix
/// up the proof value.
struct WillBeSignalled<'fence> { ... }
/// Create a PublishedFence by entering a region that promises to signal
/// it.
///
/// The only way to return from the `region` closure it to construct a
/// WillBeSignalled value, and the only way to do that (see below) is to
/// signal it, or spawn a workqueue job that promises to signal it.
/// (Since the only way for that workqueue job to exit is to construct a
/// second WillBeSignalled value.)
fn dma_fence_begin_signalling(
fence: PrivateFence,
region: impl for<'f> FnOnce(MustBeSignalled<'f>) -> WillBeSignalled<'f>,
) -> PublishedFence {
let cookie = bindings::dma_fence_begin_signalling();
region(<create token here>);
bindings::dma_fence_end_signalling(cookie);
fence.i_promise_it_will_be_signalled();
}
impl MustBeSignalled<'_> {
/// Drivers generally should not use this one.
fn i_promise_it_will_be_signalled(self) -> WillBeSignalled { ... }
/// One way to ensure the fence has been signalled is to signal it.
fn signal_fence(self) -> WillBeSignalled {
self.fence.signal();
self.i_promise_it_will_be_signalled()
}
/// Another way to ensure the fence will be signalled is to spawn a
/// workqueue item that promises to signal it.
fn transfer_to_wq(
self,
wq: &Workqueue,
item: impl DmaFenceWorkItem,
) -> WillBeSignalled {
// briefly obtain the lock class of the wq to indicate to
// lockdep that the signalling path "blocks" on arbitrary jobs
// from this wq completing
bindings::lock_acquire(&wq->key);
bindings::lock_release(&wq->key);
// enqueue the job
wq.enqueue(item, wq);
// The signature of DmaFenceWorkItem::run() promises to arrange
// for it to be signalled.
self.i_promise_it_will_be_signalled()
}
}
trait DmaFenceWorkItem {
fn run<'f>(self, fence: MustBeSignalled<'f>) -> WillBeSignalled<'f>;
}
with this API, the ioctl can do this:
let published_fence = dma_fence_begin_signalling(|fence| {
fence.transfer_to_wq(my_wq, my_work_item)
});
somehow_publish_the_fence_to_userspace(published_fence);
And we're ensured that the fence is really signalled because the
signature of the work item closure is such that it can only return from
DmaFenceWorkItem::run() by signalling the fence (or spawning it to a
second workqueue, which in turn promises to signal it).
Of course, the signature only enforces that the fence is (or will be)
signalled if you return. One can still put an infinite loop inside
dma_fence_begin_signalling() and avoid signalling it, but there's
nothing we can do about that.
> > 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.
Ok.
Alice
