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;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.23\))
Subject: Re: [PATCH 6/7] rust: sync: introduce `UniqueArc`
From:   Laine Taffin Altman <alexanderaltman@me.com>
In-Reply-To: <20221228060346.352362-6-wedsonaf@gmail.com>
Date:   Tue, 27 Dec 2022 23:19:52 -0800
Cc:     rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>,
        =?utf-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <792D12DA-73F0-4218-B0E1-7A0D8C235C21@me.com>
References: <20221228060346.352362-1-wedsonaf@gmail.com>
 <20221228060346.352362-6-wedsonaf@gmail.com>
To:     Wedson Almeida Filho <wedsonaf@gmail.com>
Precedence: bulk


> On Dec 27, 2022, at 10:03 PM, Wedson Almeida Filho =
<wedsonaf@gmail.com> wrote:
>=20
> Since `Arc<T>` does not allow mutating `T` directly (i.e., without =
inner
> mutability), it is currently not possible to do some initialisation of
> `T` post construction but before being shared.
>=20
> `UniqueArc<T>` addresses this problem essentially being an `Arc<T>` =
that
> has a refcount of 1 and is therefore writable. Once initialisation is
> completed, it can be transitioned (without failure paths) into an
> `Arc<T>`.
>=20
> Suggested-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> ---
> rust/kernel/sync.rs     |   2 +-
> rust/kernel/sync/arc.rs | 152 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 151 insertions(+), 3 deletions(-)
>=20
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 5de03ea83ea1..33da23e3076d 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -7,4 +7,4 @@
>=20
> mod arc;
>=20
> -pub use arc::{Arc, ArcBorrow};
> +pub use arc::{Arc, ArcBorrow, UniqueArc};
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 84f31c85a513..832bafc74a90 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -19,8 +19,9 @@ use crate::{bindings, error::Result, types::Opaque};
> use alloc::boxed::Box;
> use core::{
>     marker::{PhantomData, Unsize},
> -    mem::ManuallyDrop,
> -    ops::Deref,
> +    mem::{ManuallyDrop, MaybeUninit},
> +    ops::{Deref, DerefMut},
> +    pin::Pin,
>     ptr::NonNull,
> };
>=20
> @@ -222,6 +223,19 @@ impl<T: ?Sized> Drop for Arc<T> {
>     }
> }
>=20
> +impl<T: ?Sized> From<UniqueArc<T>> for Arc<T> {
> +    fn from(item: UniqueArc<T>) -> Self {
> +        item.inner
> +    }
> +}
> +
> +impl<T: ?Sized> From<Pin<UniqueArc<T>>> for Arc<T> {
> +    fn from(item: Pin<UniqueArc<T>>) -> Self {
> +        // SAFETY: The type invariants of `Arc` guarantee that the =
data is pinned.
> +        unsafe { Pin::into_inner_unchecked(item).inner }
> +    }
> +}
> +
> /// A borrowed reference to an [`Arc`] instance.
> ///
> /// For cases when one doesn't ever need to increment the refcount on =
the allocation, it is simpler
> @@ -328,3 +342,137 @@ impl<T: ?Sized> Deref for ArcBorrow<'_, T> {
>         unsafe { &self.inner.as_ref().data }
>     }
> }
> +
> +/// A refcounted object that is known to have a refcount of 1.
> +///
> +/// It is mutable and can be converted to an [`Arc`] so that it can =
be shared.
> +///
> +/// # Invariants
> +///
> +/// `inner` always has a reference count of 1.
> +///
> +/// # Examples
> +///
> +/// In the following example, we make changes to the inner object =
before turning it into an
> +/// `Arc<Test>` object (after which point, it cannot be mutated =
directly). Note that `x.into()`
> +/// cannot fail.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +///     a: u32,
> +///     b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +///     let mut x =3D UniqueArc::try_new(Example { a: 10, b: 20 })?;
> +///     x.a +=3D 1;
> +///     x.b +=3D 1;
> +///     Ok(x.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the following example we first allocate memory for a =
ref-counted `Example` but we don't
> +/// initialise it on allocation. We do initialise it later with a =
call to [`UniqueArc::write`],
> +/// followed by a conversion to `Arc<Example>`. This is particularly =
useful when allocation happens
> +/// in one context (e.g., sleepable) and initialisation in another =
(e.g., atomic):
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +///     a: u32,
> +///     b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +///     let x =3D UniqueArc::try_new_uninit()?;
> +///     Ok(x.write(Example { a: 10, b: 20 }).into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +///
> +/// In the last example below, the caller gets a pinned instance of =
`Example` while converting to
> +/// `Arc<Example>`; this is useful in scenarios where one needs a =
pinned reference during
> +/// initialisation, for example, when initialising fields that are =
wrapped in locks.
> +///
> +/// ```
> +/// use kernel::sync::{Arc, UniqueArc};
> +///
> +/// struct Example {
> +///     a: u32,
> +///     b: u32,
> +/// }
> +///
> +/// fn test() -> Result<Arc<Example>> {
> +///     let mut pinned =3D Pin::from(UniqueArc::try_new(Example { a: =
10, b: 20 })?);
> +///     // We can modify `pinned` because it is `Unpin`.
> +///     pinned.as_mut().a +=3D 1;
> +///     Ok(pinned.into())
> +/// }
> +///
> +/// # test().unwrap();
> +/// ```
> +pub struct UniqueArc<T: ?Sized> {
> +    inner: Arc<T>,
> +}
> +
> +impl<T> UniqueArc<T> {
> +    /// Tries to allocate a new [`UniqueArc`] instance.
> +    pub fn try_new(value: T) -> Result<Self> {
> +        Ok(Self {
> +            // INVARIANT: The newly-created object has a ref-count of =
1.
> +            inner: Arc::try_new(value)?,
> +        })
> +    }
> +
> +    /// Tries to allocate a new [`UniqueArc`] instance whose contents =
are not initialised yet.
> +    pub fn try_new_uninit() -> Result<UniqueArc<MaybeUninit<T>>> {
> +        Ok(UniqueArc::<MaybeUninit<T>> {
> +            // INVARIANT: The newly-created object has a ref-count of =
1.
> +            inner: Arc::try_new(MaybeUninit::uninit())?,
> +        })
> +    }
> +}
> +
> +impl<T> UniqueArc<MaybeUninit<T>> {
> +    /// Converts a `UniqueArc<MaybeUninit<T>>` into a `UniqueArc<T>` =
by writing a value into it.
> +    pub fn write(mut self, value: T) -> UniqueArc<T> {
> +        self.deref_mut().write(value);
> +        let inner =3D ManuallyDrop::new(self).inner.ptr;
> +        UniqueArc {
> +            // SAFETY: The new `Arc` is taking over `ptr` from =
`self.inner` (which won't be
> +            // dropped). The types are compatible because =
`MaybeUninit<T>` is compatible with `T`.
> +            inner: unsafe { Arc::from_inner(inner.cast()) },
> +        }
> +    }
> +}
> +
> +impl<T: ?Sized> From<UniqueArc<T>> for Pin<UniqueArc<T>> {
> +    fn from(obj: UniqueArc<T>) -> Self {
> +        // SAFETY: It is not possible to move/replace `T` inside a =
`Pin<UniqueArc<T>>` (unless `T`

Minor nit:  `Pin<UniqueArc<T>>` in this comment should just be =
`UniqueArc<T>`.

> +        // is `Unpin`), so it is ok to convert it to =
`Pin<UniqueArc<T>>`.
> +        unsafe { Pin::new_unchecked(obj) }
> +    }
> +}
> +
> +impl<T: ?Sized> Deref for UniqueArc<T> {
> +    type Target =3D T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        self.inner.deref()
> +    }
> +}
> +
> +impl<T: ?Sized> DerefMut for UniqueArc<T> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {
> +        // SAFETY: By the `Arc` type invariant, there is necessarily =
a reference to the object, so
> +        // it is safe to dereference it. Additionally, we know there =
is only one reference when
> +        // it's inside a `UniqueArc`, so it is safe to get a mutable =
reference.
> +        unsafe { &mut self.inner.ptr.as_mut().data }
> +    }
> +}
> --=20
> 2.34.1
>=20
>=20

=E2=80=94 Laine Taffin Altman