Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] Missing `#[inline]` annotation on the `synchronize_rcu()` abstraction. - [Low] Existing uses of `bindings::synchronize_rcu()` were not updated to use the new safe abstraction. --
commit 3f3ebd383ebd5385e032e8ee41ba8e2dc2c9f4e9 Author: Philipp Stanner <[email protected]> rust: sync: Add abstraction for synchronize_rcu() This commit adds a safe Rust abstraction for the synchronize_rcu() function to wait for a grace period. > diff --git a/rust/kernel/sync/rcu.rs b/rust/kernel/sync/rcu.rs > index eef34bf862596..d86d615467b1e 100644 > --- a/rust/kernel/sync/rcu.rs > +++ b/rust/kernel/sync/rcu.rs [ ... ] > @@ -56,3 +56,11 @@ pub fn rcu_barrier() { > // SAFETY: `rcu_barrier()` is always safe to be called. It just might > wait for a grace period. > unsafe { bindings::rcu_barrier() }; > } > + > +/// Wait for one RCU grace period. > +/// > +/// You typically do this to wait for everyone holding a [`Guard`]. > +pub fn synchronize_rcu() { [Severity: Low] This isn't a bug, but should this function have an #[inline] annotation? According to the Rust subsystem guidelines, functions in abstractions that are small or simply forward to a binding call should be annotated with #[inline]. Additionally, should the existing usages of the raw C binding be updated to use this new safe abstraction? For example, drop() in rust/kernel/sync/poll.rs currently does: unsafe { bindings::synchronize_rcu() }; And revoke_internal() in rust/kernel/revocable.rs: unsafe { bindings::synchronize_rcu() }; Updating these existing callers would help reduce unnecessary unsafe blocks across the codebase now that a safe wrapper is available. > + // SAFETY: `synchronize_rcu()` is always safe to be called. It just > waits for a grace period. > + unsafe { bindings::synchronize_rcu() }; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
