Re: [PATCH 3/4] rust: Add dma_fence abstractions

[Philipp Stanner <phasta@mailbox.org>]

On Mon, 2026-06-01 at 12:39 +0000, Alice Ryhl wrote:
> On Mon, Jun 01, 2026 at 02:26:17PM +0200, Philipp Stanner wrote:
> > On Mon, 2026-06-01 at 10:36 +0000, Alice Ryhl wrote:
> > > On Sat, May 30, 2026 at 04:35:11PM +0200, Philipp Stanner wrote:
> > > > +        unsafe {
> > > > +            bindings::dma_fence_remove_callback(self.fence.as_raw(), self.cb.get());
> > > > +        }
> > > 
> > > Formatting nit: Usually the ; goes outside the unsafe block.
> > 
> > I could have sworn that it was rustfmt who did that? Maybe because the
> > ; was inside to begin with.
> 
> Indeed, rustfmt will not change whether the ; is inside or outside the
> unsafe block.
> 
> > > > +/// A trait to enforce that all data in a [`DriverFence`] either does not need
> > > > +/// drop, or lives in a [`RcuBox`].
> > > > +pub trait DriverFenceAllowedData: private::Sealed {}
> > > > +
> > > > +mod private {
> > > > +    pub trait Sealed {}
> > > > +}
> > > > +
> > > > +impl<F: Copy> DriverFenceAllowedData for F {}
> > > > +impl<F: Send> DriverFenceAllowedData for RcuBox<F> {}
> > > > +
> > > > +impl<F: Copy> private::Sealed for F {}
> > > > +impl<F: Send> private::Sealed for RcuBox<F> {}
> > > 
> > > Why sealed? Just make the trait unsafe and require the things you
> > > require from the user.
> > 
> > This is far better. We definitely only allow the user to pass A or B,
> > and only then it compiles.
> 
> What if I have another type that I want to use here? For example, maybe
> I have a struct containing a copy field and an RcuBox. Or maybe I have
> an ARef<_> of some C type that uses rcu for cleanup. Then I must edit
> this file to add support for it?
> 
> > The unsafe implementation could be messed up.
> > 
> > I thought that's what Sealed is for. Or isn't it?
> 
> Sealed is for making 100% sure that downstream crates/drivers cannot
> provide their own implementations. But I don't see why you need that.
> All you require is that the value remains valid for one grace period
> after cleanup begins. As long as the type satisfies that, you are happy.
> An unsafe trait can require that sort of requirement from the user.
> 
> I think what you want is expressed well by `RcuFreeSafe` from this
> thread:
> https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/Consolidate.20.60PollCondVarBox.60.20into.20.60Rcu.2ABox.60/near/598726724
> 

I guess this is a question of design principles. If you demand an
RcuBox, you have a guarantee that it's safe.

If you demand an unsafe trait, you open the possibility for people
messing up.

Due to the unsafe-contract you'd have moved the responsibility for the
soundness to the driver.

I would not want to block your suggestion, but I am not sure whether
that's really the better design idea.

> > > 

P.