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 <[email protected]> > > > > > > > > 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 <[email protected]> > > > > --- > > > > > > 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 <[email protected]> > > > --- > > > 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(()) > > > + } > > > }
