Re: [PATCH 2/4] rust: rcu: add RcuBox type

[Boqun Feng <boqun@kernel.org>]

On Mon, Jun 01, 2026 at 09:56:23AM +0200, Philipp Stanner wrote:
> On Sat, 2026-05-30 at 08:08 -0700, Boqun Feng wrote:
> > On Sat, May 30, 2026 at 04:35:10PM +0200, Philipp Stanner wrote:
> > > From: Alice Ryhl <aliceryhl@google.com>
> > > 
> > > This adds an RcuBox container, which is like KBox except that the value
> > > is freed with kfree_rcu.
> > > 
> > > To allow containers to rely on the rcu properties of RcuBox, an
> > > extension of ForeignOwnable is added.
> > > 
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> > 
> > I have the following on top of Alice's patch. @Alice, @Danilo, thoughts?
> > 
> > Then we can have:
> > 
> > type RcuKBox<T> = RcuBox<T, Kmalloc>;
> > type RcuVBox<T> = RcuBox<T, Vmalloc>;
> 
> No objections by me.
> 
> I just think we have to decide how the treat the namespaces, though.
> Probably Alice wrote it like that so that it's very apparent that this
> is not a normal box. It still breaks the naming convention in my
> opinion.
> 
> rcu::Box vs rcu::RcuBox
> 
> With all other subsystems, naming like that seems not allowed.
> 
> dma::Fence vs dma::DmaFence
> 
> 
> I probably would allow the user to decide whether he wants to just use
> it as `rcu::Box` in all his code.
> 
> But no hard feelings.
> 

For this I think that rcu::RcuBox is a bit different than dma::Fence,
because Box has its widely-accepted meaning through all Rust code,
while `Fence` doesn't. Hence my current thought is rcu::RcuBox and
dma::Fence. My personal preference is using namespace as much as we
could until there might be some misleading.

> 
> 
> > 
> > and Philipp can use the `RcuKBox` in this patchset. We also need to impl
> > InPlaceInit for RcuBox, but that can be added later.
> 
> So shall we merge my series with Alice's patch, and later we add your
> patch and other features, or would you prefer to have the additional
> boxes from your patch from the get-go?
> 

I would like to have it from the get-go mainly because of RcuBox vs
RcuKBox naming. Thank you!

Regards,
Boqun

> 
> P.
> 
> > 
> > Regards,
> > Boqun
> > 
> > ------------->8
> > Subject: [PATCH] rust: rcu: Make RcuBox generic over Allocator
> > 
> > To support RCU-protected vmalloc allocation, we need to make `RcuBox`
> > generic over `Allocator`. Currently this works since all `Allocator`s
> > are either kmalloc() or vmalloc(), and kvfree_call_rcu() works with both
> > allocations.
> > 
> > While we are at it, add some basic test cases.
> > 
> > Signed-off-by: Boqun Feng <boqun@kernel.org>
> > ---
> >  rust/kernel/sync/rcu/rcu_box.rs | 96 +++++++++++++++++++++++----------
> >  1 file changed, 67 insertions(+), 29 deletions(-)
> > 
> > diff --git a/rust/kernel/sync/rcu/rcu_box.rs b/rust/kernel/sync/rcu/rcu_box.rs
> > index 2508fdb609ec..5c344d82c0d9 100644
> > --- a/rust/kernel/sync/rcu/rcu_box.rs
> > +++ b/rust/kernel/sync/rcu/rcu_box.rs
> > @@ -4,47 +4,59 @@
> >  
> >  //! Provides the `RcuBox` type for Rust allocations that live for a grace period.
> >  
> > -use core::{ops::Deref, ptr::NonNull};
> > +use core::{
> > +    marker::PhantomData,
> > +    ops::Deref,
> > +    ptr::NonNull, //
> > +};
> >  
> >  use kernel::{
> > -    alloc::{self, AllocError},
> > +    alloc::{
> > +        self,
> > +        AllocError,
> > +        Allocator, //
> > +    },
> >      bindings,
> >      ffi::c_void,
> >      prelude::*,
> > -    sync::rcu::{ForeignOwnableRcu, Guard},
> >      types::ForeignOwnable,
> >  };
> >  
> > +use super::{
> > +    ForeignOwnableRcu,
> > +    Guard, //
> > +};
> > +
> >  /// A box that is freed with rcu.
> >  ///
> >  /// The value must be `Send`, as rcu may drop it on another thread.
> >  ///
> >  /// # Invariants
> >  ///
> > -/// * The pointer is valid and references a pinned `RcuBoxInner<T>` allocated with `kmalloc`.
> > +/// * The pointer is valid and references a pinned `RcuBoxInner<T>` allocated with `A`.
> >  /// * This `RcuBox` holds exclusive permissions to rcu free the allocation.
> > -pub struct RcuBox<T: Send>(NonNull<RcuBoxInner<T>>);
> > +pub struct RcuBox<T: Send, A: Allocator>(NonNull<RcuBoxInner<T>>, PhantomData<A>);
> >  
> >  struct RcuBoxInner<T> {
> >      value: T,
> >      rcu_head: bindings::callback_head,
> >  }
> >  
> > -// Note that `T: Sync` is required since when moving an `RcuBox<T>`, the previous owner may still
> > -// access `&T` for one grace period.
> > +// Note that `T: Sync` is required since when moving an `RcuBox<T, A>`, the previous owner may
> > +// still access `&T` for one grace period.
> >  //
> > -// SAFETY: Ownership of the `RcuBox<T>` allows for `&T` and dropping the `T`, so `T: Send + Sync`
> > -// implies `RcuBox<T>: Send`.
> > -unsafe impl<T: Send + Sync> Send for RcuBox<T> {}
> > +// SAFETY: Ownership of the `RcuBox<T, A>` allows for `&T` and dropping the `T`, so `T: Send +
> > +// Sync` implies `RcuBox<T, A>: Send`.
> > +unsafe impl<T: Send + Sync, A: Allocator> Send for RcuBox<T, A> {}
> >  
> > -// SAFETY: `&RcuBox<T>` allows for no operations other than those permitted by `&T`, so `T: Sync`
> > -// implies `RcuBox<T>: Sync`.
> > -unsafe impl<T: Send + Sync> Sync for RcuBox<T> {}
> > +// SAFETY: `&RcuBox<T, A>` allows for no operations other than those permitted by `&T`, so `T:
> > +// Sync` implies `RcuBox<T, A>: Sync`.
> > +unsafe impl<T: Send + Sync, A: Allocator> Sync for RcuBox<T, A> {}
> >  
> > -impl<T: Send> RcuBox<T> {
> > +impl<T: Send, A: Allocator> RcuBox<T, A> {
> >      /// Create a new `RcuBox`.
> >      pub fn new(x: T, flags: alloc::Flags) -> Result<Self, AllocError> {
> > -        let b = KBox::new(
> > +        let b = Box::<_, A>::new(
> >              RcuBoxInner {
> >                  value: x,
> >                  rcu_head: Default::default(),
> > @@ -53,9 +65,9 @@ pub fn new(x: T, flags: alloc::Flags) -> Result<Self, AllocError> {
> >          )?;
> >  
> >          // INVARIANT:
> > -        // * The pointer contains a valid `RcuBoxInner` allocated with `kmalloc`.
> > +        // * The pointer contains a valid `RcuBoxInner` allocated with `A`.
> >          // * We just allocated it, so we own free permissions.
> > -        Ok(RcuBox(NonNull::from(KBox::leak(b))))
> > +        Ok(RcuBox(NonNull::from(Box::leak(b)), PhantomData))
> >      }
> >  
> >      /// Access the value for a grace period.
> > @@ -66,7 +78,7 @@ pub fn with_rcu<'rcu>(&self, _read_guard: &'rcu Guard) -> &'rcu T {
> >      }
> >  }
> >  
> > -impl<T: Send> Deref for RcuBox<T> {
> > +impl<T: Send, A: Allocator> Deref for RcuBox<T, A> {
> >      type Target = T;
> >      fn deref(&self) -> &T {
> >          // SAFETY: While the `RcuBox<T>` exists, the value remains valid.
> > @@ -75,10 +87,10 @@ fn deref(&self) -> &T {
> >  }
> >  
> >  // SAFETY:
> > -// * The `RcuBoxInner<T>` was allocated with `kmalloc`.
> > +// * The `RcuBoxInner<T>` was allocated with `A`.
> >  // * `NonNull::as_ptr` returns a non-null pointer.
> > -unsafe impl<T: Send + 'static> ForeignOwnable for RcuBox<T> {
> > -    const FOREIGN_ALIGN: usize = <KBox<RcuBoxInner<T>> as ForeignOwnable>::FOREIGN_ALIGN;
> > +unsafe impl<T: Send + 'static, A: Allocator> ForeignOwnable for RcuBox<T, A> {
> > +    const FOREIGN_ALIGN: usize = <Box<RcuBoxInner<T>, A> as ForeignOwnable>::FOREIGN_ALIGN;
> >  
> >      type Borrowed<'a> = &'a T;
> >      type BorrowedMut<'a> = &'a T;
> > @@ -88,9 +100,9 @@ fn into_foreign(self) -> *mut c_void {
> >      }
> >  
> >      unsafe fn from_foreign(ptr: *mut c_void) -> Self {
> > -        // INVARIANT: Pointer returned by `into_foreign` carries same invariants as `RcuBox<T>`.
> > +        // INVARIANT: Pointer returned by `into_foreign, A` carries same invariants as `RcuBox<T>`.
> >          // SAFETY: `into_foreign` never returns a null pointer.
> > -        Self(unsafe { NonNull::new_unchecked(ptr.cast()) })
> > +        Self(unsafe { NonNull::new_unchecked(ptr.cast()) }, PhantomData)
> >      }
> >  
> >      unsafe fn borrow<'a>(ptr: *mut c_void) -> &'a T {
> > @@ -104,7 +116,7 @@ unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> &'a T {
> >      }
> >  }
> >  
> > -impl<T: Send + 'static> ForeignOwnableRcu for RcuBox<T> {
> > +impl<T: Send + 'static, A: Allocator> ForeignOwnableRcu for RcuBox<T, A> {
> >      type RcuBorrowed<'a> = &'a T;
> >  
> >      unsafe fn rcu_borrow<'a>(ptr: *mut c_void) -> &'a T {
> > @@ -114,7 +126,7 @@ unsafe fn rcu_borrow<'a>(ptr: *mut c_void) -> &'a T {
> >      }
> >  }
> >  
> > -impl<T: Send> Drop for RcuBox<T> {
> > +impl<T: Send, A: Allocator> Drop for RcuBox<T, A> {
> >      fn drop(&mut self) {
> >          // SAFETY: The `rcu_head` field is in-bounds of a valid allocation.
> >          let rcu_head = unsafe { &raw mut (*self.0.as_ptr()).rcu_head };
> > @@ -122,9 +134,11 @@ fn drop(&mut self) {
> >              // SAFETY: `rcu_head` is the `rcu_head` field of `RcuBoxInner<T>`. All users will be
> >              // gone in an rcu grace period. This is the destructor, so we may pass ownership of the
> >              // allocation.
> > -            unsafe { bindings::call_rcu(rcu_head, Some(drop_rcu_box::<T>)) };
> > +            unsafe { bindings::call_rcu(rcu_head, Some(drop_rcu_box::<T, A>)) };
> >          } else {
> >              // SAFETY: All users will be gone in an rcu grace period.
> > +            // TODO: We are luckily since `kvfree_call_rcu()` works on both kmalloc and vmalloc,
> > +            // maybe a new `Allocator` method is needed.
> >              unsafe { bindings::kvfree_call_rcu(rcu_head, self.0.as_ptr().cast()) };
> >          }
> >      }
> > @@ -135,11 +149,35 @@ fn drop(&mut self) {
> >  /// # Safety
> >  ///
> >  /// `head` references the `rcu_head` field of an `RcuBoxInner<T>` that has no references to it.
> > -/// Ownership of the `KBox<RcuBoxInner<T>>` must be passed.
> > -unsafe extern "C" fn drop_rcu_box<T>(head: *mut bindings::callback_head) {
> > +/// Ownership of the `Box<RcuBoxInner<T>, A>` must be passed.
> > +unsafe extern "C" fn drop_rcu_box<T, A: Allocator>(head: *mut bindings::callback_head) {
> >      // SAFETY: Caller provides a pointer to the `rcu_head` field of a `RcuBoxInner<T>`.
> >      let box_inner = unsafe { crate::container_of!(head, RcuBoxInner<T>, rcu_head) };
> >  
> >      // SAFETY: Caller ensures exclusive access and passed ownership.
> > -    drop(unsafe { KBox::from_raw(box_inner) });
> > +    drop(unsafe { Box::<_, A>::from_raw(box_inner) });
> > +}
> > +
> > +#[kunit_tests(rust_rcu_box)]
> > +mod tests {
> > +    use super::*;
> > +
> > +    #[test]
> > +    fn rcu_box_basic() -> Result {
> > +        let rb = RcuBox::<_, alloc::allocator::Kmalloc>::new(42i32, alloc::flags::GFP_KERNEL)?;
> > +
> > +        assert_eq!(*rb, 42);
> > +        assert_eq!(*rb.with_rcu(&Guard::new()), 42);
> > +
> > +        drop(rb);
> > +
> > +        let rb = RcuBox::<_, alloc::allocator::Vmalloc>::new(42i32, alloc::flags::GFP_KERNEL)?;
> > +
> > +        assert_eq!(*rb, 42);
> > +        assert_eq!(*rb.with_rcu(&Guard::new()), 42);
> > +
> > +        drop(rb);
> > +
> > +        Ok(())
> > +    }
> >  }