Re: [PATCH 9/9] rust: list: add ListArcField

[Benno Lossin]

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.
> 
> 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" :)

> 
> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> ---
>  rust/kernel/list.rs           |  3 ++
>  rust/kernel/list/arc_field.rs | 94 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+)
> 
> 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, TryNewListArc,
>  };
> 
> +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 the value. Since a value can
> diff --git a/rust/kernel/list/arc_field.rs b/rust/kernel/list/arc_field.rs
> 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 reference 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 = 0> {
> +    value: UnsafeCell<T>,
> +}
> +
> +// SAFETY: If the inner type is thread-safe, then it's also okay for `ListArc` 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 `ListArc` 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 `ListArcField`.
> +    ///
> +    /// 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>` containing 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 they also have shared access
> +        // to this field.
> +        unsafe { &*self.value.get() }
> +    }
> +
> +    /// Unsafely assert that you have mutable access to the `ListArc` for this field.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must have mutable access to the `ListArc<ID>` containing 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.

-- 
Cheers,
Benno

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