[PATCH v3] rust: Implement the smart pointer `InPlaceInit` for `Arc`

[PATCH v3] rust: Implement the smart pointer `InPlaceInit` for `Arc`

[Alex Mantel]

For pinned and unpinned initialization of structs, a trait named
`InPlaceInit` exists for uniform access. `Arc` did not implement
`InPlaceInit` yet, although the functions already existed. The main
reason for that, was that the trait itself returned a `Pin<Self>`. The
`Arc` implementation of the kernel is already implicitly pinned.

To enable `Arc` to implement `InPlaceInit` and to have uniform access,
for in-place and pinned in-place initialization, an associated type is
introduced for `InPlaceInit`. The new implementation of `InPlaceInit`
for `Arc` sets `Arc` as the associated type. Older implementations use
an explicit `Pin<T>` as the associated type. The implemented methods for
`Arc` are mostly moved from a direct implementation on `Arc`. There
should be no user impact. The implementation for `ListArc` is omitted,
because it is not merged yet.

Link: https://github.com/Rust-for-Linux/linux/issues/1079
Signed-off-by: Alex Mantel <alexmantel93@mailbox.org>
---
Hello again!

This is the 3rd version of my very first patch. I applied the
suggestions on the 2nd submission. Thank you again for your feedback,
looking for more!

v1: 
  * https://lore.kernel.org/rust-for-linux/20240717034801.262343-2-alexmantel93@mailbox.org/

v2: 
  * remove the `From:` from the patch.
  * add the prefix `rust: ` to the subject.
  * Remove the empty line between `Link` and `Signed-off-by`.
  * https://lore.kernel.org/all/20240719192234.330341-1-alexmantel93@mailbox.org/

v3:
  * Rename PinnedResult to PinnedSelf
  * Adjust documentation for PinnedSelf

 rust/kernel/init.rs     | 39 +++++++++++++++++++++++++++++++++++----
 rust/kernel/sync/arc.rs | 25 ++-----------------------
 2 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 68605b633..fa5f182fe 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -213,6 +213,7 @@
 use crate::{
     alloc::{box_ext::BoxExt, AllocError, Flags},
     error::{self, Error},
+    sync::Arc,
     sync::UniqueArc,
     types::{Opaque, ScopeGuard},
 };
@@ -1112,11 +1113,17 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
 
 /// Smart pointer that can initialize memory in-place.
 pub trait InPlaceInit<T>: Sized {
+    /// Pinned version of Rusts `Self`.
+    ///
+    /// If a type already implicitly pins its pointee, `Pin<Self>` is unnecessary. In this case use
+    /// `Self`, otherwise just use `Pin<Self>`.
+    type PinnedSelf;
+
     /// 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>, flags: Flags) -> Result<Pin<Self>, E>
+    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedSelf, E>
     where
         E: From<AllocError>;
 
@@ -1124,7 +1131,7 @@ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>,
     /// type.
     ///
     /// If `T: !Unpin` it will not be able to move afterwards.
-    fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Pin<Self>>
+    fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Self::PinnedSelf>
     where
         Error: From<E>,
     {
@@ -1153,9 +1160,31 @@ fn init<E>(init: impl Init<T, E>, flags: Flags) -> error::Result<Self>
     }
 }
 
+impl<T> InPlaceInit<T> for Arc<T> {
+    type PinnedSelf = Self;
+
+    #[inline]
+    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedSelf, E>
+    where
+        E: From<AllocError>,
+    {
+        UniqueArc::try_pin_init(init, flags).map(|u| u.into())
+    }
+
+    #[inline]
+    fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
+    where
+        E: From<AllocError>,
+    {
+        UniqueArc::try_init(init, flags).map(|u| u.into())
+    }
+}
+
 impl<T> InPlaceInit<T> for Box<T> {
+    type PinnedSelf = Pin<Self>;
+
     #[inline]
-    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>, E>
+    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedSelf, E>
     where
         E: From<AllocError>,
     {
@@ -1184,8 +1213,10 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
 }
 
 impl<T> InPlaceInit<T> for UniqueArc<T> {
+    type PinnedSelf = Pin<Self>;
+
     #[inline]
-    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Pin<Self>, E>
+    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedSelf, E>
     where
         E: From<AllocError>,
     {
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 3673496c2..3021f30fd 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -12,12 +12,13 @@
 //! 2. It does not support weak references, which allows it to be half the size.
 //! 3. It saturates the reference count instead of aborting when it goes over a threshold.
 //! 4. It does not provide a `get_mut` method, so the ref counted object is pinned.
+//! 5. The object in [`Arc`] is pinned implicitly.
 //!
 //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
 
 use crate::{
     alloc::{box_ext::BoxExt, AllocError, Flags},
-    error::{self, Error},
+    bindings,
     init::{self, InPlaceInit, Init, PinInit},
     try_init,
     types::{ForeignOwnable, Opaque},
@@ -209,28 +210,6 @@ pub fn new(contents: T, flags: Flags) -> Result<Self, AllocError> {
         // `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>, flags: Flags) -> error::Result<Self>
-    where
-        Error: From<E>,
-    {
-        UniqueArc::pin_init(init, flags).map(|u| u.into())
-    }
-
-    /// Use the given initializer to in-place initialize a `T`.
-    ///
-    /// This is equivalent to [`Arc<T>::pin_init`], since an [`Arc`] is always pinned.
-    #[inline]
-    pub fn init<E>(init: impl Init<T, E>, flags: Flags) -> error::Result<Self>
-    where
-        Error: From<E>,
-    {
-        UniqueArc::init(init, flags).map(|u| u.into())
-    }
 }
 
 impl<T: ?Sized> Arc<T> {
-- 
2.39.2

Re: [PATCH v3] rust: Implement the smart pointer `InPlaceInit` for `Arc`

[Alice Ryhl]

On Sat, Jul 27, 2024 at 6:25 AM Alex Mantel <alexmantel93@mailbox.org> wrote:
>
> For pinned and unpinned initialization of structs, a trait named
> `InPlaceInit` exists for uniform access. `Arc` did not implement
> `InPlaceInit` yet, although the functions already existed. The main
> reason for that, was that the trait itself returned a `Pin<Self>`. The
> `Arc` implementation of the kernel is already implicitly pinned.
>
> To enable `Arc` to implement `InPlaceInit` and to have uniform access,
> for in-place and pinned in-place initialization, an associated type is
> introduced for `InPlaceInit`. The new implementation of `InPlaceInit`
> for `Arc` sets `Arc` as the associated type. Older implementations use
> an explicit `Pin<T>` as the associated type. The implemented methods for
> `Arc` are mostly moved from a direct implementation on `Arc`. There
> should be no user impact. The implementation for `ListArc` is omitted,
> because it is not merged yet.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1079
> Signed-off-by: Alex Mantel <alexmantel93@mailbox.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v3] rust: Implement the smart pointer `InPlaceInit` for `Arc`

[Benno Lossin]

On 27.07.24 06:24, Alex Mantel wrote:
> For pinned and unpinned initialization of structs, a trait named
> `InPlaceInit` exists for uniform access. `Arc` did not implement
> `InPlaceInit` yet, although the functions already existed. The main
> reason for that, was that the trait itself returned a `Pin<Self>`. The
> `Arc` implementation of the kernel is already implicitly pinned.
> 
> To enable `Arc` to implement `InPlaceInit` and to have uniform access,
> for in-place and pinned in-place initialization, an associated type is
> introduced for `InPlaceInit`. The new implementation of `InPlaceInit`
> for `Arc` sets `Arc` as the associated type. Older implementations use
> an explicit `Pin<T>` as the associated type. The implemented methods for
> `Arc` are mostly moved from a direct implementation on `Arc`. There
> should be no user impact. The implementation for `ListArc` is omitted,
> because it is not merged yet.
> 
> Link: https://github.com/Rust-for-Linux/linux/issues/1079
> Signed-off-by: Alex Mantel <alexmantel93@mailbox.org>

One documentation nit below, otherwise this LGTM:

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
> Hello again!
> 
> This is the 3rd version of my very first patch. I applied the
> suggestions on the 2nd submission. Thank you again for your feedback,
> looking for more!
> 
> v1:
>   * https://lore.kernel.org/rust-for-linux/20240717034801.262343-2-alexmantel93@mailbox.org/
> 
> v2:
>   * remove the `From:` from the patch.
>   * add the prefix `rust: ` to the subject.
>   * Remove the empty line between `Link` and `Signed-off-by`.
>   * https://lore.kernel.org/all/20240719192234.330341-1-alexmantel93@mailbox.org/
> 
> v3:
>   * Rename PinnedResult to PinnedSelf
>   * Adjust documentation for PinnedSelf
> 
>  rust/kernel/init.rs     | 39 +++++++++++++++++++++++++++++++++++----
>  rust/kernel/sync/arc.rs | 25 ++-----------------------
>  2 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 68605b633..fa5f182fe 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -213,6 +213,7 @@
>  use crate::{
>      alloc::{box_ext::BoxExt, AllocError, Flags},
>      error::{self, Error},
> +    sync::Arc,
>      sync::UniqueArc,
>      types::{Opaque, ScopeGuard},
>  };
> @@ -1112,11 +1113,17 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> {
> 
>  /// Smart pointer that can initialize memory in-place.
>  pub trait InPlaceInit<T>: Sized {
> +    /// Pinned version of Rusts `Self`.

I would not have mentioned "Rust" here, since that should be obvious.

---
Cheers,
Benno

> +    ///
> +    /// If a type already implicitly pins its pointee, `Pin<Self>` is unnecessary. In this case use
> +    /// `Self`, otherwise just use `Pin<Self>`.
> +    type PinnedSelf;
> +
>      /// 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>, flags: Flags) -> Result<Pin<Self>, E>
> +    fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::PinnedSelf, E>
>      where
>          E: From<AllocError>;
> 

Re: [PATCH v3] rust: Implement the smart pointer `InPlaceInit` for `Arc`

[Miguel Ojeda]

On Sat, Jul 27, 2024 at 6:25 AM Alex Mantel <alexmantel93@mailbox.org> wrote:
>
> For pinned and unpinned initialization of structs, a trait named
> `InPlaceInit` exists for uniform access. `Arc` did not implement
> `InPlaceInit` yet, although the functions already existed. The main
> reason for that, was that the trait itself returned a `Pin<Self>`. The
> `Arc` implementation of the kernel is already implicitly pinned.
>
> To enable `Arc` to implement `InPlaceInit` and to have uniform access,
> for in-place and pinned in-place initialization, an associated type is
> introduced for `InPlaceInit`. The new implementation of `InPlaceInit`
> for `Arc` sets `Arc` as the associated type. Older implementations use
> an explicit `Pin<T>` as the associated type. The implemented methods for
> `Arc` are mostly moved from a direct implementation on `Arc`. There
> should be no user impact. The implementation for `ListArc` is omitted,
> because it is not merged yet.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1079
> Signed-off-by: Alex Mantel <alexmantel93@mailbox.org>

Applied to `rust-next` -- thanks everyone!

    [ Removed "Rusts" (Benno). - Miguel ]

Cheers,
Miguel