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 <[email protected]> > --- > 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? Regards, Boqun > 6 files changed, 420 insertions(+) > create mode 100644 rust/helpers/dma_fence.c > create mode 100644 rust/kernel/sync/dma_fence.rs > [...]
