On Mon Jun 1, 2026 at 10:46 AM CEST, Philipp Stanner wrote:
On Sat, 2026-05-30 at 17:16 +0200, Danilo Krummrich wrote:
(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?
You know that :-P
I don't, it's a serious question.
+ // 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.
Is that an official rule? If so, the linters should inform about it.
At first glance, I don't see any advantage to it and the disadvantage of greatly reducing readability.
The advantage is that it separates the safety justifications per unsafe call, which increases the chances of catching a bug, makes it easier for the reader to match requirements against justifications and potentially allows tooling to perform checks of the justification against the requirement.
For the specific case above, there's no documented requirements in the sense of Rust safety requirements of course, as they are all FFI calls, but dma_fence_signal_locked() for instance has the obvious requirement that it must only be called with the fence lock held and dma_fence_set_error() must only be called when the fence is actually signaled.
Besides that, since this pattern seems to occur at least twice, you could also consider adding a lock guard.