Re: [PATCH v3 06/13] rust: init/sync: add `InPlaceInit` trait to pin-initialize smart pointers

From: Andreas Hindborg
Date: Thu Mar 30 2023 - 09:40:00 EST



y86-dev@xxxxxxxxxxxxxx writes:

> From: Benno Lossin <y86-dev@xxxxxxxxxxxxxx>
>
> The `InPlaceInit` trait that provides two functions, for initializing
> using `PinInit<T, E>` and `Init<T>`. It is implemented by `Arc<T>`,
> `UniqueArc<T>` and `Box<T>`.
>
> Signed-off-by: Benno Lossin <y86-dev@xxxxxxxxxxxxxx>
> ---

Reviewed-by: Andreas Hindborg <a.hindborg@xxxxxxxxxxx>

> rust/kernel/init.rs | 97 +++++++++++++++++++++++++++++++++++------
> rust/kernel/sync/arc.rs | 25 ++++++++++-
> 2 files changed, 108 insertions(+), 14 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 85e8d5f41b60..3d89c7e3bdb5 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -114,10 +114,13 @@
> //! [`impl Init<T, E>`]: Init
> //! [`Opaque`]: kernel::types::Opaque
> //! [`pin_data`]: ::macros::pin_data
> -//! [`UniqueArc<T>`]: kernel::sync::UniqueArc
>
> +use crate::{
> + error::{self, Error},
> + sync::UniqueArc,
> +};
> use alloc::boxed::Box;
> -use core::{cell::Cell, convert::Infallible, marker::PhantomData, mem::MaybeUninit, ptr};
> +use core::{cell::Cell, convert::Infallible, marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr};
>
> #[doc(hidden)]
> pub mod __internal;
> @@ -309,7 +312,6 @@ pub mod macros;
> ///
> /// [`try_pin_init!`]: kernel::try_pin_init
> /// [`NonNull<Self>`]: core::ptr::NonNull
> -/// [`Error`]: kernel::error::Error
> // For a detailed example of how this macro works, see the module documentation of the hidden
> // module `__internal` inside of `init/__internal.rs`.
> #[macro_export]
> @@ -366,8 +368,6 @@ macro_rules! pin_init {
> /// }
> /// }
> /// ```
> -///
> -/// [`Error`]: kernel::error::Error
> // For a detailed example of how this macro works, see the module documentation of the hidden
> // module `__internal` inside of `init/__internal.rs`.
> #[macro_export]
> @@ -589,8 +589,6 @@ macro_rules! try_pin_init {
> ///
> /// This initializer is for initializing data in-place that might later be moved. If you want to
> /// pin-initialize, use [`pin_init!`].
> -///
> -/// [`Error`]: kernel::error::Error
> // For a detailed example of how this macro works, see the module documentation of the hidden
> // module `__internal` inside of `init/__internal.rs`.
> #[macro_export]
> @@ -641,8 +639,6 @@ macro_rules! init {
> /// }
> /// }
> /// ```
> -///
> -/// [`Error`]: kernel::error::Error
> // For a detailed example of how this macro works, see the module documentation of the hidden
> // module `__internal` inside of `init/__internal.rs`.
> #[macro_export]
> @@ -848,7 +844,8 @@ macro_rules! try_init {
> /// A pinned initializer for `T`.
> ///
> /// To use this initializer, you will need a suitable memory location that can hold a `T`. This can
> -/// be [`Box<T>`], [`Arc<T>`], [`UniqueArc<T>`].
> +/// be [`Box<T>`], [`Arc<T>`], [`UniqueArc<T>`]. Use the [`InPlaceInit::pin_init`] function of a
> +/// smart pointer like [`Arc<T>`] on this.
> ///
> /// Also see the [module description](self).
> ///
> @@ -867,7 +864,6 @@ macro_rules! try_init {
> ///
> /// [`Arc<T>`]: crate::sync::Arc
> /// [`Arc::pin_init`]: crate::sync::Arc::pin_init
> -/// [`UniqueArc<T>`]: kernel::sync::UniqueArc
> #[must_use = "An initializer must be used in order to create its value."]
> pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
> /// Initializes `slot`.
> @@ -884,7 +880,8 @@ pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
> /// An initializer for `T`.
> ///
> /// To use this initializer, you will need a suitable memory location that can hold a `T`. This can
> -/// be [`Box<T>`], [`Arc<T>`], [`UniqueArc<T>`]. Because [`PinInit<T, E>`] is a super trait, you can
> +/// be [`Box<T>`], [`Arc<T>`], [`UniqueArc<T>`]. Use the [`InPlaceInit::init`] function of a smart
> +/// pointer like [`Arc<T>`] on this. Because [`PinInit<T, E>`] is a super trait, you can
> /// use every function that takes it as well.
> ///
> /// Also see the [module description](self).
> @@ -909,7 +906,6 @@ pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
> /// move the pointee after initialization.
> ///
> /// [`Arc<T>`]: crate::sync::Arc
> -/// [`UniqueArc<T>`]: kernel::sync::UniqueArc
> #[must_use = "An initializer must be used in order to create its value."]
> pub unsafe trait Init<T: ?Sized, E = Infallible>: PinInit<T, E> {
> /// Initializes `slot`.
> @@ -1014,3 +1010,78 @@ unsafe impl<T> Init<T> for T {
> Ok(())
> }
> }
> +
> +/// Smart pointer that can initialize memory in-place.
> +pub trait InPlaceInit<T>: Sized {
> + /// Use the given initializer to in-place initialize a `T`.
> + ///
> + /// If `T: !Unpin` it will not be able to move afterwards.
> + fn pin_init<E>(init: impl PinInit<T, E>) -> error::Result<Pin<Self>>
> + where
> + Error: From<E>;
> +
> + /// Use the given initializer to in-place initialize a `T`.
> + fn init<E>(init: impl Init<T, E>) -> error::Result<Self>
> + where
> + Error: From<E>;
> +}
> +
> +impl<T> InPlaceInit<T> for Box<T> {
> + #[inline]
> + fn pin_init<E>(init: impl PinInit<T, E>) -> error::Result<Pin<Self>>
> + where
> + Error: From<E>,
> + {
> + let mut this = Box::try_new_uninit()?;
> + 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 of the `Pin::new_unchecked`
> + unsafe { init.__pinned_init(slot)? };
> + // SAFETY: All fields have been initialized.
> + Ok(unsafe { Pin::new_unchecked(this.assume_init()) })
> + }
> +
> + #[inline]
> + fn init<E>(init: impl Init<T, E>) -> error::Result<Self>
> + where
> + Error: From<E>,
> + {
> + let mut this = Box::try_new_uninit()?;
> + 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() })
> + }
> +}
> +
> +impl<T> InPlaceInit<T> for UniqueArc<T> {
> + #[inline]
> + fn pin_init<E>(init: impl PinInit<T, E>) -> error::Result<Pin<Self>>
> + where
> + Error: From<E>,
> + {
> + let mut this = UniqueArc::try_new_uninit()?;
> + 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 of the `Pin::new_unchecked`.
> + unsafe { init.__pinned_init(slot)? };
> + // SAFETY: All fields have been initialized.
> + Ok(unsafe { Pin::new_unchecked(this.assume_init()) })
> + }
> +
> + #[inline]
> + fn init<E>(init: impl Init<T, E>) -> error::Result<Self>
> + where
> + Error: From<E>,
> + {
> + let mut this = UniqueArc::try_new_uninit()?;
> + 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() })
> + }
> +}
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 16ec174637b2..77a3833cc265 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -17,7 +17,8 @@
>
> use crate::{
> bindings,
> - error::Result,
> + error::{Error, Result},
> + init::{InPlaceInit, Init, PinInit},
> types::{ForeignOwnable, Opaque},
> };
> use alloc::boxed::Box;
> @@ -163,6 +164,28 @@ impl<T> Arc<T> {
> // `Arc` object.
> Ok(unsafe { Self::from_inner(Box::leak(inner).into()) })
> }
> +
> + /// Use the given initializer to in-place initialize a `T`.
> + ///
> + /// If `T: !Unpin` it will not be able to move afterwards.
> + #[inline]
> + pub fn pin_init<E>(init: impl PinInit<T, E>) -> Result<Self>
> + where
> + Error: From<E>,
> + {
> + UniqueArc::pin_init(init).map(|u| u.into())
> + }
> +
> + /// Use the given initializer to in-place initialize a `T`.
> + ///
> + /// This is equivalent to [`pin_init`], since an [`Arc`] is always pinned.
> + #[inline]
> + pub fn init<E>(init: impl Init<T, E>) -> Result<Self>
> + where
> + Error: From<E>,
> + {
> + UniqueArc::init(init).map(|u| u.into())
> + }
> }
>
> impl<T: ?Sized> Arc<T> {