Re: [PATCH] rust: extend kbox with a new constructor

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Aug 07, 2025 at 09:56:09PM +0200, Danilo Krummrich wrote:
> On Thu Aug 7, 2025 at 2:10 PM CEST, Vitaly Wool wrote:
> 
> Please start the patch subject with "rust: alloc:" and make the subject more
> concrete, i.e. name the constructor you add, e.g. "rust: alloc: implement
> Box::pin_slice()".
> 
> This makes things much more obvious when using 'git log --oneline'.
> 
> > From: Alice Ryhl <aliceryhl@google.com>
> >
> > Add a new constructor to KBox to facilitate KBox creation from a
> 
> NIT: You're adding it for all allorcators, hence "Box".
> 
> > pinned slice of elements. This allows to efficiently allocate memory for
> > e.g. arrays of structrures containing spinlocks or mutexes.
> 
> Sounds reasonable, can you please mention where this will be used?
> 
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
> > ---
> >  rust/kernel/alloc/kbox.rs | 51 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> >
> > diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> > index 1fef9beb57c8..74877afab0a3 100644
> > --- a/rust/kernel/alloc/kbox.rs
> > +++ b/rust/kernel/alloc/kbox.rs
> > @@ -290,6 +290,57 @@ pub fn pin(x: T, flags: Flags) -> Result<Pin<Box<T, A>>, AllocError>
> >          Ok(Self::new(x, flags)?.into())
> >      }
> >  
> > +    /// Construct a pinned slice of elements `Pin<Box<[T], A>>`. This is a convenient means for
> > +    /// creation of e.g. arrays of structrures containing spinlocks or mutexes.
> 
> Please add an empty line after the first sentence.
> 
> NIT: "slices of structures"
> 
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// #[pin_data]
> > +    /// struct Example {
> > +    ///     c: u32,
> > +    ///     #[pin]
> > +    ///     d: SpinLock<Inner>,
> > +    /// }
> > +    ///
> > +    /// impl Example {
> > +    ///     fn new() -> impl PinInit<Self> {
> > +    ///         pin_init!(Self {
> > +    ///             c: 10,
> > +    ///             d <- new_spinlock!(Inner { a: 20, b: 30 }),
> > +    ///         })
> > +    ///     }
> > +    /// }
> > +    /// // Allocate a boxed slice of 10 `Example`s.
> > +    /// let s = KBox::pin_slice(
> > +    ///     | _i | Example::new(),
> > +    ///     10,
> > +    ///     GFP_KERNEL
> > +    /// )?;
> 
> How would a more complex example look like where the slice items are not
> identical, i.e. the impl PinInit<T, E> objects returned by the FnMut?
> 
> Do we need a temporary Vec for this? If so, it would probably make more sense to
> have the following signature.
> 
> 	pub fn pin_slice<F, I, E, Arg>(mut init: F, &[Arg], flags: Flags) -> Result<Pin<Box<[T], A>>, E>
> 	where
> 	   F: FnMut(&Arg) -> I,
> 	   I: PinInit<T, E>,
> 	   E: From<AllocError>,
> 
> 
> Where Arg is some generic type containing the arguments passed to the closure
> to create the impl PinInit<T, E>. For the example above Args could just be ().

Forcing the user to construct an array of the appropriate length seems
unfortunate. Right now we provide the index being initialized, which you
can use to index an array if that's what you want.

I used a temporary Vec because its destructor does the right thing we if
exit early on error.

Alice