[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87355et3lf.fsf@metaspace.dk>
Date: Wed, 05 Apr 2023 23:34:35 +0200
From: Andreas Hindborg <nmi@...aspace.dk>
To: Benno Lossin <y86-dev@...tonmail.com>
Cc: Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Alice Ryhl <alice@...l.io>, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, patches@...ts.linux.dev,
Alice Ryhl <aliceryhl@...gle.com>,
Andreas Hindborg <a.hindborg@...sung.com>
Subject: Re: [PATCH v6 08/15] rust: init/sync: add `InPlaceInit` trait to
pin-initialize smart pointers
Benno Lossin <y86-dev@...tonmail.com> writes:
> 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@...tonmail.com>
> Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> Reviewed-by: Gary Guo <gary@...yguo.net>
> Cc: Andreas Hindborg <a.hindborg@...sung.com>
> ---
Reviewed-by: Andreas Hindborg <a.hindborg@...sung.com>
> rust/kernel/init.rs | 128 ++++++++++++++++++++++++++++++++++++----
> rust/kernel/sync/arc.rs | 24 ++++++++
> 2 files changed, 139 insertions(+), 13 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index ecef0376d726..6499cf5c9c20 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -114,10 +114,16 @@
> //! [`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::{
> + alloc::AllocError, cell::Cell, convert::Infallible, marker::PhantomData, mem::MaybeUninit,
> + pin::Pin, ptr,
> +};
>
> #[doc(hidden)]
> pub mod __internal;
> @@ -309,7 +315,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]
> @@ -363,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]
> @@ -586,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]
> @@ -635,8 +636,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]
> @@ -842,7 +841,8 @@ macro_rules! try_init {
> /// A pin-initializer for the type `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).
> ///
> @@ -861,7 +861,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`.
> @@ -878,7 +877,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).
> @@ -903,7 +903,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>: Sized {
> /// Initializes `slot`.
> @@ -982,3 +981,106 @@ unsafe impl<T> Init<T> for T {
> Ok(())
> }
> }
> +
> +/// Smart pointer that can initialize memory in-place.
> +pub trait InPlaceInit<T>: Sized {
> + /// Use the given pin-initializer to pin-initialize a `T` inside of a new smart pointer of this
> + /// type.
> + ///
> + /// If `T: !Unpin` it will not be able to move afterwards.
> + fn try_pin_init<E>(init: impl PinInit<T, E>) -> Result<Pin<Self>, E>
> + where
> + E: From<AllocError>;
> +
> + /// Use the given pin-initializer to pin-initialize a `T` inside of a new smart pointer of this
> + /// type.
> + ///
> + /// 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>,
> + {
> + // SAFETY: We delegate to `init` and only change the error type.
> + let init = unsafe {
> + pin_init_from_closure(|slot| init.__pinned_init(slot).map_err(|e| Error::from(e)))
> + };
> + Self::try_pin_init(init)
> + }
> +
> + /// Use the given initializer to in-place initialize a `T`.
> + fn try_init<E>(init: impl Init<T, E>) -> Result<Self, E>
> + where
> + E: From<AllocError>;
> +
> + /// 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>,
> + {
> + // SAFETY: We delegate to `init` and only change the error type.
> + let init = unsafe {
> + init_from_closure(|slot| init.__pinned_init(slot).map_err(|e| Error::from(e)))
> + };
> + Self::try_init(init)
> + }
> +}
> +
> +impl<T> InPlaceInit<T> for Box<T> {
> + #[inline]
> + fn try_pin_init<E>(init: impl PinInit<T, E>) -> Result<Pin<Self>, E>
> + where
> + E: From<AllocError>,
> + {
> + 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 we pin it later.
> + unsafe { init.__pinned_init(slot)? };
> + // SAFETY: All fields have been initialized.
> + Ok(unsafe { this.assume_init() }.into())
> + }
> +
> + #[inline]
> + fn try_init<E>(init: impl Init<T, E>) -> Result<Self, E>
> + where
> + E: From<AllocError>,
> + {
> + 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 try_pin_init<E>(init: impl PinInit<T, E>) -> Result<Pin<Self>, E>
> + where
> + E: From<AllocError>,
> + {
> + 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 we pin it later.
> + unsafe { init.__pinned_init(slot)? };
> + // SAFETY: All fields have been initialized.
> + Ok(unsafe { this.assume_init() }.into())
> + }
> +
> + #[inline]
> + fn try_init<E>(init: impl Init<T, E>) -> Result<Self, E>
> + where
> + E: From<AllocError>,
> + {
> + 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 eee7008e5e3e..43a53fbe175d 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -17,6 +17,8 @@
>
> use crate::{
> bindings,
> + error::{self, Error},
> + init::{InPlaceInit, Init, PinInit},
> types::{ForeignOwnable, Opaque},
> };
> use alloc::boxed::Box;
> @@ -163,6 +165,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>) -> error::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>) -> error::Result<Self>
> + where
> + Error: From<E>,
> + {
> + UniqueArc::init(init).map(|u| u.into())
> + }
> }
>
> impl<T: ?Sized> Arc<T> {
Powered by blists - more mailing lists