2024-04-25 21:34:58

by Benno Lossin

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

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 <[email protected]>
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [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




2024-04-25 21:35:29

by Benno Lossin

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

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 <[email protected]>
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [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



2024-04-29 12:25:23

by Gary Guo

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

On Thu, 25 Apr 2024 21:34:44 +0000
Benno Lossin <[email protected]> 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 <[email protected]>
> Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [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

2024-04-29 17:44:50

by Benno Lossin

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

On 29.04.24 14:24, Gary Guo wrote:
> On Thu, 25 Apr 2024 21:34:44 +0000
> Benno Lossin <[email protected]> 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 <[email protected]>
>> Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [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


2024-05-03 11:35:41

by Alice Ryhl

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

On Thu, Apr 25, 2024 at 11:34 PM Benno Lossin <[email protected]> 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 <[email protected]>
> Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]

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

Alice

2024-05-04 15:45:41

by Benno Lossin

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

On 03.05.24 13:34, Alice Ryhl wrote:
> On Thu, Apr 25, 2024 at 11:34 PM Benno Lossin <[email protected]> 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 <[email protected]>
>> Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [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


2024-05-15 09:48:45

by Alice Ryhl

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

On Sat, May 4, 2024 at 5:45 PM Benno Lossin <[email protected]> wrote:
>
> On 03.05.24 13:34, Alice Ryhl wrote:
> > On Thu, Apr 25, 2024 at 11:34 PM Benno Lossin <[email protected]> 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 <[email protected]>
> >> Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [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?