Re: [PATCH 2/4] rust: rcu: add RcuBox type
From: Philipp Stanner
Date: Wed Jun 03 2026 - 05:38:18 EST
On Mon, 2026-06-01 at 06:41 -0700, Boqun Feng wrote:
> 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@xxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxx>
> > > > ---
> > >
> > > 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.
Yoah, probably better we're safer rather than hyper-consistent.
>
> >
> >
> > >
> > > 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!
Fine by me. Just process-wise: how should we do it?
I could include your patch on top of Alice's. Would be a bit more
consistent regarding the git-workflow if we'd squash the two patches,
but then you two would have to agree on authorship.
All is fine by me, but I wanted to ask instead of just do A or B.
P.
>
> 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@xxxxxxxxxx>
> > > ---
> > > 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(())
> > > + }
> > > }