From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D860E37F8C9; Mon, 1 Jun 2026 13:41:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780321279; cv=none; b=CzJPXiLa8PxMQtlbk28zFS5jHfhBiIz96v69oS6MtGYmlUYfBcRvTVrQyH8ez7IGiSt04kigWcdDNFwnlBRpD0xbQwM54vxuhp4IvsR9iv2IJNZvvf8CDfXe5//pkHNv4pbwELfVXkgmUWWqutuEFUjWgwD7PlENBzza75DcuUk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780321279; c=relaxed/simple; bh=5/o6NQqtTVwjIlNjXVO4Xt/qjxB+FqG1olLOJTKzZw0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=vB31v3uWfrX4ZbmsWAUk/rwVePBb/oET+NqF+jKA08QVBdkJN8CyUZ0AZ0o34BoxamNfxmI9yzaNMNNmqzrVbxsSd4B0660rTtI16CbD4VtsCSPZHlXwOVmqYtdiMrCqKrSBvaSLemNge+dE0wKOl+cZMjO5/ZZnmDb4ypGe6W4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=ZD/1qwUQ; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="ZD/1qwUQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 90D0F1F0089A; Mon, 1 Jun 2026 13:41:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780321277; bh=Cy/Wx8iHE+k3tnhZgJfl2Qf8HrXMI7saH/g6ishR5cw=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=ZD/1qwUQecSnKfTT66yrGOSNo8WOMEXQQQ9R3IbvQfQhjd651FrPeqeL9Sn+VJ3ax BhVO0KmIo1FTz6s2UpPkb2oYSpR8G16KywEdVOf7XlNJOOsprbclirHrtIeOIh8XlD 9FTViaKyzZLTta5KwN4fdKsMj0UPdzR8r5vZi0M8mvA0GZOInaBEKLSfvnNsl/O878 L5K5x/D/fK50wEW7ei7xUvYiAUp1WNnHvvHfIHKWJoetIptRK+EuazavG0TZg0EMQC bIJwzQ2KWq0h00FyAkNS2XmSfHclYfo2Ew+WkNKhCqg/GxdHYlj+Llt4JJHfAzw1tI SibGx2RMqrrKQ== Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfauth.phl.internal (Postfix) with ESMTP id BA7BFF4008A; Mon, 1 Jun 2026 09:41:15 -0400 (EDT) Received: from phl-frontend-04 ([10.202.2.163]) by phl-compute-06.internal (MEProxy); Mon, 01 Jun 2026 09:41:15 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTEQx9WLyMJiwmRV/+uLnLi6x4tXtGJyKzl7rVMSP6cJN4a3LJQXrGvoa5s5QegsPj cBG9+VIEViyPnp9vQVotsHJtdGjqtL8FjYMeqtYA04kgCDQXIM0kLMdz+KfFxGn/jm2kHV UPnXdzWfZjVLyVBPSvBY72/HFGneZCGM2MkwQhN5CaUI8t2NuPJFNw0c4FeZxthAmhyRiv NN0aTXWc0JrQXjEClv0gxSkxg9jvRYyZeqON0CnaaBjd6BvKn/mQbMrHfoUJUig1DYbcUD XxkCwq+IvtsoGKWSaiESv2T4BxPHNSjkq4ZmvgZ7/OVZkTpvSG3l8Q7eAtkEZapasA3Rco tQ7QJ65HWT9eVssyAmvMK2peoVFi3X7P2LXRootOWWQ45mZ4ro3AWW/OvA4g7MF1Sf/jgP mBGJ2O81xU6Q4lsGyjn0nIMJN+CG9xgVjzeGgSgZQYQJM7XG7oQ6amNwo+5eGdcWoKNiYE Bi/DK+s1w/v6o4mdBq9vPdYSX+rsc5L50f3RnkZhV9On5d6fsJOXwS4FtcqG9bZGRmyUQA FJjm/HtDkgcrhhDS1LcKFDdQovY02T4TIBh1TpJvMCvl+aisTIozYS6Oz7JdstEJ14gIpx EAExkYvv/PzMdfThegFq1qzG2GVqxy80uwMbuMemZqJzsIOZnaJFjK0PpOOQ X-ME-Proxy: Feedback-ID: i8dbe485b:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 1 Jun 2026 09:41:15 -0400 (EDT) Date: Mon, 1 Jun 2026 06:41:13 -0700 From: Boqun Feng To: phasta@kernel.org Cc: Miguel Ojeda , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross , Danilo Krummrich , Sumit Semwal , Christian =?iso-8859-1?Q?K=F6nig?= , "Paul E. McKenney" , Frederic Weisbecker , Neeraj Upadhyay , Joel Fernandes , Josh Triplett , Uladzislau Rezki , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Zqiang , Daniel Almeida , Greg Kroah-Hartman , Igor Korotin , Lorenzo Stoakes , Alexandre Courbot , FUJITA Tomonori , Krishna Ketan Rai , Shankari Anand , manos@pitsidianak.is, Boris Brezillon , linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, rcu@vger.kernel.org Subject: Re: [PATCH 2/4] rust: rcu: add RcuBox type Message-ID: References: <20260530143541.229628-2-phasta@kernel.org> <20260530143541.229628-4-phasta@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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 > > > > > > 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 > > > --- > > > > I have the following on top of Alice's patch. @Alice, @Danilo, thoughts? > > > > Then we can have: > > > > type RcuKBox = RcuBox; > > type RcuVBox = RcuBox; > > 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 > > --- > >  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` allocated with `kmalloc`. > > +/// * The pointer is valid and references a pinned `RcuBoxInner` allocated with `A`. > >  /// * This `RcuBox` holds exclusive permissions to rcu free the allocation. > > -pub struct RcuBox(NonNull>); > > +pub struct RcuBox(NonNull>, PhantomData); > >   > >  struct RcuBoxInner { > >      value: T, > >      rcu_head: bindings::callback_head, > >  } > >   > > -// Note that `T: Sync` is required since when moving an `RcuBox`, the previous owner may still > > -// access `&T` for one grace period. > > +// Note that `T: Sync` is required since when moving an `RcuBox`, the previous owner may > > +// still access `&T` for one grace period. > >  // > > -// SAFETY: Ownership of the `RcuBox` allows for `&T` and dropping the `T`, so `T: Send + Sync` > > -// implies `RcuBox: Send`. > > -unsafe impl Send for RcuBox {} > > +// SAFETY: Ownership of the `RcuBox` allows for `&T` and dropping the `T`, so `T: Send + > > +// Sync` implies `RcuBox: Send`. > > +unsafe impl Send for RcuBox {} > >   > > -// SAFETY: `&RcuBox` allows for no operations other than those permitted by `&T`, so `T: Sync` > > -// implies `RcuBox: Sync`. > > -unsafe impl Sync for RcuBox {} > > +// SAFETY: `&RcuBox` allows for no operations other than those permitted by `&T`, so `T: > > +// Sync` implies `RcuBox: Sync`. > > +unsafe impl Sync for RcuBox {} > >   > > -impl RcuBox { > > +impl RcuBox { > >      /// Create a new `RcuBox`. > >      pub fn new(x: T, flags: alloc::Flags) -> Result { > > -        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 { > >          )?; > >   > >          // 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 Deref for RcuBox { > > +impl Deref for RcuBox { > >      type Target = T; > >      fn deref(&self) -> &T { > >          // SAFETY: While the `RcuBox` exists, the value remains valid. > > @@ -75,10 +87,10 @@ fn deref(&self) -> &T { > >  } > >   > >  // SAFETY: > > -// * The `RcuBoxInner` was allocated with `kmalloc`. > > +// * The `RcuBoxInner` was allocated with `A`. > >  // * `NonNull::as_ptr` returns a non-null pointer. > > -unsafe impl ForeignOwnable for RcuBox { > > -    const FOREIGN_ALIGN: usize = > as ForeignOwnable>::FOREIGN_ALIGN; > > +unsafe impl ForeignOwnable for RcuBox { > > +    const FOREIGN_ALIGN: usize = , 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`. > > +        // INVARIANT: Pointer returned by `into_foreign, A` carries same invariants as `RcuBox`. > >          // 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 ForeignOwnableRcu for RcuBox { > > +impl ForeignOwnableRcu for RcuBox { > >      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 Drop for RcuBox { > > +impl Drop for RcuBox { > >      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`. 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::)) }; > > +            unsafe { bindings::call_rcu(rcu_head, Some(drop_rcu_box::)) }; > >          } 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` that has no references to it. > > -/// Ownership of the `KBox>` must be passed. > > -unsafe extern "C" fn drop_rcu_box(head: *mut bindings::callback_head) { > > +/// Ownership of the `Box, A>` must be passed. > > +unsafe extern "C" fn drop_rcu_box(head: *mut bindings::callback_head) { > >      // SAFETY: Caller provides a pointer to the `rcu_head` field of a `RcuBoxInner`. > >      let box_inner = unsafe { crate::container_of!(head, RcuBoxInner, 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(()) > > +    } > >  }