Message-ID: <6e0ea7fa210be3b7592197a276d99a9986834f6e.camel@mailbox.org>
Date: Wed, 26 Nov 2025 14:42:16 +0100
From: Philipp Stanner <phasta@...lbox.org>
To: Boris Brezillon <boris.brezillon@...labora.com>
Cc: phasta@...nel.org, Daniel Almeida <daniel.almeida@...labora.com>, Alice
 Ryhl <aliceryhl@...gle.com>, Danilo Krummrich <dakr@...nel.org>, Christian
 König <ckoenig.leichtzumerken@...il.com>, Tvrtko Ursulin
 <tursulin@...ulin.net>, Alexandre Courbot <acourbot@...dia.com>, Dave
 Airlie <airlied@...hat.com>, Lyude Paul <lyude@...hat.com>, Peter Colberg
 <pcolberg@...hat.com>,  dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org,  rust-for-linux@...r.kernel.org
Subject: Re: [RFC WIP 2/3] rust: sync: Add dma_fence abstractions

On Tue, 2025-11-25 at 14:50 +0100, Boris Brezillon wrote:
> On Tue, 25 Nov 2025 13:20:03 +0100
> Philipp Stanner <phasta@...lbox.org> wrote:
> 

[…]

> 
> > 
> > Agreed, everything should signal according to the seqno.
> > 
> > The question we are facing with Rust (or rather, my design here) is
> > rather to what degree the infrastructure shall enforce this. As you
> > know, in C there isn't even a real "fence context", it's just an
> > abstract concept, represented by an integer maintained by the driver.
> > 
> > In Rust we can model it more exactly. For instance, we could enforce
> > ordered signaling by creating a function as the only way to signal
> > fences:
> > 
> > fctx.signal_all_fences_up_to_seqno(9042);
> > 
> > which then iterates over the fences, signaling all in order.
> 
> Up to you, but then it implies keeping a list of currently active
> fences attached to the context, plus some locks to protect this list.
> Another option would be to track the last signalled seqno at the
> context level, and complain if a fence with a lower seqno is signalled.

Well, JQ (or, maybe: the fence context) needs a list of fences to be
able to signal them.

At second glance, considering a more robust signalling API for DmaFence
as drafted out above, it might be the cleaner solution to have those
lists in the fctx and lock the lists there.

I'll investigate that.

> 
> > 
> > 
> > With some other APIs, such as jobqueue.submit_job(), which creates a
> > fence with the code above, it's trivial as long as the driver only
> > calls submit_job() with 1 thread.
> 
> As I was explaining to Daniel yesterday, you need some sort of
> serialization when you submit to the same context anyway. In Tyr,
> things will be serialized through the GPUVM resv, which guarantees that
> no more than one thread can allocate seqnos on a given context inside
> this critical section.
> 
> > 
> > If the driver came up with the idea of using >=2 threads firing on
> > submit_job(), then you by design have ordering issues, independently
> > from the fence context using this or that atomic operation or even full
> > locking.
> 
> In practice you don't, because submission to a given context are
> serialized one way or another (see above). Maybe what we should assume
> is that only one thread gets a mutable ref to this FenceCtx/JobQueue,
> and seqno allocation is something requiring mutability. The locking is
> something that's provided by the layer giving a mutable ref to this
> fence context.

I think that would then be achieved the Rust-way by having submit_job()
take a &mut self.

Anyways, enforcing that sounds like a great idea to me; that solves the
seqno issue.

> 
> > 
> > If the driver scrambles calls to submit_job() or new_fence(), then it
> > can certainly happen that done_fences are signaled by JQ out of order –
> > though I'm not sure how horrible that would actually be, considering
> > that this does not imply that anything gets executed before all
> > dependencies are fullfiled. AFAICS it's more "nice to have / would be
> > the cleanest possible design".
> 
> A fence context should really be bound to a GPU queue, and jobs on a
> GPU queue are expected to be executed in order. So long as the jobs are
> queued in order, they should be executed and signalled in order.
> 

Jobs will ALWAYS be run in order, and their corresponding fences be
signalled in order. Just the seqno might be out-of-order, if there were
no solution as drafted above by you and me.

But no argument here, we should enforce that as much as possible.

>  Of
> course, this implies some locking when you prepare/queue jobs because
> preparation and queuing are two distinct steps, and if you let 2
> threads do that concurrently, the queuing won't be ordered. 
> That's
> already stuff drivers have to deal with today, so I'm not sure we
> should make a difference for rust drivers, other than making this
> explicit by requiring mutable JobQueue/FenceCtx references in
> situations where we expect some higher-level locking to be in place.

Yes, the problem already exists. I brought to point up to answer the
question: to what degree do we want to enforce absolute correctness.

> 
> > 
> > I think I have a TODO in jobqueue where we could solve that by only
> > signaling pending done_fence's once all preceding fences have been
> > signaled.
> 
> I believe this is something we want to enforce, not fix. If signalling
> is done out of order, that's probably a driver bug, and it should be
> reported as such IMHO.

We can only enforcing that by designing DmaFence the way I said above:

the only way to signal a fence is via the fctx:

fctx.signal_all_up_to_seqno(9001);


If we're sure that there's really never a use case where someone
definitely needs a raw dma_fence_signal() equivalent where you can just
randomly signal whatever you want, I think that's the right thing to
do.


P.

Powered by blists - more mailing lists