(Not a full review, but a few drive-by comments.)
On Sat May 30, 2026 at 4:35 PM CEST, Philipp Stanner wrote:
+#[allow(unused_unsafe)]
What is this needed for?
+impl<F: Send + Sync + DriverFenceAllowedData, C: Send + Sync> FenceCtx<F, C> {
<snip>
+impl<F: Send + Sync, C: Send + Sync> PinnedDrop for FenceCtx<F, C> {
- fn drop(self: Pin<&mut Self>) {
// SAFETY: `rcu_barrier()` is always safe to be called.unsafe { bindings::rcu_barrier() };
We should probably add a safe function for this.
+impl<T: FenceCb> FenceCbRegistration<T> {
- /// Register a callback on a fence.
- ///
- /// On success the callback is pinned in place and will fire when the fence
- /// signals. On `AlreadySignaled` the callback is returned to the caller so
- /// that owned resources can be reclaimed.
- pub fn new<'a>(fence: &'a Fence, callback: T) -> impl PinInit<Self, CallbackError<T>> + 'a
- where
T: 'a,- {
// Uses `pin_init_from_closure` instead of `try_pin_init!` so that on// `-ENOENT` (already signaled) the callback can be read back from the// partially-initialized slot and returned through the error.
Seems a bit odd that this needs pin_init_from_closure(). You can still use try_pin_init!() with &this in Self an a _: initializer at the end in the worst case. But the fence and callback fields should be fine to initialize "normally"?
//// SAFETY: `pin_init_from_closure` requires:// - On `Ok(())`: the slot is fully initialized and valid for `Drop`.// - On `Err(_)`: the slot is clean, i.e.: no partially-initialized fields// remain, and the slot can be deallocated without dropping.//// We uphold this as follows:// - On success: all three fields are initialized. Ok(()) is returned.// - On ENOENT (already signaled): `callback` and `fence` are read back// from the slot via `ptr::read`, leaving the slot clean. `cb` was// initialized by `dma_fence_add_callback` (it calls// `INIT_LIST_HEAD(&cb->node)` even on error), but `cb` is// `Opaque<dma_fence_cb>` which has no `Drop`, so not dropping it is// fine. The callback is returned through `AlreadySignaled(T)`.// - On other errors: same cleanup as ENOENT, error returned as// `Other(e)`.unsafe {pin_init_from_closure(move |slot: *mut Self| {let slot_callback = &raw mut (*slot).callback;let slot_fence = &raw mut (*slot).fence;let slot_cb = &raw mut (*slot).cb;// Write callback and fence first — must be visible before// dma_fence_add_callback makes the registration live.core::ptr::write(slot_callback, callback);core::ptr::write(slot_fence, ARef::from(fence));let ret = to_result(bindings::dma_fence_add_callback(fence.inner.get(),Opaque::cast_into(slot_cb),Some(Self::dma_fence_callback),));match ret {Ok(()) => Ok(()),Err(e) => {// Read back what we wrote to leave the slot clean.let cb_back = core::ptr::read(slot_callback);let _fence_back = core::ptr::read(slot_fence);
What's the purpose of _fence_back?
if e.to_errno() == ENOENT.to_errno() {Err(CallbackError::AlreadySignaled(cb_back))} else {Err(CallbackError::Other(e))}}}})}- }
- /// Signal the fence. This will invoke all registered callbacks.
- pub fn signal(self, res: Result) {
let fence = self.as_raw();let mut fence_flags: usize = 0;let flag_ptr = &raw mut fence_flags;// SAFETY: Once a `DriverFence` is initialized, the inner `fence` is// valid and initialized. It is valid until the refcount drops// to 0, which can earliest happen once the `DriverFence` has been dropped.unsafe {bindings::dma_fence_lock_irqsave(fence, flag_ptr);if !bindings::dma_fence_is_signaled_locked(fence) {if let Err(err) = res {bindings::dma_fence_set_error(fence, err.to_errno());}bindings::dma_fence_signal_locked(fence);}bindings::dma_fence_unlock_irqrestore(fence, flag_ptr);}
Please use a single unsafe block per unsafe function call, here and in a few other places.
- }
+}
+// SAFETY: Fences are literally designed to be shared between threads. +unsafe impl<F: Send + Sync, C: Send + Sync> Send for DriverFence<F, C> {}
+impl<F: Send + Sync, C: Send + Sync> Deref for DriverFence<F, C> {
- type Target = F;
- fn deref(&self) -> &Self::Target {
// SAFETY: Thanks to refcounting, `data` is always valid as long as `self` is.let data = unsafe { &*self.data.as_ptr() };&data.data- }
+}
+/// A borrowed [`DriverFence`]. All you can do with it is access your user data +/// and obtain a [`Fence`]. +pub struct DriverFenceBorrow<F: Send + Sync, C: Send + Sync> {
This misses the lifetime bound, which is the purpose of this struct.
- /// The actual content of the fence. Lives in a raw pointer so that its
- /// memory can be managed independently. Valid until both the [`DriverFence`]
- /// and all associated [`Fence`]s have disappeared.
- data: NonNull<DriverFenceData<F, C>>,
Why not use ManuallyDrop<DriverFence>? This way you would only need a Deref impl to &'a DriverFence.
This way you basically reimplement the DriverFence type just without the destructor.