linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: alloc: implement Box::pin_slice()
@ 2025-08-08 15:07 Vitaly Wool
  2025-08-09 12:26 ` Danilo Krummrich
  0 siblings, 1 reply; 2+ messages in thread
From: Vitaly Wool @ 2025-08-08 15:07 UTC (permalink / raw)
  To: rust-for-linux
  Cc: linux-kernel, Uladzislau Rezki, Danilo Krummrich, Alice Ryhl,
	Vlastimil Babka, Lorenzo Stoakes, Liam R . Howlett, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross, Vitaly Wool

From: Alice Ryhl <aliceryhl@google.com>

Add a new constructor to Box to facilitate Box creation from a pinned
slice of elements. This allows to efficiently allocate memory for e.g.
slices of structrures containing spinlocks or mutexes. Such slices may
be used in kmemcache like or zpool API implementations.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
---

This patch supersedes "rust: extend kbox with a new constructor" posted
a day earlier.

 rust/kernel/alloc/kbox.rs | 61 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index 1fef9beb57c8..f0be307f7242 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -290,6 +290,67 @@ 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. slices of structrures containing spinlocks
+    /// or mutexes.
+    ///
+    /// # 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
+    /// )?;
+    /// assert_eq!(s[5].c, 10);
+    /// assert_eq!(s[3].d.lock().a, 20),
+    /// ```
+    pub fn pin_slice<Func, Item, E>(
+        mut init: Func,
+        len: usize,
+        flags: Flags,
+    ) -> Result<Pin<Box<[T], A>>, E>
+    where
+        Func: FnMut(usize) -> Item,
+        Item: PinInit<T, E>,
+        E: From<AllocError>,
+    {
+        let mut buffer = super::Vec::<T, A>::with_capacity(len, flags)?;
+        for i in 0..len {
+            let ptr = buffer.spare_capacity_mut().as_mut_ptr().cast();
+            // SAFETY:
+            // - `ptr` is a valid pointer to uninitialized memory.
+            // - `ptr` is not used if an error is returned.
+            // - `ptr` won't be moved until it is dropped, i.e. it is pinned.
+            unsafe { init(i).__pinned_init(ptr)? };
+            // SAFETY:
+            // - `i + 1 <= len` => we don't exceed the capacity
+            // - this new value is initialized
+            unsafe { buffer.inc_len(1) };
+        }
+        let (ptr, _, _) = buffer.into_raw_parts();
+        let slice = core::ptr::slice_from_raw_parts_mut(ptr, len);
+        // SAFETY: `slice` is not a NULL pointer because it is a valid pointer to [T]
+        Ok(Pin::from(unsafe { Box::from_raw(slice) }))
+    }
+
     /// Convert a [`Box<T,A>`] to a [`Pin<Box<T,A>>`]. If `T` does not implement
     /// [`Unpin`], then `x` will be pinned in memory and can't be moved.
     pub fn into_pin(this: Self) -> Pin<Self> {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] rust: alloc: implement Box::pin_slice()
  2025-08-08 15:07 [PATCH] rust: alloc: implement Box::pin_slice() Vitaly Wool
@ 2025-08-09 12:26 ` Danilo Krummrich
  0 siblings, 0 replies; 2+ messages in thread
From: Danilo Krummrich @ 2025-08-09 12:26 UTC (permalink / raw)
  To: Vitaly Wool
  Cc: rust-for-linux, linux-kernel, Uladzislau Rezki, Alice Ryhl,
	Vlastimil Babka, Lorenzo Stoakes, Liam R . Howlett, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Trevor Gross

On Fri Aug 8, 2025 at 5:07 PM CEST, Vitaly Wool wrote:
> From: Alice Ryhl <aliceryhl@google.com>
>
> Add a new constructor to Box to facilitate Box creation from a pinned
> slice of elements. This allows to efficiently allocate memory for e.g.
> slices of structrures containing spinlocks or mutexes. Such slices may
> be used in kmemcache like or zpool API implementations.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
> ---
>
> This patch supersedes "rust: extend kbox with a new constructor" posted
> a day earlier.

It is still the same patch, just with the review feedback applied, hence it
should be a normal v2. I assume the change of commit subject got you confused on
what's the correct thing to do process wise. :)

I have a few more comments below, when you send a new version, please declare it
as v3.

>  rust/kernel/alloc/kbox.rs | 61 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index 1fef9beb57c8..f0be307f7242 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -290,6 +290,67 @@ 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. slices of structrures containing spinlocks
> +    /// or mutexes.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// #[pin_data]
> +    /// struct Example {
> +    ///     c: u32,
> +    ///     #[pin]
> +    ///     d: SpinLock<Inner>,

I did not compile the patch (yet), but I'm pretty sure this can't work; I don't
see a definition of Inner anywhere.

Do you test with CONFIG_RUST_KERNEL_DOCTESTS=y? If not, you should enable this
config.

> +    /// }
> +    ///
> +    /// 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
> +    /// )?;
> +    /// assert_eq!(s[5].c, 10);
> +    /// assert_eq!(s[3].d.lock().a, 20),
> +    /// ```

I think this is missing `# Ok::<(), Error>(())`.

> +    pub fn pin_slice<Func, Item, E>(
> +        mut init: Func,
> +        len: usize,
> +        flags: Flags,
> +    ) -> Result<Pin<Box<[T], A>>, E>
> +    where
> +        Func: FnMut(usize) -> Item,
> +        Item: PinInit<T, E>,
> +        E: From<AllocError>,
> +    {
> +        let mut buffer = super::Vec::<T, A>::with_capacity(len, flags)?;
> +        for i in 0..len {
> +            let ptr = buffer.spare_capacity_mut().as_mut_ptr().cast();
> +            // SAFETY:
> +            // - `ptr` is a valid pointer to uninitialized memory.
> +            // - `ptr` is not used if an error is returned.
> +            // - `ptr` won't be moved until it is dropped, i.e. it is pinned.
> +            unsafe { init(i).__pinned_init(ptr)? };

NIT: Please add an empty line here.

> +            // SAFETY:
> +            // - `i + 1 <= len` => we don't exceed the capacity

Please make this a sentence, e.g. "`i + 1 <= len`, hence we don't exceed the
capacity due to the call to with_capacity() above."

> +            // - this new value is initialized

Same here, please make this a full sentence.

Also, it is unclear what "this new value" refers to. I know what you mean, but
it would be better to be more clear about it, e.g. "With the above call to
`init(i).__pinned_init(ptr)` we just initialized the element at index
`buffer.len() + 1`.

> +            unsafe { buffer.inc_len(1) };
> +        }
> +        let (ptr, _, _) = buffer.into_raw_parts();
> +        let slice = core::ptr::slice_from_raw_parts_mut(ptr, len);
> +        // SAFETY: `slice` is not a NULL pointer because it is a valid pointer to [T]

Please have a look at the safety requirements of Box::from_raw() and try to come
up with a sentence that explains why the pointer you pass in fulfills the safety
requirements.

> +        Ok(Pin::from(unsafe { Box::from_raw(slice) }))
> +    }
> +
>      /// Convert a [`Box<T,A>`] to a [`Pin<Box<T,A>>`]. If `T` does not implement
>      /// [`Unpin`], then `x` will be pinned in memory and can't be moved.
>      pub fn into_pin(this: Self) -> Pin<Self> {
> -- 
> 2.39.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-09 12:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-08 15:07 [PATCH] rust: alloc: implement Box::pin_slice() Vitaly Wool
2025-08-09 12:26 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).