Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 4 Apr 2023 14:15:49 +0100
From:   Gary Guo <gary@garyguo.net>
To:     Benno Lossin <y86-dev@protonmail.com>
Cc:     Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        =?UTF-8?B?QmrDtnJu?= Roy Baron <bjorn3_gh@protonmail.com>,
        Alice Ryhl <alice@ryhl.io>, rust-for-linux@vger.kernel.org,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev,
        Andreas Hindborg <a.hindborg@samsung.com>,
        Alice Ryhl <aliceryhl@google.com>
Subject: Re: [PATCH v5 08/15] rust: init/sync: add `InPlaceInit` trait to
 pin-initialize smart pointers
Message-ID: <20230404141549.48230994.gary@garyguo.net>
In-Reply-To: <20230403160511.174894-1-y86-dev@protonmail.com>
References: <20230403154422.168633-1-y86-dev@protonmail.com>
        <20230403160511.174894-1-y86-dev@protonmail.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Cc4e37kMYo6YMdOKY7iBE+21dQ5G2sFy/27E9xSUEnACQbvYHLHWsdPm/43/?=
 =?us-ascii?Q?NAqMScAIXcfoQt+aplS7mmO68x6cUar0KNhD3p0up9bCH1ym0sQBtNUj+Whp?=
 =?us-ascii?Q?3tJfFo09as6Yp1Jx5mrQVnUycdhxj24ls/8Jhh2sx/vb4m3OjRiTXAXkwcq2?=
 =?us-ascii?Q?GQIRxo7jzPVGgquwTnV0XUCw/9aEpOo2R3N4OjxcTjH1DTE9+NCZwtAWh60u?=
 =?us-ascii?Q?p93JmHAcjkBR5LF1+cU4/ET7FyBD7jx4X0y381j9owXF/ULSvZTvPhRvsOKU?=
 =?us-ascii?Q?o8lNzIx+uHgEGkM60ULiahyDXY6+G4aahq29vovrZX7ui+5q5rdA0Kc5THpd?=
 =?us-ascii?Q?wiZtBnYEc5tOfPciK0EYCab37jBac4Fd+ZUIBwY1Yq2SV5DaEH305WugdRjp?=
 =?us-ascii?Q?kiSwWehflVpdjt9TtPDoXmAHHNmWewSnIuz1S7D/fcD/rX3tTdY4ZP/4W5e+?=
 =?us-ascii?Q?bp8IAAWkY4Tzz3/aDOXwiokbfk6RHkgxhjz7XqMcMgpYb+oInbsEtNIfucZh?=
 =?us-ascii?Q?FTEtR544BhlNJrnQVzMVEkGbib99teI6vZKnLwiu9RO5GrKVSP2SKhWkGo70?=
 =?us-ascii?Q?NxH25wZr5TAMIZmdy0S5DZyxFOVABO7o2oSAleeYQYzgYPtZHUy2NhLqS5oj?=
 =?us-ascii?Q?G3x6Q8Vt/JdhZYKJQLd3Iap+Y9O0Y50PSTSwSo2gL2PTj2ry7oFHUfH1yOC7?=
 =?us-ascii?Q?82Ug0wD5roSbAUCBjTUdQdXmtNb+7zbwv1IBCPWavxhnB8e1rmNgOH4cBDX9?=
 =?us-ascii?Q?A1vYkCVoKmVMuuqBPGJ9rdcaiwk5skLeurgAEgdbe7eYq4i7KOUCX0qXsiV4?=
 =?us-ascii?Q?pSGWK1bCqF5vJ2Z2I6OQG8kpbZpi9mgulSNJfTycTs/lFG3oo4l9lDyFGJut?=
 =?us-ascii?Q?34Qn3W7h+rYW5wWtLtb3wOwSwM6ckCUkuwdtTCioEWAdokDdCSlIMShH80gO?=
 =?us-ascii?Q?w1vMlzLS5Kpyv+ibwoRhTbfaEF9tWOePadpk/0F6XQWeeBx4xR5iy9dcGuil?=
 =?us-ascii?Q?RQDfnesWD+nZDsZ1Z6BSTvXgXpAfmY/uhwT92bE/0ge3SsUgthK3l29Mdx3X?=
 =?us-ascii?Q?UQdp9vAA5HNq8edJZwpk5fRI27hqvTws67bsvQo5UQbwE1CAozuJl9P8wdzx?=
 =?us-ascii?Q?UIoIDzgaWGT0n6o9Rt/kB6DAQ6+ktVLdUyHZMqxN7+MV0M3ZO+wfoMvLF/+C?=
 =?us-ascii?Q?wWVt2jS2cL1EQ9puKhqz28j7O+wBREMEDby1MPqruRZ1+Q5WQSKIP31Mjrqa?=
 =?us-ascii?Q?v9rRySMqeDRs+WKnIYRQl3xwVYeBR2eDrP7FsfY3OiZdQSHIw9Y+5iWVFNok?=
 =?us-ascii?Q?ePBlRaHaUzDfx4YLwSpym9E6KmX+Sk1UBhOATOv8hUDwD+/f4Jvnv2qrpvew?=
 =?us-ascii?Q?Luqnuvj4wFxCTljaTxup5bnRrXe3fFt4jFg6CxevYHFy5Yy76nbZ4Ga5slN5?=
 =?us-ascii?Q?kyo3P+31qMPDaHGqORpAWurojYtQn+9OrG1fVJp856nEL+FmaoYm+ZaI5a79?=
 =?us-ascii?Q?qbCCTPOviiNO1XdF1ZknhTUfwvaaQlrzN8ounNPxHygtZ9cmCgG9kaKJeiB1?=
 =?us-ascii?Q?UJapJaHUwqAco2th9QRi4ejzbv3EWt7AAkOXuEIZ?=
X-OriginatorOrg: garyguo.net
X-MS-Exchange-CrossTenant-Network-Message-Id: 61f50f6a-6e15-4727-7b62-08db350eb6af
X-MS-Exchange-CrossTenant-AuthSource: LO2P265MB5183.GBRP265.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2023 13:15:51.5466
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bbc898ad-b10f-4e10-8552-d9377b823d45
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: vOmB6mJuVP+P35mDNv7kNUpWpnB0nfbFZNh0YC+MDwJE3BogyZs5jp+1WKMUSjyHSqZcrh0fRUOCiiTNuxkQzA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LO2P265MB6309
X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_PASS
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, 03 Apr 2023 16:05:22 +0000
Benno Lossin <y86-dev@protonmail.com> wrote:

> 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@protonmail.com>
> Cc: Andreas Hindborg <a.hindborg@samsung.com>
> Cc: Alice Ryhl <aliceryhl@google.com>
> Cc: Gary Guo <gary@garyguo.net>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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> {
> --
> 2.39.2
> 
>