Message-ID: <3d3304ea-e229-4fca-bea2-5675d24684bc@amd.com>
Date: Sun, 28 Sep 2025 17:57:29 +0200
From: Christian König <christian.koenig@....com>
To: phasta@...nel.org, Boqun Feng <boqun.feng@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
 Gary Guo <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>,
 Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
 Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 Sumit Semwal <sumit.semwal@...aro.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Viresh Kumar <viresh.kumar@...aro.org>,
 Asahi Lina <lina+kernel@...hilina.net>,
 Daniel Almeida <daniel.almeida@...labora.com>,
 Tamir Duberstein <tamird@...il.com>,
 Wedson Almeida Filho <wedsonaf@...il.com>,
 FUJITA Tomonori <fujita.tomonori@...il.com>,
 Krishna Ketan Rai <prafulrai522@...il.com>, Lyude Paul <lyude@...hat.com>,
 Mitchell Levy <levymitchell0@...il.com>, linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org, llvm@...ts.linux.dev,
 dri-devel@...ts.freedesktop.org
Subject: Re: [RFC PATCH] rust: sync: Add dma_fence abstractions

On 28.09.25 17:26, Philipp Stanner wrote:
> On Sun, 2025-09-28 at 16:34 +0200, Christian König wrote:
>> On 27.09.25 11:01, Philipp Stanner wrote:
>>> On Fri, 2025-09-26 at 09:10 -0700, Boqun Feng wrote:
>>>> On Thu, Sep 18, 2025 at 02:30:59PM +0200, Philipp Stanner wrote:
>>>>> dma_fence is a synchronization mechanism which is needed by virtually
>>>>> all GPU drivers.
>>>>>
>>>>> A dma_fence offers many features, among which the most important ones
>>>>> are registering callbacks (for example to kick off a work item) which
>>>>> get executed once a fence gets signalled.
>>>>>
>>>>> dma_fence has a number of callbacks. Only the two most basic ones
>>>>> (get_driver_name(), get_timeline_name() are abstracted since they are
>>>>> enough to enable the basic functionality.
>>>>>
>>>>> Callbacks in Rust are registered by passing driver data which implements
>>>>> the Rust callback trait, whose function will be called by the C backend.
>>>>>
>>>>> dma_fence's are always refcounted, so implement AlwaysRefcounted for
>>>>> them. Once a reference drops to zero, the C backend calls a release
>>>>> function, where we implement drop_in_place() to conveniently marry that
>>>>> C-cleanup mechanism with Rust's ownership concepts.
>>>>>
>>>>> This patch provides basic functionality, but is still missing:
>>>>>   - An implementation of PinInit<T, Error> for all driver data.
>>>>>   - A clever implementation for working dma_fence_begin_signalling()
>>>>>     guards. See the corresponding TODO in the code.
>>>>>   - Additional useful helper functions such as dma_fence_is_signaled().
>>>>>     These _should_ be relatively trivial to implement, though.
>>>>>
>>>>> Signed-off-by: Philipp Stanner <phasta@...nel.org>
>>>>> ---
>>>>> So. ¡Hola!
>>>>>
>>>>> This is a highly WIP RFC. It's obviously at many places not yet
>>>>> conforming very well to Rust's standards.
>>>>>
>>>>> Nevertheless, it has progressed enough that I want to request comments
>>>>> from the community.
>>>>>
>>>>> There are a number of TODOs in the code to which I need input.
>>>>>
>>>>> Notably, it seems (half-)illegal to use a shared static reference to an
>>>>> Atomic, which I currently use for the dma_fence unit test / docstring
>>>>> test. I'm willing to rework that if someone suggests how.
>>>>> (Still, shouldn't changing a global Atomic always be legal? It can race,
>>>>> of course. But that's kind of the point of an atomic)
>>>>>
>>>>> What I want comments on the most is the design of the callbacks. I think
>>>>> it's a great opportunity to provide Rust drivers with rust-only
>>>>> callbacks, so that they don't have to bother about the C functions.
>>>>>
>>>>> dma_fence wise, only the most basic callbacks currently get implemented.
>>>>> For Nova, AFAICS, we don't need much more than signalling fences and
>>>>> registering callbacks.
>>>>>
>>>>>
>>>>> Another, solvable, issue I'm having is designing the
>>>>> dma_fence_begin_signallin() abstractions. There are TODOs about that in
>>>>> the code. That should ideally be robust and not racy. So we might want
>>>>> some sort of synchronized (locked?) way for using that abstraction.
>>>>>
>>>>>
>>>>> Regarding the manually created spinlock of mine: I so far never need
>>>>> that spinlock anywhere in Rust and wasn't sure what's then the best way
>>>>> to pass a "raw" spinlock to C.
>>>>>
>>>>>
>>>>> So much from my side. Hope to hear from you.
>>>>>
>>>>> (I've compiled and tested this with the unit test on the current -rc3)
>>>>>
>>>>> Philipp
>>>>> ---
>>>>>  rust/bindings/bindings_helper.h |   1 +
>>>>>  rust/helpers/dma_fence.c        |  23 ++
>>>>>  rust/helpers/helpers.c          |   1 +
>>>>>  rust/helpers/spinlock.c         |   5 +
>>>>>  rust/kernel/sync.rs             |   2 +
>>>>>  rust/kernel/sync/dma_fence.rs   | 388 ++++++++++++++++++++++++++++++++
>>>>
>>>> I missed this part, and I don't think kernel::sync is where dma_fence
>>>> should be, as kernel::sync is mostly for the basic synchronization
>>>> between threads/irqs. dma_fence is probably better to be grouped with
>>>> dma-buf and other dma related primitives. Maybe in kernel::dma? Like:
>>>>
>>>> rust/kernel/dma.rs
>>>> rust/kernel/dma/dma_buf.rs
>>>> rust/kernel/dma/dma_fence.rs
>>>>
>>>> Thoughts? Miguel, Greg, Danilo and Lyude, any idea or suggestion?
>>>
>>> @Christian König's opinion would be valuable, too.
>>
>> Oh yes, please don't mix dma_fences into SW synchronization, it's a HW synchronization primitive.
> 
> So do you agree that it should be regarded as a part of DMA_BUF?

That works for me.

> 
>>
>>> I'm not super convinced of that because dma_fence has not really much
>>> to do with DMA. They're not very different from completions and are a
>>> mechanism to synchronize consumers and producers.
>>
>> That we mixed up fences and completion events caused a lot of trouble in the past.
>>
>> It's astonishing how often Sima and I had to reject peoples ideas to use the dma_fence as SW sync.
>>
>>> Actually, before f54d1867005c3 they were just called "fence" and then
>>> renamed to "dma_fence" because someone wanted that name.
>>
>> That was also partially done to make sure that people understand that this is not for inter SW sync.
>>
>> On the other hand the rename didn't helped much :/
> 
> gpu_fence might have been appropriate?

No, it's also used for think like RDMA, V4L etc...

There was also a proposal for HW to HW synchronization at some point, but we abandoned that after we found how problematic HW to SW sync already is.

Regards,
Christian.

> 
> 
> Grüße
> P.
> 
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>>
>>> Anyways, I don't have strong objections and mostly care about having
>>> them available somewhere.
>>>
>>> P.
>>>
>>>>
>>>> Regards,
>>>> Boqun
>>>>
>>>>>  6 files changed, 420 insertions(+)
>>>>>  create mode 100644 rust/helpers/dma_fence.c
>>>>>  create mode 100644 rust/kernel/sync/dma_fence.rs
>>>>>
>>>> [...]
>>>
>>
> 

Powered by blists - more mailing lists