Received-SPF: pass (google.com: domain of linux-kernel+bounces-131764-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 04 Apr 2024 15:47:49 +0000
To: Alice Ryhl <aliceryhl@google.com>, Miguel Ojeda <ojeda@kernel.org>, Andrew Morton <akpm@linux-foundation.org>
From: Benno Lossin <benno.lossin@proton.me>
Cc: Alex Gaynor <alex.gaynor@gmail.com>, Wedson Almeida Filho <wedsonaf@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>, Andreas Hindborg <a.hindborg@samsung.com>, Marco Elver <elver@google.com>, Kees Cook <keescook@chromium.org>, Coly Li <colyli@suse.de>, Paolo Abeni <pabeni@redhat.com>, Pierre Gondois <pierre.gondois@arm.com>, Ingo Molnar <mingo@kernel.org>, Jakub Kicinski <kuba@kernel.org>, Wei Yang <richard.weiyang@gmail.com>, Matthew Wilcox <willy@infradead.org>, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH 9/9] rust: list: add ListArcField
Message-ID: <86ac172a-4ad5-4cc5-844e-7b928d0765bd@proton.me>
In-Reply-To: <20240402-linked-list-v1-9-b1c59ba7ae3b@google.com>
References: <20240402-linked-list-v1-0-b1c59ba7ae3b@google.com> <20240402-linked-list-v1-9-b1c59ba7ae3b@google.com>
Feedback-ID: 71780778:user:proton
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On 02.04.24 14:17, Alice Ryhl wrote:
> One way to explain what `ListArc` does is that it controls exclusive
> access to the prev/next pointer field in a refcounted object. The
> feature of having a special reference to a refcounted object with
> exclusive access to specific fields is useful for other things, so
> provide a general utility for that.
>=20
> This is used by Rust Binder to keep track of which processes have a
> reference to a given node. This involves an object for each process/node
> pair, that is referenced by both the process and the node. For some
> fields in this object, only the process's reference needs to access
> them (and it needs mutable access), so Binder uses a ListArc to give the
> process's reference exclusive access.

This pattern screams "field projection" :)

>=20
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/list.rs           |  3 ++
>  rust/kernel/list/arc_field.rs | 94 +++++++++++++++++++++++++++++++++++++=
++++++
>  2 files changed, 97 insertions(+)
>=20
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index 68d03b100863..a59b35b67e9b 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -23,6 +23,9 @@
>      impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryN=
ewListArc,
>  };
>=20
> +mod arc_field;
> +pub use self::arc_field::{define_list_arc_field_getter, ListArcField};
> +
>  /// A linked list.
>  ///
>  /// All elements in this linked list will be [`ListArc`] references to t=
he value. Since a value can
> diff --git a/rust/kernel/list/arc_field.rs b/rust/kernel/list/arc_field.r=
s
> new file mode 100644
> index 000000000000..936fd97bc5ac
> --- /dev/null
> +++ b/rust/kernel/list/arc_field.rs
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! A field that is exclusively owned by a [`ListArc`].
> +//!
> +//! This can be used to have reference counted struct where one of the r=
eference counted pointers
> +//! has exclusive access to a field of the struct.
> +//!
> +//! [`ListArc`]: crate::list::ListArc
> +
> +use core::cell::UnsafeCell;
> +
> +/// A field owned by a specific `ListArc`.

Missing doc link.

> +pub struct ListArcField<T, const ID: u64 =3D 0> {
> +    value: UnsafeCell<T>,
> +}
> +
> +// SAFETY: If the inner type is thread-safe, then it's also okay for `Li=
stArc` to be thread-safe.
> +unsafe impl<T: Send + Sync, const ID: u64> Send for ListArcField<T, ID> =
{}
> +// SAFETY: If the inner type is thread-safe, then it's also okay for `Li=
stArc` to be thread-safe.
> +unsafe impl<T: Send + Sync, const ID: u64> Sync for ListArcField<T, ID> =
{}
> +
> +impl<T, const ID: u64> ListArcField<T, ID> {
> +    /// Creates a new `ListArcField`.
> +    pub fn new(value: T) -> Self {
> +        Self {
> +            value: UnsafeCell::new(value),
> +        }
> +    }
> +
> +    /// Access the value when we have exclusive access to the `ListArcFi=
eld`.
> +    ///
> +    /// This allows access to the field using an `UniqueArc` instead of =
a `ListArc`.
> +    pub fn get_mut(&mut self) -> &mut T {
> +        self.value.get_mut()
> +    }
> +
> +    /// Unsafely assert that you have shared access to the `ListArc` for=
 this field.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must have shared access to the `ListArc<ID>` containi=
ng the struct with this
> +    /// field for the duration of the returned reference.
> +    pub unsafe fn assert_ref(&self) -> &T {
> +        // SAFETY: The caller has shared access to the `ListArc`, so the=
y also have shared access
> +        // to this field.
> +        unsafe { &*self.value.get() }
> +    }
> +
> +    /// Unsafely assert that you have mutable access to the `ListArc` fo=
r this field.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must have mutable access to the `ListArc<ID>` contain=
ing the struct with this
> +    /// field for the duration of the returned reference.
> +    #[allow(clippy::mut_from_ref)]
> +    pub unsafe fn assert_mut(&self) -> &mut T {
> +        // SAFETY: The caller has exclusive access to the `ListArc`, so =
they also have exclusive
> +        // access to this field.
> +        unsafe { &mut *self.value.get() }
> +    }
> +}
> +
> +/// Defines.

Missing docs.

--=20
Cheers,
Benno

> +#[macro_export]
> +macro_rules! define_list_arc_field_getter {
> +    ($pub:vis fn $name:ident(&self $(<$id:tt>)?) -> &$typ:ty { $field:id=
ent }
> +     $($rest:tt)*
> +    ) =3D> {
> +        $pub fn $name<'a>(self: &'a $crate::list::ListArc<Self $(, $id)?=
>) -> &'a $typ {
> +            let field =3D &(&**self).$field;
> +            // SAFETY: We have a shared reference to the `ListArc`.
> +            unsafe { $crate::list::ListArcField::<$typ $(, $id)?>::asser=
t_ref(field) }
> +        }
> +
> +        $crate::list::define_list_arc_field_getter!($($rest)*);
> +    };
> +
> +    ($pub:vis fn $name:ident(&mut self $(<$id:tt>)?) -> &mut $typ:ty { $=
field:ident }
> +     $($rest:tt)*
> +    ) =3D> {
> +        $pub fn $name<'a>(self: &'a mut $crate::list::ListArc<Self $(, $=
id)?>) -> &'a mut $typ {
> +            let field =3D &(&**self).$field;
> +            // SAFETY: We have a mutable reference to the `ListArc`.
> +            unsafe { $crate::list::ListArcField::<$typ $(, $id)?>::asser=
t_mut(field) }
> +        }
> +
> +        $crate::list::define_list_arc_field_getter!($($rest)*);
> +    };
> +
> +    () =3D> {};
> +}
> +pub use define_list_arc_field_getter;
>=20
> --
> 2.44.0.478.gd926399ef9-goog
>=20