[PATCH 1/2] rust: kernel: add `drop_contents` to `BoxExt`

[PATCH 1/2] rust: kernel: add `drop_contents` to `BoxExt`

[Benno Lossin]

Sometimes (see [1]) it is necessary to drop the value inside of a
`Box<T>`, but retain the allocation. For example to reuse the allocation
in the future.
Introduce a new function `drop_contents` that turns a `Box<T>` into
`Box<MaybeUninit<T>>` by dropping the value.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/rust-for-linux/20240418-b4-rbtree-v3-5-323e134390ce@google.com/ [1]
---
 rust/kernel/alloc/box_ext.rs | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/rust/kernel/alloc/box_ext.rs b/rust/kernel/alloc/box_ext.rs
index cdbb5ad166d9..3ddb353b776e 100644
--- a/rust/kernel/alloc/box_ext.rs
+++ b/rust/kernel/alloc/box_ext.rs
@@ -5,6 +5,7 @@
 use super::{AllocError, Flags};
 use alloc::boxed::Box;
 use core::mem::MaybeUninit;
+use core::ptr;
 use core::result::Result;
 
 /// Extensions to [`Box`].
@@ -18,6 +19,18 @@ pub trait BoxExt<T>: Sized {
     ///
     /// The allocation may fail, in which case an error is returned.
     fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>>, AllocError>;
+
+    /// Drops the contents, but keeps the allocation.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let value = Box::new([0; 32], flags::GFP_KERNEL)
+    /// let value = value.drop_contents();
+    /// // Now we can re-use `value`:
+    /// Box::write(value, [1; 32]);
+    /// ```
+    fn drop_contents(self) -> Box<MaybeUninit<T>>;
 }
 
 impl<T> BoxExt<T> for Box<T> {
@@ -54,4 +67,12 @@ fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>>, AllocError> {
         // zero-sized types, we use `NonNull::dangling`.
         Ok(unsafe { Box::from_raw(ptr) })
     }
+
+    fn drop_contents(self) -> Box<MaybeUninit<T>> {
+        let ptr = Box::into_raw(self);
+        // SAFETY: `ptr` is valid, because it came from `Box::into_raw`.
+        unsafe { ptr::drop_in_place(ptr) };
+        // SAFETY: `ptr` is valid, because it came from `Box::into_raw`.
+        unsafe { Box::from_raw(ptr) }
+    }
 }
-- 
2.44.0

[PATCH 2/2] rust: init: add re-initialization functions

[Benno Lossin]

Sometimes it is necessary to split allocation and initialization into
two steps. One such situation is when reusing existing allocations
obtained via `Box::drop_contents`. See [1] for an example.
In order to support this use case add `re_[pin_]init` functions to the
pin-init API. These functions operate on already allocated smart
pointers that contain `MaybeUninit<T>`.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]
---
 rust/kernel/init.rs    | 88 ++++++++++++++++++++++++++++++------------
 rust/kernel/prelude.rs |  2 +-
 2 files changed, 65 insertions(+), 25 deletions(-)

diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 9608f2bd2211..b37b23f07bf7 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -1159,13 +1159,8 @@ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>,
     where
         E: From<AllocError>,
     {
-        let mut this = <Box<_> as BoxExt<_>>::new_uninit(flags)?;
-        let slot = this.as_mut_ptr();
-        // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
-        // slot is valid and will not be moved, because we pin it later.
-        unsafe { init.__pinned_init(slot)? };
-        // SAFETY: All fields have been initialized.
-        Ok(unsafe { this.assume_init() }.into())
+        let this = <Box<_> as BoxExt<_>>::new_uninit(flags)?;
+        this.re_pin_init(init)
     }
 
     #[inline]
@@ -1173,13 +1168,8 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
     where
         E: From<AllocError>,
     {
-        let mut this = <Box<_> as BoxExt<_>>::new_uninit(flags)?;
-        let slot = this.as_mut_ptr();
-        // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
-        // slot is valid.
-        unsafe { init.__init(slot)? };
-        // SAFETY: All fields have been initialized.
-        Ok(unsafe { this.assume_init() })
+        let this = <Box<_> as BoxExt<_>>::new_uninit(flags)?;
+        this.re_init(init)
     }
 }
 
@@ -1189,13 +1179,8 @@ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>,
     where
         E: From<AllocError>,
     {
-        let mut this = UniqueArc::new_uninit(flags)?;
-        let slot = this.as_mut_ptr();
-        // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
-        // slot is valid and will not be moved, because we pin it later.
-        unsafe { init.__pinned_init(slot)? };
-        // SAFETY: All fields have been initialized.
-        Ok(unsafe { this.assume_init() }.into())
+        let this = UniqueArc::new_uninit(flags)?;
+        this.re_pin_init(init)
     }
 
     #[inline]
@@ -1203,13 +1188,68 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
     where
         E: From<AllocError>,
     {
-        let mut this = UniqueArc::new_uninit(flags)?;
-        let slot = this.as_mut_ptr();
+        let this = UniqueArc::new_uninit(flags)?;
+        this.re_init(init)
+    }
+}
+
+/// Smart pointer that can re-initialize its content.
+pub trait InPlaceReInit<T> {
+    /// The type `Self` turns into when re-initialized.
+    type Initialized;
+
+    /// Re-initializes `self` with the given initializer.
+    ///
+    /// Does not drop the current value and considers it as uninitialized memory.
+    fn re_init<E>(self, init: impl Init<T, E>) -> Result<Self::Initialized, E>;
+
+    /// Re-initializes `self` with the given initializer.
+    ///
+    /// Does not drop the current value and considers it as uninitialized memory.
+    fn re_pin_init<E>(self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E>;
+}
+
+impl<T> InPlaceReInit<T> for Box<MaybeUninit<T>> {
+    type Initialized = Box<T>;
+
+    fn re_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E> {
+        let slot = self.as_mut_ptr();
         // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
         // slot is valid.
         unsafe { init.__init(slot)? };
         // SAFETY: All fields have been initialized.
-        Ok(unsafe { this.assume_init() })
+        Ok(unsafe { self.assume_init() })
+    }
+
+    fn re_pin_init<E>(mut self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E> {
+        let slot = self.as_mut_ptr();
+        // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
+        // slot is valid and will not be moved, because we pin it later.
+        unsafe { init.__pinned_init(slot)? };
+        // SAFETY: All fields have been initialized.
+        Ok(unsafe { self.assume_init() }.into())
+    }
+}
+
+impl<T> InPlaceReInit<T> for UniqueArc<MaybeUninit<T>> {
+    type Initialized = UniqueArc<T>;
+
+    fn re_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E> {
+        let slot = self.as_mut_ptr();
+        // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
+        // slot is valid.
+        unsafe { init.__init(slot)? };
+        // SAFETY: All fields have been initialized.
+        Ok(unsafe { self.assume_init() })
+    }
+
+    fn re_pin_init<E>(mut self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E> {
+        let slot = self.as_mut_ptr();
+        // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
+        // slot is valid and will not be moved, because we pin it later.
+        unsafe { init.__pinned_init(slot)? };
+        // SAFETY: All fields have been initialized.
+        Ok(unsafe { self.assume_init() }.into())
     }
 }
 
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index b37a0b3180fb..078b2b1d84ae 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -37,6 +37,6 @@
 
 pub use super::{str::CStr, ThisModule};
 
-pub use super::init::{InPlaceInit, Init, PinInit};
+pub use super::init::{InPlaceInit, InPlaceReInit, Init, PinInit};
 
 pub use super::current;
-- 
2.44.0

Re: [PATCH 2/2] rust: init: add re-initialization functions

[Gary Guo]

On Thu, 25 Apr 2024 21:34:44 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> Sometimes it is necessary to split allocation and initialization into
> two steps. One such situation is when reusing existing allocations
> obtained via `Box::drop_contents`. See [1] for an example.
> In order to support this use case add `re_[pin_]init` functions to the
> pin-init API. These functions operate on already allocated smart
> pointers that contain `MaybeUninit<T>`.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]


I don't find the re_init name very intuitive. From the name I would
imagine these functions be taking a `Box<T>` and a `impl Init<T, E>`,
dropping the content and produces a `Box<T>` again.

Would it make more to rename the existing functions to have `new` in
their name to indiciate that they allocate, e.g. `pin_new`, and have
these functions that only does initialisation `init`/`pin_init`?

Best,
Gary

Re: [PATCH 2/2] rust: init: add re-initialization functions

[Benno Lossin]

On 29.04.24 14:24, Gary Guo wrote:
> On Thu, 25 Apr 2024 21:34:44 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
> 
>> Sometimes it is necessary to split allocation and initialization into
>> two steps. One such situation is when reusing existing allocations
>> obtained via `Box::drop_contents`. See [1] for an example.
>> In order to support this use case add `re_[pin_]init` functions to the
>> pin-init API. These functions operate on already allocated smart
>> pointers that contain `MaybeUninit<T>`.
>>
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]
> 
> 
> I don't find the re_init name very intuitive. From the name I would
> imagine these functions be taking a `Box<T>` and a `impl Init<T, E>`,
> dropping the content and produces a `Box<T>` again.

I see your point, but if you look at the link [1] from above, you will
see that there such a function wouldn't be helpful.

> Would it make more to rename the existing functions to have `new` in
> their name to indiciate that they allocate, e.g. `pin_new`, and have
> these functions that only does initialisation `init`/`pin_init`?

Since we now have full control over `Box::new` (via `BoxExt`), we could
also make it take a `impl Init<T, E>` instead of just `T`.
And we could also provide `fn pin(impl PinInit<T>) -> Pin<Box<T>>`.

I would happily rename the `re_init` functions to `init` in that case.
But if we don't want to do the other rename, then I think it would be
confusing to have the functions `new(T)`, `pin(T)`,
`pin_new(impl PinInit<T, E>)` and `new_in_place(impl Init<T, E>)`...

--
Cheers,
Benno

Re: [PATCH 2/2] rust: init: add re-initialization functions

[Alice Ryhl]

On Thu, Apr 25, 2024 at 11:34 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Sometimes it is necessary to split allocation and initialization into
> two steps. One such situation is when reusing existing allocations
> obtained via `Box::drop_contents`. See [1] for an example.
> In order to support this use case add `re_[pin_]init` functions to the
> pin-init API. These functions operate on already allocated smart
> pointers that contain `MaybeUninit<T>`.
>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]

I'm not a big fan of the name. Perhaps we can use a name similar to
`Box::write`?

Alice

Re: [PATCH 2/2] rust: init: add re-initialization functions

[Benno Lossin]

On 03.05.24 13:34, Alice Ryhl wrote:
> On Thu, Apr 25, 2024 at 11:34 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> Sometimes it is necessary to split allocation and initialization into
>> two steps. One such situation is when reusing existing allocations
>> obtained via `Box::drop_contents`. See [1] for an example.
>> In order to support this use case add `re_[pin_]init` functions to the
>> pin-init API. These functions operate on already allocated smart
>> pointers that contain `MaybeUninit<T>`.
>>
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]
> 
> I'm not a big fan of the name. Perhaps we can use a name similar to
> `Box::write`?

Sure, what would be your suggestion? I can only think of `write_pinned`,
but no idea what to do for `impl Init<T>`...

-- 
Cheers,
Benno

Re: [PATCH 2/2] rust: init: add re-initialization functions

[Alice Ryhl]

On Sat, May 4, 2024 at 5:45 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On 03.05.24 13:34, Alice Ryhl wrote:
> > On Thu, Apr 25, 2024 at 11:34 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> Sometimes it is necessary to split allocation and initialization into
> >> two steps. One such situation is when reusing existing allocations
> >> obtained via `Box::drop_contents`. See [1] for an example.
> >> In order to support this use case add `re_[pin_]init` functions to the
> >> pin-init API. These functions operate on already allocated smart
> >> pointers that contain `MaybeUninit<T>`.
> >>
> >> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> >> Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]
> >
> > I'm not a big fan of the name. Perhaps we can use a name similar to
> > `Box::write`?
>
> Sure, what would be your suggestion? I can only think of `write_pinned`,
> but no idea what to do for `impl Init<T>`...

write_init and write_pin_init?