* [PATCH v2 1/9] rust: list: add ListArc
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 9:25 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 2/9] rust: list: add tracking for ListArc Alice Ryhl
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
The `ListArc` type can be thought of as a special reference to a
refcounted object that owns the permission to manipulate the
`next`/`prev` pointers stored in the refcounted object. By ensuring that
each object has only one `ListArc` reference, the owner of that
reference is assured exclusive access to the `next`/`prev` pointers.
When a `ListArc` is inserted into a `List`, the `List` takes ownership
of the `ListArc` reference.
There are various strategies for ensuring that a value has only one
`ListArc` reference. The simplest is to convert a `UniqueArc` into a
`ListArc`. However, the refcounted object could also keep track of
whether a `ListArc` exists using a boolean, which could allow for the
creation of new `ListArc` references from an `Arc` reference. Whatever
strategy is used, the relevant tracking is referred to as "the tracking
inside `T`", and the `ListArcSafe` trait (and its subtraits) are used to
update the tracking when a `ListArc` is created or destroyed.
Note that we allow the case where the tracking inside `T` thinks that a
`ListArc` exists, but actually, there isn't a `ListArc`. However, we do
not allow the opposite situation where a `ListArc` exists, but the
tracking thinks it doesn't. This is because the former can at most
result in us failing to create a `ListArc` when the operation could
succeed, whereas the latter can result in the creation of two `ListArc`
references.
This patch introduces the `impl_list_arc_safe!` macro that allows you to
implement `ListArcSafe` for types using the strategy where a `ListArc`
can only be created from a `UniqueArc`. Other strategies are introduced
in later patches.
This is part of the linked list that Rust Binder will use for many
different things. The strategy where a `ListArc` can only be created
from a `UniqueArc` is actually sufficient for most of the objects that
Rust Binder needs to insert into linked lists. Usually, these are todo
items that are created and then immediately inserted into a queue.
The const generic ID allows objects to have several prev/next pointer
pairs so that the same object can be inserted into several different
lists. You are able to have several `ListArc` references as long as they
correspond to different pointer pairs. The ID itself is purely a
compile-time concept and will not be present in the final binary. Both
the `List` and the `ListArc` will need to agree on the ID for them to
work together. Rust Binder uses this in a few places (e.g. death
recipients) where the same object can be inserted into both generic todo
lists and some other lists for tracking the status of the object.
The ID is a const generic rather than a type parameter because the
`pair_from_unique` method needs to be able to assert that the two ids
are different. There's no easy way to assert that when using types
instead of integers.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/list.rs | 8 ++
rust/kernel/list/arc.rs | 315 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 324 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 9a943d99c71a..51d007e031b2 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -33,6 +33,7 @@
pub mod ioctl;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
+pub mod list;
#[cfg(CONFIG_NET)]
pub mod net;
pub mod prelude;
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
new file mode 100644
index 000000000000..fb16ea43b2ba
--- /dev/null
+++ b/rust/kernel/list.rs
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! A linked list implementation.
+
+mod arc;
+pub use self::arc::{impl_list_arc_safe, ListArc, ListArcSafe};
diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs
new file mode 100644
index 000000000000..560ff07fe9b7
--- /dev/null
+++ b/rust/kernel/list/arc.rs
@@ -0,0 +1,315 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! A wrapper around `Arc` for linked lists.
+
+use crate::alloc::{AllocError, Flags};
+use crate::prelude::*;
+use crate::sync::{Arc, ArcBorrow, UniqueArc};
+use core::marker::Unsize;
+use core::ops::Deref;
+use core::pin::Pin;
+
+/// Declares that this type has some way to ensure that there is exactly one `ListArc` instance for
+/// this id.
+pub trait ListArcSafe<const ID: u64 = 0> {
+ /// Informs the tracking inside this type that it now has a [`ListArc`] reference.
+ ///
+ /// This method may be called even if the tracking inside this type thinks that a `ListArc`
+ /// reference exists. (But only if that's not actually the case.)
+ ///
+ /// # Safety
+ ///
+ /// Must not be called if a [`ListArc`] already exist for this value.
+ unsafe fn on_create_list_arc_from_unique(self: Pin<&mut Self>);
+
+ /// Informs the tracking inside this type that there is no [`ListArc`] reference anymore.
+ ///
+ /// # Safety
+ ///
+ /// Must only be called if there is no [`ListArc`] reference, but the tracking thinks there is.
+ unsafe fn on_drop_list_arc(&self);
+}
+
+/// Declares that this type supports [`ListArc`].
+///
+/// When using this macro, it will only be possible to create a [`ListArc`] from a [`UniqueArc`].
+#[macro_export]
+macro_rules! impl_list_arc_safe {
+ (impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty { untracked; } $($rest:tt)*) => {
+ impl$(<$($generics)*>)? $crate::list::ListArcSafe<$num> for $t {
+ unsafe fn on_create_list_arc_from_unique(self: ::core::pin::Pin<&mut Self>) {}
+ unsafe fn on_drop_list_arc(&self) {}
+ }
+ $crate::list::impl_list_arc_safe! { $($rest)* }
+ };
+
+ () => {};
+}
+pub use impl_list_arc_safe;
+
+/// A wrapper around [`Arc`] that's guaranteed unique for the given id.
+///
+/// The `ListArc` type can be thought of as a special reference to a refcounted object that owns the
+/// permission to manipulate the `next`/`prev` pointers stored in the refcounted object. By ensuring
+/// that each object has only one `ListArc` reference, the owner of that reference is assured
+/// exclusive access to the `next`/`prev` pointers. When a `ListArc` is inserted into a `List`, the
+/// `List` takes ownership of the `ListArc` reference.
+///
+/// There are various strategies to ensuring that a value has only one `ListArc` reference. The
+/// simplest is to convert a [`UniqueArc`] into a `ListArc`. However, the refcounted object could
+/// also keep track of whether a `ListArc` exists using a boolean, which could allow for the
+/// creation of new `ListArc` references from an [`Arc`] reference. Whatever strategy is used, the
+/// relevant tracking is referred to as "the tracking inside `T`", and the [`ListArcSafe`] trait
+/// (and its subtraits) are used to update the tracking when a `ListArc` is created or destroyed.
+///
+/// Note that we allow the case where the tracking inside `T` thinks that a `ListArc` exists, but
+/// actually, there isn't a `ListArc`. However, we do not allow the opposite situation where a
+/// `ListArc` exists, but the tracking thinks it doesn't. This is because the former can at most
+/// result in us failing to create a `ListArc` when the operation could succeed, whereas the latter
+/// can result in the creation of two `ListArc` references.
+///
+/// # Invariants
+///
+/// * Each reference counted object has at most one `ListArc` for each value of `ID`.
+/// * The tracking inside `T` is aware that a `ListArc` reference exists.
+#[repr(transparent)]
+pub struct ListArc<T, const ID: u64 = 0>
+where
+ T: ListArcSafe<ID> + ?Sized,
+{
+ arc: Arc<T>,
+}
+
+impl<T: ListArcSafe<ID>, const ID: u64> ListArc<T, ID> {
+ /// Constructs a new reference counted instance of `T`.
+ #[inline]
+ pub fn new(contents: T, flags: Flags) -> Result<Self, AllocError> {
+ Ok(Self::from_unique(UniqueArc::new(contents, flags)?))
+ }
+
+ /// Use the given initializer to in-place initialize a `T`.
+ ///
+ /// If `T: !Unpin` it will not be able to move afterwards.
+ // We don't implement `InPlaceInit` because `ListArc` is implicitly pinned. This is similar to
+ // what we do for `Arc`.
+ #[inline]
+ pub fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self, E>
+ where
+ E: From<AllocError>,
+ {
+ Ok(Self::from_pin_unique(UniqueArc::try_pin_init(init, flags)?))
+ }
+
+ /// Use the given initializer to in-place initialize a `T`.
+ ///
+ /// This is equivalent to [`ListArc<T>::pin_init`], since a [`ListArc`] is always pinned.
+ #[inline]
+ pub fn init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
+ where
+ E: From<AllocError>,
+ {
+ Ok(Self::from_unique(UniqueArc::try_init(init, flags)?))
+ }
+}
+
+impl<T, const ID: u64> ListArc<T, ID>
+where
+ T: ListArcSafe<ID> + ?Sized,
+{
+ /// Convert a [`UniqueArc`] into a [`ListArc`].
+ #[inline]
+ pub fn from_unique(unique: UniqueArc<T>) -> Self {
+ Self::from_pin_unique(Pin::from(unique))
+ }
+
+ /// Convert a pinned [`UniqueArc`] into a [`ListArc`].
+ #[inline]
+ pub fn from_pin_unique(mut unique: Pin<UniqueArc<T>>) -> Self {
+ // SAFETY: We have a `UniqueArc`, so there is no `ListArc`.
+ unsafe { T::on_create_list_arc_from_unique(unique.as_mut()) };
+ let arc = Arc::from(unique);
+ // SAFETY: We just called `on_create_list_arc_from_unique` on an arc without a `ListArc`,
+ // so we can create a `ListArc`.
+ unsafe { Self::transmute_from_arc(arc) }
+ }
+
+ /// Like [`from_unique`], but creates two `ListArcs`.
+ ///
+ /// The two ids must be different.
+ ///
+ /// [`from_unique`]: ListArc::from_unique
+ #[inline]
+ pub fn pair_from_unique<const ID2: u64>(unique: UniqueArc<T>) -> (Self, ListArc<T, ID2>)
+ where
+ T: ListArcSafe<ID2>,
+ {
+ Self::pair_from_pin_unique(Pin::from(unique))
+ }
+
+ /// Like [`pair_from_unique`], but uses a pinned arc.
+ ///
+ /// The two ids must be different.
+ ///
+ /// [`pair_from_unique`]: ListArc::pair_from_unique
+ #[inline]
+ pub fn pair_from_pin_unique<const ID2: u64>(
+ mut unique: Pin<UniqueArc<T>>,
+ ) -> (Self, ListArc<T, ID2>)
+ where
+ T: ListArcSafe<ID2>,
+ {
+ build_assert!(ID != ID2);
+
+ // SAFETY: We have a `UniqueArc`, so there is no `ListArc`.
+ unsafe { <T as ListArcSafe<ID>>::on_create_list_arc_from_unique(unique.as_mut()) };
+ // SAFETY: We have a `UniqueArc`, so there is no `ListArc`.
+ unsafe { <T as ListArcSafe<ID2>>::on_create_list_arc_from_unique(unique.as_mut()) };
+
+ let arc1 = Arc::from(unique);
+ let arc2 = Arc::clone(&arc1);
+
+ // SAFETY: We just called `on_create_list_arc_from_unique` on an arc without a `ListArc`
+ // for both IDs (which are different), so we can create two `ListArc`s.
+ unsafe {
+ (
+ Self::transmute_from_arc(arc1),
+ ListArc::transmute_from_arc(arc2),
+ )
+ }
+ }
+
+ /// Transmutes an [`Arc`] into a `ListArc` without updating the tracking inside `T`.
+ ///
+ /// # Safety
+ ///
+ /// * The value must not already have a `ListArc` reference.
+ /// * The tracking inside `T` must think that there is a `ListArc` reference.
+ #[inline]
+ unsafe fn transmute_from_arc(arc: Arc<T>) -> Self {
+ // INVARIANT: By the safety requirements, the invariants on `ListArc` are satisfied.
+ Self { arc }
+ }
+
+ /// Transmutes a `ListArc` into an [`Arc`] without updating the tracking inside `T`.
+ ///
+ /// After this call, the tracking inside `T` will still think that there is a `ListArc`
+ /// reference.
+ #[inline]
+ fn transmute_to_arc(self) -> Arc<T> {
+ // SAFETY: ListArc is repr(transparent).
+ unsafe { core::mem::transmute(self) }
+ }
+
+ /// Convert ownership of this `ListArc` into a raw pointer.
+ ///
+ /// The returned pointer is indistinguishable from pointers returned by [`Arc::into_raw`]. The
+ /// tracking inside `T` will still think that a `ListArc` exists after this call.
+ #[inline]
+ pub fn into_raw(self) -> *const T {
+ Arc::into_raw(Self::transmute_to_arc(self))
+ }
+
+ /// Take ownership of the `ListArc` from a raw pointer.
+ ///
+ /// # Safety
+ ///
+ /// * `ptr` must satisfy the safety requirements of [`Arc::from_raw`].
+ /// * The value must not already have a `ListArc` reference.
+ /// * The tracking inside `T` must think that there is a `ListArc` reference.
+ #[inline]
+ pub unsafe fn from_raw(ptr: *const T) -> Self {
+ // SAFETY: The pointer satisfies the safety requirements for `Arc::from_raw`.
+ let arc = unsafe { Arc::from_raw(ptr) };
+ // SAFETY: The value doesn't already have a `ListArc` reference, but the tracking thinks it
+ // does.
+ unsafe { Self::transmute_from_arc(arc) }
+ }
+
+ /// Converts the `ListArc` into an [`Arc`].
+ #[inline]
+ pub fn into_arc(self) -> Arc<T> {
+ let arc = Self::transmute_to_arc(self);
+ // SAFETY: There is no longer a `ListArc`, but the tracking thinks there is.
+ unsafe { T::on_drop_list_arc(&arc) };
+ arc
+ }
+
+ /// Clone a `ListArc` into an [`Arc`].
+ #[inline]
+ pub fn clone_arc(&self) -> Arc<T> {
+ self.arc.clone()
+ }
+
+ /// Returns a reference to an [`Arc`] from the given [`ListArc`].
+ ///
+ /// This is useful when the argument of a function call is an [`&Arc`] (e.g., in a method
+ /// receiver), but we have a [`ListArc`] instead.
+ ///
+ /// [`&Arc`]: Arc
+ #[inline]
+ pub fn as_arc(&self) -> &Arc<T> {
+ &self.arc
+ }
+
+ /// Returns an [`ArcBorrow`] from the given [`ListArc`].
+ ///
+ /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
+ /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
+ #[inline]
+ pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
+ self.arc.as_arc_borrow()
+ }
+
+ /// Compare whether two [`ListArc`] pointers reference the same underlying object.
+ #[inline]
+ pub fn ptr_eq(this: &Self, other: &Self) -> bool {
+ Arc::ptr_eq(&this.arc, &other.arc)
+ }
+}
+
+impl<T, const ID: u64> Deref for ListArc<T, ID>
+where
+ T: ListArcSafe<ID> + ?Sized,
+{
+ type Target = T;
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ self.arc.deref()
+ }
+}
+
+impl<T, const ID: u64> Drop for ListArc<T, ID>
+where
+ T: ListArcSafe<ID> + ?Sized,
+{
+ #[inline]
+ fn drop(&mut self) {
+ // SAFETY: There is no longer a `ListArc`, but the tracking thinks there is by the type
+ // invariants on `Self`.
+ unsafe { T::on_drop_list_arc(&self.arc) };
+ }
+}
+
+// This is to allow [`ListArc`] (and variants) to be used as the type of `self`.
+impl<T, const ID: u64> core::ops::Receiver for ListArc<T, ID> where T: ListArcSafe<ID> + ?Sized {}
+
+// This is to allow coercion from `ListArc<T>` to `ListArc<U>` if `T` can be converted to the
+// dynamically-sized type (DST) `U`.
+impl<T, U, const ID: u64> core::ops::CoerceUnsized<ListArc<U, ID>> for ListArc<T, ID>
+where
+ T: ListArcSafe<ID> + Unsize<U> + ?Sized,
+ U: ListArcSafe<ID> + ?Sized,
+{
+}
+
+// This is to allow `ListArc<U>` to be dispatched on when `ListArc<T>` can be coerced into
+// `ListArc<U>`.
+impl<T, U, const ID: u64> core::ops::DispatchFromDyn<ListArc<U, ID>> for ListArc<T, ID>
+where
+ T: ListArcSafe<ID> + Unsize<U> + ?Sized,
+ U: ListArcSafe<ID> + ?Sized,
+{
+}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 1/9] rust: list: add ListArc
2024-05-06 9:53 ` [PATCH v2 1/9] rust: list: add ListArc Alice Ryhl
@ 2024-05-27 9:25 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 9:25 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> +impl<T, const ID: u64> ListArc<T, ID>
> +where
> + T: ListArcSafe<ID> + ?Sized,
> +{
> + /// Convert a [`UniqueArc`] into a [`ListArc`].
> + #[inline]
> + pub fn from_unique(unique: UniqueArc<T>) -> Self {
> + Self::from_pin_unique(Pin::from(unique))
> + }
> +
> + /// Convert a pinned [`UniqueArc`] into a [`ListArc`].
> + #[inline]
> + pub fn from_pin_unique(mut unique: Pin<UniqueArc<T>>) -> Self {
> + // SAFETY: We have a `UniqueArc`, so there is no `ListArc`.
> + unsafe { T::on_create_list_arc_from_unique(unique.as_mut()) };
> + let arc = Arc::from(unique);
> + // SAFETY: We just called `on_create_list_arc_from_unique` on an arc without a `ListArc`,
> + // so we can create a `ListArc`.
> + unsafe { Self::transmute_from_arc(arc) }
> + }
I think these two functions would make sense as `From` impls.
> +
> + /// Like [`from_unique`], but creates two `ListArcs`.
> + ///
> + /// The two ids must be different.
> + ///
> + /// [`from_unique`]: ListArc::from_unique
> + #[inline]
> + pub fn pair_from_unique<const ID2: u64>(unique: UniqueArc<T>) -> (Self, ListArc<T, ID2>)
> + where
> + T: ListArcSafe<ID2>,
> + {
> + Self::pair_from_pin_unique(Pin::from(unique))
> + }
[...]
> + /// Returns a reference to an [`Arc`] from the given [`ListArc`].
> + ///
> + /// This is useful when the argument of a function call is an [`&Arc`] (e.g., in a method
> + /// receiver), but we have a [`ListArc`] instead.
> + ///
> + /// [`&Arc`]: Arc
> + #[inline]
> + pub fn as_arc(&self) -> &Arc<T> {
> + &self.arc
> + }
Should this be an `AsRef` impl instead?
---
Cheers,
Benno
> +
> + /// Returns an [`ArcBorrow`] from the given [`ListArc`].
> + ///
> + /// This is useful when the argument of a function call is an [`ArcBorrow`] (e.g., in a method
> + /// receiver), but we have an [`Arc`] instead. Getting an [`ArcBorrow`] is free when optimised.
> + #[inline]
> + pub fn as_arc_borrow(&self) -> ArcBorrow<'_, T> {
> + self.arc.as_arc_borrow()
> + }
[...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/9] rust: list: add tracking for ListArc
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
2024-05-06 9:53 ` [PATCH v2 1/9] rust: list: add ListArc Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 9:39 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 3/9] rust: list: add struct with prev/next pointers Alice Ryhl
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
Add the ability to track whether a ListArc exists for a given value,
allowing for the creation of ListArcs without going through UniqueArc.
The `impl_list_arc_safe!` macro is extended with a `tracked_by` strategy
that defers the tracking of ListArcs to a field of the struct.
Additionally, the AtomicListArcTracker type is introduced, which can
track whether a ListArc exists using an atomic. By deferring the
tracking to a field of type AtomicListArcTracker, structs gain the
ability to create ListArcs without going through a UniqueArc.
Rust Binder uses this for some objects where we want to be able to
insert them into a linked list at any time. Using the
AtomicListArcTracker, we are able to check whether an item is already in
the list, and if not, we can create a `ListArc` and push it.
The macro has the ability to defer the tracking of ListArcs to a field,
using whatever strategy that field has. Since we don't add any
strategies other than AtomicListArcTracker, another similar option would
be to hard-code that the field should be an AtomicListArcTracker.
However, Rust Binder has a case where the AtomicListArcTracker is not
stored directly in the struct, but in a sub-struct. Furthermore, the
outer struct is generic:
struct Wrapper<T: ?Sized> {
links: ListLinks,
inner: T,
}
Here, the Wrapper struct implements ListArcSafe with `tracked_by inner`,
and then the various types used with `inner` also uses the macro to
implement ListArcSafe. Some of them use the untracked strategy, and some
of them use tracked_by with an AtomicListArcTracker. This way, Wrapper
just inherits whichever choice `inner` has made.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 4 +-
rust/kernel/list/arc.rs | 154 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 155 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index fb16ea43b2ba..c5caa0f6105c 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -5,4 +5,6 @@
//! A linked list implementation.
mod arc;
-pub use self::arc::{impl_list_arc_safe, ListArc, ListArcSafe};
+pub use self::arc::{
+ impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
+};
diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs
index 560ff07fe9b7..4c2e33f40597 100644
--- a/rust/kernel/list/arc.rs
+++ b/rust/kernel/list/arc.rs
@@ -7,9 +7,10 @@
use crate::alloc::{AllocError, Flags};
use crate::prelude::*;
use crate::sync::{Arc, ArcBorrow, UniqueArc};
-use core::marker::Unsize;
+use core::marker::{PhantomPinned, Unsize};
use core::ops::Deref;
use core::pin::Pin;
+use core::sync::atomic::{AtomicBool, Ordering};
/// Declares that this type has some way to ensure that there is exactly one `ListArc` instance for
/// this id.
@@ -32,9 +33,24 @@ pub trait ListArcSafe<const ID: u64 = 0> {
unsafe fn on_drop_list_arc(&self);
}
+/// Declares that this type is able to safely attempt to create `ListArc`s at any time.
+///
+/// # Safety
+///
+/// Implementers must ensure that `try_new_list_arc` does not return `true` if a `ListArc` already
+/// exists.
+pub unsafe trait TryNewListArc<const ID: u64 = 0>: ListArcSafe<ID> {
+ /// Attempts to convert an `Arc<Self>` into an `ListArc<Self>`. Returns `true` if the
+ /// conversion was successful.
+ fn try_new_list_arc(&self) -> bool;
+}
+
/// Declares that this type supports [`ListArc`].
///
-/// When using this macro, it will only be possible to create a [`ListArc`] from a [`UniqueArc`].
+/// When using this macro, you may choose between the `untracked` strategy where it is not tracked
+/// whether a [`ListArc`] exists, and the `tracked_by` strategy where the tracking is deferred to a
+/// field of the struct. The `tracked_by` strategy can be combined with a field of type
+/// [`AtomicListArcTracker`] to track whether a [`ListArc`] exists.
#[macro_export]
macro_rules! impl_list_arc_safe {
(impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty { untracked; } $($rest:tt)*) => {
@@ -45,6 +61,37 @@ unsafe fn on_drop_list_arc(&self) {}
$crate::list::impl_list_arc_safe! { $($rest)* }
};
+ (impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty {
+ tracked_by $field:ident : $fty:ty;
+ } $($rest:tt)*) => {
+ impl$(<$($generics)*>)? $crate::list::ListArcSafe<$num> for $t {
+ unsafe fn on_create_list_arc_from_unique(self: ::core::pin::Pin<&mut Self>) {
+ // SAFETY: This field is structurally pinned.
+ let field = unsafe {
+ ::core::pin::Pin::map_unchecked_mut(self, |me| &mut me.$field)
+ };
+ // SAFETY: The caller promises that there is no `ListArc`.
+ unsafe {
+ <$fty as $crate::list::ListArcSafe<$num>>::on_create_list_arc_from_unique(field)
+ };
+ }
+ unsafe fn on_drop_list_arc(&self) {
+ // SAFETY: The caller promises that there is no `ListArc` reference, and also
+ // promises that the tracking thinks there is a `ListArc` reference.
+ unsafe { <$fty as $crate::list::ListArcSafe<$num>>::on_drop_list_arc(&self.$field) };
+ }
+ }
+ unsafe impl$(<$($generics)*>)? $crate::list::TryNewListArc<$num> for $t
+ where
+ $fty: TryNewListArc<$num>,
+ {
+ fn try_new_list_arc(&self) -> bool {
+ <$fty as $crate::list::TryNewListArc<$num>>::try_new_list_arc(&self.field)
+ }
+ }
+ $crate::list::impl_list_arc_safe! { $($rest)* }
+ };
+
() => {};
}
pub use impl_list_arc_safe;
@@ -180,6 +227,52 @@ pub fn pair_from_pin_unique<const ID2: u64>(
}
}
+ /// Try to create a new `ListArc`.
+ ///
+ /// This fails if this value already has a `ListArc`.
+ pub fn try_from_arc(arc: Arc<T>) -> Result<Self, Arc<T>>
+ where
+ T: TryNewListArc<ID>,
+ {
+ if arc.try_new_list_arc() {
+ // SAFETY: The `try_new_list_arc` method returned true, so the tracking now thinks that
+ // a `ListArc` exists, so we can create one.
+ Ok(unsafe { Self::transmute_from_arc(arc) })
+ } else {
+ Err(arc)
+ }
+ }
+
+ /// Try to create a new `ListArc`.
+ ///
+ /// This fails if this value already has a `ListArc`.
+ pub fn try_from_arc_borrow(arc: ArcBorrow<'_, T>) -> Option<Self>
+ where
+ T: TryNewListArc<ID>,
+ {
+ if arc.try_new_list_arc() {
+ // SAFETY: The `try_new_list_arc` method returned true, so the tracking now thinks that
+ // a `ListArc` exists, so we can create one.
+ Some(unsafe { Self::transmute_from_arc(Arc::from(arc)) })
+ } else {
+ None
+ }
+ }
+
+ /// Try to create a new `ListArc`.
+ ///
+ /// If it's not possible to create a new `ListArc`, then the `Arc` is dropped. This will never
+ /// run the destructor of the value.
+ pub fn try_from_arc_or_drop(arc: Arc<T>) -> Option<Self>
+ where
+ T: TryNewListArc<ID>,
+ {
+ match Self::try_from_arc(arc) {
+ Ok(list_arc) => Some(list_arc),
+ Err(arc) => Arc::into_unique_or_drop(arc).map(Self::from_pin_unique),
+ }
+ }
+
/// Transmutes an [`Arc`] into a `ListArc` without updating the tracking inside `T`.
///
/// # Safety
@@ -313,3 +406,60 @@ impl<T, U, const ID: u64> core::ops::DispatchFromDyn<ListArc<U, ID>> for ListArc
U: ListArcSafe<ID> + ?Sized,
{
}
+
+/// A utility for tracking whether a [`ListArc`] exists using an atomic.
+///
+/// # Invariant
+///
+/// If the boolean is `false`, then there is no [`ListArc`] for this value.
+#[repr(transparent)]
+pub struct AtomicListArcTracker<const ID: u64 = 0> {
+ inner: AtomicBool,
+ _pin: PhantomPinned,
+}
+
+impl<const ID: u64> AtomicListArcTracker<ID> {
+ /// Creates a new initializer for this type.
+ pub fn new() -> impl PinInit<Self> {
+ // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
+ // not be constructed in an `Arc` that already has a `ListArc`.
+ Self {
+ inner: AtomicBool::new(false),
+ _pin: PhantomPinned,
+ }
+ }
+
+ fn project_inner(self: Pin<&mut Self>) -> &mut AtomicBool {
+ // SAFETY: The `inner` field is not structurally pinned, so we may obtain a mutable
+ // reference to it even if we only have a pinned reference to `self`.
+ unsafe { &mut Pin::into_inner_unchecked(self).inner }
+ }
+}
+
+impl<const ID: u64> ListArcSafe<ID> for AtomicListArcTracker<ID> {
+ unsafe fn on_create_list_arc_from_unique(self: Pin<&mut Self>) {
+ // INVARIANT: We just created a ListArc, so the boolean should be true.
+ *self.project_inner().get_mut() = true;
+ }
+
+ unsafe fn on_drop_list_arc(&self) {
+ // INVARIANT: We just dropped a ListArc, so the boolean should be false.
+ self.inner.store(false, Ordering::Release);
+ }
+}
+
+// SAFETY: If this method returns `true`, then by the type invariant there is no `ListArc` before
+// this call, so it is okay to create a new `ListArc`.
+//
+// The acquire ordering will synchronize with the release store from the destruction of any
+// previous `ListArc`, so if there was a previous `ListArc`, then the destruction of the previous
+// `ListArc` happens-before the creation of the new `ListArc`.
+unsafe impl<const ID: u64> TryNewListArc<ID> for AtomicListArcTracker<ID> {
+ fn try_new_list_arc(&self) -> bool {
+ // INVARIANT: If this method returns true, then the boolean used to be false, and is no
+ // longer false, so it is okay for the caller to create a new [`ListArc`].
+ self.inner
+ .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
+ .is_ok()
+ }
+}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 2/9] rust: list: add tracking for ListArc
2024-05-06 9:53 ` [PATCH v2 2/9] rust: list: add tracking for ListArc Alice Ryhl
@ 2024-05-27 9:39 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 9:39 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> @@ -32,9 +33,24 @@ pub trait ListArcSafe<const ID: u64 = 0> {
> unsafe fn on_drop_list_arc(&self);
> }
>
> +/// Declares that this type is able to safely attempt to create `ListArc`s at any time.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that `try_new_list_arc` does not return `true` if a `ListArc` already
> +/// exists.
> +pub unsafe trait TryNewListArc<const ID: u64 = 0>: ListArcSafe<ID> {
> + /// Attempts to convert an `Arc<Self>` into an `ListArc<Self>`. Returns `true` if the
> + /// conversion was successful.
> + fn try_new_list_arc(&self) -> bool;
> +}
> +
> /// Declares that this type supports [`ListArc`].
> ///
> -/// When using this macro, it will only be possible to create a [`ListArc`] from a [`UniqueArc`].
> +/// When using this macro, you may choose between the `untracked` strategy where it is not tracked
> +/// whether a [`ListArc`] exists, and the `tracked_by` strategy where the tracking is deferred to a
> +/// field of the struct. The `tracked_by` strategy can be combined with a field of type
> +/// [`AtomicListArcTracker`] to track whether a [`ListArc`] exists.
I think it would make sense to use bullet points here.
Also, you should mention that in the `tracked_by` strategy, the field is
required to implement `TryNewListArc`.
> #[macro_export]
> macro_rules! impl_list_arc_safe {
> (impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty { untracked; } $($rest:tt)*) => {
> @@ -45,6 +61,37 @@ unsafe fn on_drop_list_arc(&self) {}
> $crate::list::impl_list_arc_safe! { $($rest)* }
> };
>
> + (impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty {
> + tracked_by $field:ident : $fty:ty;
> + } $($rest:tt)*) => {
> + impl$(<$($generics)*>)? $crate::list::ListArcSafe<$num> for $t {
> + unsafe fn on_create_list_arc_from_unique(self: ::core::pin::Pin<&mut Self>) {
> + // SAFETY: This field is structurally pinned.
Who ensures this? This is not documented on the macro.
The only way that I see to fix this would be to make the `tracked_by`
strategy `unsafe`. At least until we implement proper structural pinning
of fields.
> + let field = unsafe {
> + ::core::pin::Pin::map_unchecked_mut(self, |me| &mut me.$field)
> + };
> + // SAFETY: The caller promises that there is no `ListArc`.
> + unsafe {
> + <$fty as $crate::list::ListArcSafe<$num>>::on_create_list_arc_from_unique(field)
> + };
> + }
> + unsafe fn on_drop_list_arc(&self) {
> + // SAFETY: The caller promises that there is no `ListArc` reference, and also
> + // promises that the tracking thinks there is a `ListArc` reference.
> + unsafe { <$fty as $crate::list::ListArcSafe<$num>>::on_drop_list_arc(&self.$field) };
> + }
> + }
> + unsafe impl$(<$($generics)*>)? $crate::list::TryNewListArc<$num> for $t
> + where
> + $fty: TryNewListArc<$num>,
> + {
> + fn try_new_list_arc(&self) -> bool {
> + <$fty as $crate::list::TryNewListArc<$num>>::try_new_list_arc(&self.field)
> + }
> + }
> + $crate::list::impl_list_arc_safe! { $($rest)* }
> + };
> +
> () => {};
> }
> pub use impl_list_arc_safe;
[...]
> @@ -313,3 +406,60 @@ impl<T, U, const ID: u64> core::ops::DispatchFromDyn<ListArc<U, ID>> for ListArc
> U: ListArcSafe<ID> + ?Sized,
> {
> }
> +
> +/// A utility for tracking whether a [`ListArc`] exists using an atomic.
> +///
> +/// # Invariant
> +///
> +/// If the boolean is `false`, then there is no [`ListArc`] for this value.
"If `inner` is `false`, ..."
---
Cheers,
Benno
> +#[repr(transparent)]
> +pub struct AtomicListArcTracker<const ID: u64 = 0> {
> + inner: AtomicBool,
> + _pin: PhantomPinned,
> +}
[...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/9] rust: list: add struct with prev/next pointers
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
2024-05-06 9:53 ` [PATCH v2 1/9] rust: list: add ListArc Alice Ryhl
2024-05-06 9:53 ` [PATCH v2 2/9] rust: list: add tracking for ListArc Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 9:58 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 4/9] rust: list: add macro for implementing ListItem Alice Ryhl
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
Define the ListLinks struct, which wraps the prev/next pointers that
will be used to insert values into a List in a future patch. Also
define the ListItem trait, which is implemented by structs that have a
ListLinks field.
The ListItem trait provides four different methods that are all
essentially container_of or the reverse of container_of. Two of them are
used before inserting/after removing an item from the list, and the two
others are used when looking at a value without changing whether it is
in a list. This distinction is introduced because it is needed for the
patch that adds support for heterogeneous lists, which are implemented
by adding a third pointer field with a fat pointer to the full struct.
When inserting into the heterogeneous list, the pointer-to-self is
updated to have the right vtable, and the container_of operation is
implemented by just returning that pointer instead of using the real
container_of operation.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index c5caa0f6105c..b5cfbb96a392 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -4,7 +4,123 @@
//! A linked list implementation.
+use crate::init::PinInit;
+use crate::types::Opaque;
+use core::ptr;
+
mod arc;
pub use self::arc::{
impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
};
+
+/// Implemented by types where a [`ListArc<Self>`] can be inserted into a `List`.
+///
+/// # Safety
+///
+/// Implementers must ensure that they provide the guarantees documented on the three methods
+/// below.
+///
+/// [`ListArc<Self>`]: ListArc
+pub unsafe trait ListItem<const ID: u64 = 0>: ListArcSafe<ID> {
+ /// Views the [`ListLinks`] for this value.
+ ///
+ /// # Guarantees
+ ///
+ /// If there is a previous call to `prepare_to_insert` and there is no call to `post_remove`
+ /// since the most recent such call, then this returns the same pointer as the one returned by
+ /// the most recent call to `prepare_to_insert`.
+ ///
+ /// Otherwise, the returned pointer points at a read-only [`ListLinks`] with two null pointers.
+ ///
+ /// # Safety
+ ///
+ /// The provided pointer must point at a valid value. (It need not be in an `Arc`.)
+ unsafe fn view_links(me: *const Self) -> *mut ListLinks<ID>;
+
+ /// View the full value given its [`ListLinks`] field.
+ ///
+ /// Can only be used when the value is in a list.
+ ///
+ /// # Guarantees
+ ///
+ /// * Returns the same pointer as the one passed to the previous call to `prepare_to_insert`.
+ /// * The returned pointer is valid until the next call to `post_remove`.
+ ///
+ /// # Safety
+ ///
+ /// * The provided pointer must originate from the previous call to `prepare_to_insert`, or
+ /// from a call to `view_links` that happened after the previous call to `prepare_to_insert`.
+ /// * Since the previous call to `prepare_to_insert`, the `post_remove` method must not have
+ /// been called.
+ unsafe fn view_value(me: *mut ListLinks<ID>) -> *const Self;
+
+ /// This is called when an item is inserted into a `List`.
+ ///
+ /// # Guarantees
+ ///
+ /// The caller is granted exclusive access to the returned [`ListLinks`] until `post_remove` is
+ /// called.
+ ///
+ /// # Safety
+ ///
+ /// * The provided pointer must point at a valid value in an [`Arc`].
+ /// * Calls to `prepare_to_insert` and `post_remove` on the same value must alternate.
+ /// * The caller must own the [`ListArc`] for this value.
+ /// * The caller must not give up ownership of the [`ListArc`] unless `post_remove` has been
+ /// called after this call to `prepare_to_insert`.
+ ///
+ /// [`Arc`]: crate::sync::Arc
+ unsafe fn prepare_to_insert(me: *const Self) -> *mut ListLinks<ID>;
+
+ /// This undoes a previous call to `prepare_to_insert`.
+ ///
+ /// # Guarantees
+ ///
+ /// The returned pointer is the pointer that was originally passed to `prepare_to_insert`.
+ ///
+ /// The caller is free to recreate the `ListArc` after this call.
+ ///
+ /// # Safety
+ ///
+ /// The provided pointer must be the pointer returned by the previous call to
+ /// `prepare_to_insert`.
+ unsafe fn post_remove(me: *mut ListLinks<ID>) -> *const Self;
+}
+
+#[repr(C)]
+#[derive(Copy, Clone)]
+struct ListLinksFields {
+ next: *mut ListLinksFields,
+ prev: *mut ListLinksFields,
+}
+
+/// The prev/next pointers for an item in a linked list.
+///
+/// # Invariants
+///
+/// The fields are null if and only if this item is not in a list.
+#[repr(transparent)]
+pub struct ListLinks<const ID: u64 = 0> {
+ #[allow(dead_code)]
+ inner: Opaque<ListLinksFields>,
+}
+
+// SAFETY: The next/prev fields of a ListLinks can be moved across thread boundaries.
+unsafe impl<const ID: u64> Send for ListLinks<ID> {}
+// SAFETY: The type is opaque so immutable references to a ListLinks are useless. Therefore, it's
+// okay to have immutable access to a ListLinks from several threads at once.
+unsafe impl<const ID: u64> Sync for ListLinks<ID> {}
+
+impl<const ID: u64> ListLinks<ID> {
+ /// Creates a new initializer for this type.
+ pub fn new() -> impl PinInit<Self> {
+ // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
+ // not be constructed in an `Arc` that already has a `ListArc`.
+ ListLinks {
+ inner: Opaque::new(ListLinksFields {
+ prev: ptr::null_mut(),
+ next: ptr::null_mut(),
+ }),
+ }
+ }
+}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/9] rust: list: add struct with prev/next pointers
2024-05-06 9:53 ` [PATCH v2 3/9] rust: list: add struct with prev/next pointers Alice Ryhl
@ 2024-05-27 9:58 ` Benno Lossin
2024-05-27 11:42 ` Alice Ryhl
0 siblings, 1 reply; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 9:58 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> Define the ListLinks struct, which wraps the prev/next pointers that
> will be used to insert values into a List in a future patch. Also
> define the ListItem trait, which is implemented by structs that have a
> ListLinks field.
>
> The ListItem trait provides four different methods that are all
> essentially container_of or the reverse of container_of. Two of them are
> used before inserting/after removing an item from the list, and the two
> others are used when looking at a value without changing whether it is
> in a list. This distinction is introduced because it is needed for the
> patch that adds support for heterogeneous lists, which are implemented
> by adding a third pointer field with a fat pointer to the full struct.
> When inserting into the heterogeneous list, the pointer-to-self is
> updated to have the right vtable, and the container_of operation is
> implemented by just returning that pointer instead of using the real
> container_of operation.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/list.rs | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 116 insertions(+)
>
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index c5caa0f6105c..b5cfbb96a392 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -4,7 +4,123 @@
>
> //! A linked list implementation.
>
> +use crate::init::PinInit;
> +use crate::types::Opaque;
> +use core::ptr;
> +
> mod arc;
> pub use self::arc::{
> impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
> };
> +
> +/// Implemented by types where a [`ListArc<Self>`] can be inserted into a `List`.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that they provide the guarantees documented on the three methods
I would not mention the number of methods, since it is difficult to
maintain and doesn't actually provide any value (it already is incorrect :)
> +/// below.
> +///
> +/// [`ListArc<Self>`]: ListArc
> +pub unsafe trait ListItem<const ID: u64 = 0>: ListArcSafe<ID> {
> + /// Views the [`ListLinks`] for this value.
> + ///
> + /// # Guarantees
> + ///
> + /// If there is a previous call to `prepare_to_insert` and there is no call to `post_remove`
> + /// since the most recent such call, then this returns the same pointer as the one returned by
> + /// the most recent call to `prepare_to_insert`.
> + ///
> + /// Otherwise, the returned pointer points at a read-only [`ListLinks`] with two null pointers.
> + ///
> + /// # Safety
> + ///
> + /// The provided pointer must point at a valid value. (It need not be in an `Arc`.)
> + unsafe fn view_links(me: *const Self) -> *mut ListLinks<ID>;
> +
> + /// View the full value given its [`ListLinks`] field.
> + ///
> + /// Can only be used when the value is in a list.
> + ///
> + /// # Guarantees
> + ///
> + /// * Returns the same pointer as the one passed to the previous call to `prepare_to_insert`.
> + /// * The returned pointer is valid until the next call to `post_remove`.
> + ///
> + /// # Safety
> + ///
> + /// * The provided pointer must originate from the previous call to `prepare_to_insert`, or
> + /// from a call to `view_links` that happened after the previous call to `prepare_to_insert`.
> + /// * Since the previous call to `prepare_to_insert`, the `post_remove` method must not have
> + /// been called.
> + unsafe fn view_value(me: *mut ListLinks<ID>) -> *const Self;
> +
> + /// This is called when an item is inserted into a `List`.
> + ///
> + /// # Guarantees
> + ///
> + /// The caller is granted exclusive access to the returned [`ListLinks`] until `post_remove` is
> + /// called.
> + ///
> + /// # Safety
> + ///
> + /// * The provided pointer must point at a valid value in an [`Arc`].
> + /// * Calls to `prepare_to_insert` and `post_remove` on the same value must alternate.
Are there any synchronization requirements? Am I allowed to call
`prepare_to_insert` and `post_remove` on different threads without
synchronizing?
> + /// * The caller must own the [`ListArc`] for this value.
> + /// * The caller must not give up ownership of the [`ListArc`] unless `post_remove` has been
> + /// called after this call to `prepare_to_insert`.
> + ///
> + /// [`Arc`]: crate::sync::Arc
> + unsafe fn prepare_to_insert(me: *const Self) -> *mut ListLinks<ID>;
> +
> + /// This undoes a previous call to `prepare_to_insert`.
> + ///
> + /// # Guarantees
> + ///
> + /// The returned pointer is the pointer that was originally passed to `prepare_to_insert`.
> + ///
> + /// The caller is free to recreate the `ListArc` after this call.
As I read the requirements on `prepare_to_insert`, the caller is not
required to deconstruct the `ListArc`. For example the caller is allowed
to `clone_arc()` and then `into_raw()` and then pass that pointer to
`prepare_to_insert`.
So I would just remove this sentence.
> + ///
> + /// # Safety
> + ///
> + /// The provided pointer must be the pointer returned by the previous call to
Does "most recent call" make more sense? I find previous call a bit
weird. (also in the requirements above)
---
Cheers,
Benno
> + /// `prepare_to_insert`.
> + unsafe fn post_remove(me: *mut ListLinks<ID>) -> *const Self;
> +}
> +
> +#[repr(C)]
> +#[derive(Copy, Clone)]
> +struct ListLinksFields {
> + next: *mut ListLinksFields,
> + prev: *mut ListLinksFields,
> +}
> +
> +/// The prev/next pointers for an item in a linked list.
> +///
> +/// # Invariants
> +///
> +/// The fields are null if and only if this item is not in a list.
> +#[repr(transparent)]
> +pub struct ListLinks<const ID: u64 = 0> {
> + #[allow(dead_code)]
> + inner: Opaque<ListLinksFields>,
> +}
> +
> +// SAFETY: The next/prev fields of a ListLinks can be moved across thread boundaries.
> +unsafe impl<const ID: u64> Send for ListLinks<ID> {}
> +// SAFETY: The type is opaque so immutable references to a ListLinks are useless. Therefore, it's
> +// okay to have immutable access to a ListLinks from several threads at once.
> +unsafe impl<const ID: u64> Sync for ListLinks<ID> {}
> +
> +impl<const ID: u64> ListLinks<ID> {
> + /// Creates a new initializer for this type.
> + pub fn new() -> impl PinInit<Self> {
> + // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
> + // not be constructed in an `Arc` that already has a `ListArc`.
> + ListLinks {
> + inner: Opaque::new(ListLinksFields {
> + prev: ptr::null_mut(),
> + next: ptr::null_mut(),
> + }),
> + }
> + }
> +}
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v2 3/9] rust: list: add struct with prev/next pointers
2024-05-27 9:58 ` Benno Lossin
@ 2024-05-27 11:42 ` Alice Ryhl
0 siblings, 0 replies; 20+ messages in thread
From: Alice Ryhl @ 2024-05-27 11:42 UTC (permalink / raw)
To: Benno Lossin
Cc: Miguel Ojeda, Andrew Morton, Alex Gaynor, Wedson Almeida Filho,
Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Marco Elver, Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois,
Ingo Molnar, Jakub Kicinski, Wei Yang, Matthew Wilcox,
linux-kernel, rust-for-linux
On Mon, May 27, 2024 at 11:58 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On 06.05.24 11:53, Alice Ryhl wrote:
> > Define the ListLinks struct, which wraps the prev/next pointers that
> > will be used to insert values into a List in a future patch. Also
> > define the ListItem trait, which is implemented by structs that have a
> > ListLinks field.
> >
> > The ListItem trait provides four different methods that are all
> > essentially container_of or the reverse of container_of. Two of them are
> > used before inserting/after removing an item from the list, and the two
> > others are used when looking at a value without changing whether it is
> > in a list. This distinction is introduced because it is needed for the
> > patch that adds support for heterogeneous lists, which are implemented
> > by adding a third pointer field with a fat pointer to the full struct.
> > When inserting into the heterogeneous list, the pointer-to-self is
> > updated to have the right vtable, and the container_of operation is
> > implemented by just returning that pointer instead of using the real
> > container_of operation.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > rust/kernel/list.rs | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 116 insertions(+)
> >
> > diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> > index c5caa0f6105c..b5cfbb96a392 100644
> > --- a/rust/kernel/list.rs
> > +++ b/rust/kernel/list.rs
> > @@ -4,7 +4,123 @@
> >
> > //! A linked list implementation.
> >
> > +use crate::init::PinInit;
> > +use crate::types::Opaque;
> > +use core::ptr;
> > +
> > mod arc;
> > pub use self::arc::{
> > impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
> > };
> > +
> > +/// Implemented by types where a [`ListArc<Self>`] can be inserted into a `List`.
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that they provide the guarantees documented on the three methods
>
> I would not mention the number of methods, since it is difficult to
> maintain and doesn't actually provide any value (it already is incorrect :)
I will remove the mention of a number.
> > + /// This is called when an item is inserted into a `List`.
> > + ///
> > + /// # Guarantees
> > + ///
> > + /// The caller is granted exclusive access to the returned [`ListLinks`] until `post_remove` is
> > + /// called.
> > + ///
> > + /// # Safety
> > + ///
> > + /// * The provided pointer must point at a valid value in an [`Arc`].
> > + /// * Calls to `prepare_to_insert` and `post_remove` on the same value must alternate.
>
> Are there any synchronization requirements? Am I allowed to call
> `prepare_to_insert` and `post_remove` on different threads without
> synchronizing?
No, if you call them at the same time, then they aren't alternating.
> > + /// * The caller must own the [`ListArc`] for this value.
> > + /// * The caller must not give up ownership of the [`ListArc`] unless `post_remove` has been
> > + /// called after this call to `prepare_to_insert`.
> > + ///
> > + /// [`Arc`]: crate::sync::Arc
> > + unsafe fn prepare_to_insert(me: *const Self) -> *mut ListLinks<ID>;
> > +
> > + /// This undoes a previous call to `prepare_to_insert`.
> > + ///
> > + /// # Guarantees
> > + ///
> > + /// The returned pointer is the pointer that was originally passed to `prepare_to_insert`.
> > + ///
> > + /// The caller is free to recreate the `ListArc` after this call.
>
> As I read the requirements on `prepare_to_insert`, the caller is not
> required to deconstruct the `ListArc`. For example the caller is allowed
> to `clone_arc()` and then `into_raw()` and then pass that pointer to
> `prepare_to_insert`.
> So I would just remove this sentence.
I will remove it.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The provided pointer must be the pointer returned by the previous call to
>
> Does "most recent call" make more sense? I find previous call a bit
> weird. (also in the requirements above)
Sure, I can say "most recent".
Alice
> ---
> Cheers,
> Benno
>
> > + /// `prepare_to_insert`.
> > + unsafe fn post_remove(me: *mut ListLinks<ID>) -> *const Self;
> > +}
> > +
> > +#[repr(C)]
> > +#[derive(Copy, Clone)]
> > +struct ListLinksFields {
> > + next: *mut ListLinksFields,
> > + prev: *mut ListLinksFields,
> > +}
> > +
> > +/// The prev/next pointers for an item in a linked list.
> > +///
> > +/// # Invariants
> > +///
> > +/// The fields are null if and only if this item is not in a list.
> > +#[repr(transparent)]
> > +pub struct ListLinks<const ID: u64 = 0> {
> > + #[allow(dead_code)]
> > + inner: Opaque<ListLinksFields>,
> > +}
> > +
> > +// SAFETY: The next/prev fields of a ListLinks can be moved across thread boundaries.
> > +unsafe impl<const ID: u64> Send for ListLinks<ID> {}
> > +// SAFETY: The type is opaque so immutable references to a ListLinks are useless. Therefore, it's
> > +// okay to have immutable access to a ListLinks from several threads at once.
> > +unsafe impl<const ID: u64> Sync for ListLinks<ID> {}
> > +
> > +impl<const ID: u64> ListLinks<ID> {
> > + /// Creates a new initializer for this type.
> > + pub fn new() -> impl PinInit<Self> {
> > + // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
> > + // not be constructed in an `Arc` that already has a `ListArc`.
> > + ListLinks {
> > + inner: Opaque::new(ListLinksFields {
> > + prev: ptr::null_mut(),
> > + next: ptr::null_mut(),
> > + }),
> > + }
> > + }
> > +}
> >
> > --
> > 2.45.0.rc1.225.g2a3ae87e7f-goog
> >
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 4/9] rust: list: add macro for implementing ListItem
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
` (2 preceding siblings ...)
2024-05-06 9:53 ` [PATCH v2 3/9] rust: list: add struct with prev/next pointers Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 10:06 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 5/9] rust: list: add List Alice Ryhl
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
Adds a macro for safely implementing the ListItem trait. As part of the
implementation of the macro, we also provide a HasListLinks trait
similar to the workqueue's HasWorkItem trait.
The HasListLinks trait is only necessary if you are implementing
ListItem using the impl_list_item macro.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 3 ++
rust/kernel/list/impl_list_item_mod.rs | 99 ++++++++++++++++++++++++++++++++++
2 files changed, 102 insertions(+)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index b5cfbb96a392..f2eca542e090 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -8,6 +8,9 @@
use crate::types::Opaque;
use core::ptr;
+mod impl_list_item_mod;
+pub use self::impl_list_item_mod::{impl_has_list_links, impl_list_item, HasListLinks};
+
mod arc;
pub use self::arc::{
impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
new file mode 100644
index 000000000000..3ff483be89d1
--- /dev/null
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 Google LLC.
+
+//! Helpers for implementing list traits safely.
+
+use crate::list::ListLinks;
+
+/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
+///
+/// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented
+/// manually, then this trait is not needed.
+///
+/// # Safety
+///
+/// All values of this type must have a `ListLinks<ID>` field at the given offset.
+pub unsafe trait HasListLinks<const ID: u64 = 0> {
+ /// The offset of the `ListLinks` field.
+ const OFFSET: usize;
+
+ /// Returns a pointer to the [`ListLinks<T, ID>`] field.
+ ///
+ /// # Safety
+ ///
+ /// The provided pointer must point at a valid struct of type `Self`.
+ ///
+ /// [`ListLinks<T, ID>`]: ListLinks
+ // We don't really need this method, but it's necessary for the implementation of
+ // `impl_has_work!` to be correct.
+ #[inline]
+ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
+ // SAFETY: The caller promises that the pointer is valid. The implementer promises that the
+ // `OFFSET` constant is correct.
+ unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
+ }
+}
+
+/// Implements the [`HasListLinks`] trait for the given type.
+#[macro_export]
+macro_rules! impl_has_list_links {
+ ($(impl$(<$($implarg:ident),*>)?
+ HasListLinks$(<$id:tt>)?
+ for $self:ident $(<$($selfarg:ty),*>)?
+ { self$(.$field:ident)* }
+ )*) => {$(
+ // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
+ // right type.
+ unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for
+ $self $(<$($selfarg),*>)?
+ {
+ const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
+
+ #[inline]
+ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
+ // SAFETY: The caller promises that the pointer is not dangling.
+ unsafe {
+ ::core::ptr::addr_of_mut!((*ptr)$(.$field)*)
+ }
+ }
+ }
+ )*};
+}
+pub use impl_has_list_links;
+
+/// Implements the [`ListItem`] trait for the given type.
+///
+/// Assumes that the type implements [`HasListLinks`].
+///
+/// [`ListItem`]: crate::list::ListItem
+#[macro_export]
+macro_rules! impl_list_item {
+ (
+ impl$({$($generics:tt)*})? $crate::list::ListItem<$num:tt> for $t:ty {
+ using ListLinks;
+ } $($rest:tt)*
+ ) => {
+ unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
+ unsafe {
+ <Self as $crate::list::HasListLinks<$num>>::raw_get_list_links(me.cast_mut())
+ }
+ }
+
+ unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
+ let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
+ unsafe { (me as *const u8).sub(offset) as *const Self }
+ }
+
+ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
+ unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) }
+ }
+
+ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
+ unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
+ }
+ }
+ };
+}
+pub use impl_list_item;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 4/9] rust: list: add macro for implementing ListItem
2024-05-06 9:53 ` [PATCH v2 4/9] rust: list: add macro for implementing ListItem Alice Ryhl
@ 2024-05-27 10:06 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 10:06 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> Adds a macro for safely implementing the ListItem trait. As part of the
> implementation of the macro, we also provide a HasListLinks trait
> similar to the workqueue's HasWorkItem trait.
>
> The HasListLinks trait is only necessary if you are implementing
> ListItem using the impl_list_item macro.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/list.rs | 3 ++
> rust/kernel/list/impl_list_item_mod.rs | 99 ++++++++++++++++++++++++++++++++++
> 2 files changed, 102 insertions(+)
>
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index b5cfbb96a392..f2eca542e090 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -8,6 +8,9 @@
> use crate::types::Opaque;
> use core::ptr;
>
> +mod impl_list_item_mod;
> +pub use self::impl_list_item_mod::{impl_has_list_links, impl_list_item, HasListLinks};
> +
> mod arc;
> pub use self::arc::{
> impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
> diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> new file mode 100644
> index 000000000000..3ff483be89d1
> --- /dev/null
> +++ b/rust/kernel/list/impl_list_item_mod.rs
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 Google LLC.
> +
> +//! Helpers for implementing list traits safely.
> +
> +use crate::list::ListLinks;
> +
> +/// Declares that this type has a `ListLinks<ID>` field at a fixed offset.
> +///
> +/// This trait is only used to help implement `ListItem` safely. If `ListItem` is implemented
> +/// manually, then this trait is not needed.
> +///
> +/// # Safety
> +///
> +/// All values of this type must have a `ListLinks<ID>` field at the given offset.
> +pub unsafe trait HasListLinks<const ID: u64 = 0> {
> + /// The offset of the `ListLinks` field.
> + const OFFSET: usize;
> +
> + /// Returns a pointer to the [`ListLinks<T, ID>`] field.
> + ///
> + /// # Safety
> + ///
> + /// The provided pointer must point at a valid struct of type `Self`.
> + ///
> + /// [`ListLinks<T, ID>`]: ListLinks
> + // We don't really need this method, but it's necessary for the implementation of
> + // `impl_has_work!` to be correct.
You could also check for correctness within the expansion of
`impl_has_list_links` (by creating an invisible function there), so you
don't need this function. It still could be useful for `dyn Trait`
though.
> + #[inline]
> + unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
> + // SAFETY: The caller promises that the pointer is valid. The implementer promises that the
> + // `OFFSET` constant is correct.
> + unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
> + }
> +}
> +
> +/// Implements the [`HasListLinks`] trait for the given type.
> +#[macro_export]
> +macro_rules! impl_has_list_links {
> + ($(impl$(<$($implarg:ident),*>)?
> + HasListLinks$(<$id:tt>)?
> + for $self:ident $(<$($selfarg:ty),*>)?
> + { self$(.$field:ident)* }
> + )*) => {$(
> + // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
> + // right type.
> + unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for
> + $self $(<$($selfarg),*>)?
> + {
> + const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize;
AFAIK only a single field is supported on stable.
> +
> + #[inline]
> + unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
> + // SAFETY: The caller promises that the pointer is not dangling.
> + unsafe {
> + ::core::ptr::addr_of_mut!((*ptr)$(.$field)*)
Note that this runs into the `Box` issue, but `offset_of` will fail to
compile in that case, so it is fine.
Do we want to note this in a comment?
> + }
> + }
> + }
> + )*};
> +}
> +pub use impl_has_list_links;
> +
> +/// Implements the [`ListItem`] trait for the given type.
> +///
> +/// Assumes that the type implements [`HasListLinks`].
> +///
> +/// [`ListItem`]: crate::list::ListItem
> +#[macro_export]
> +macro_rules! impl_list_item {
> + (
> + impl$({$($generics:tt)*})? $crate::list::ListItem<$num:tt> for $t:ty {
> + using ListLinks;
> + } $($rest:tt)*
> + ) => {
> + unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
Missing SAFETY comment.
---
Cheers,
Benno
> + unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> + unsafe {
> + <Self as $crate::list::HasListLinks<$num>>::raw_get_list_links(me.cast_mut())
> + }
> + }
> +
> + unsafe fn view_value(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> + let offset = <Self as $crate::list::HasListLinks<$num>>::OFFSET;
> + unsafe { (me as *const u8).sub(offset) as *const Self }
> + }
> +
> + unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> + unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) }
> + }
> +
> + unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> + unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
> + }
> + }
> + };
> +}
> +pub use impl_list_item;
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 5/9] rust: list: add List
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
` (3 preceding siblings ...)
2024-05-06 9:53 ` [PATCH v2 4/9] rust: list: add macro for implementing ListItem Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 10:25 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 6/9] rust: list: add iterators Alice Ryhl
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
Add the actual linked list itself.
The linked list uses the following design: The List type itself just has
a single pointer to the first element of the list. And the actual list
items then form a cycle. So the last item is `first->prev`.
This is slightly different from the usual kernel linked list. Matching
that exactly would amount to giving List two pointers, and having it be
part of the cycle of items. This alternate design has the advantage that
the cycle is never completely empty, which can reduce the number of
branches in some cases. However, it also has the disadvantage that List
must be pinned, which this design is trying to avoid.
Having the list items form a cycle rather than having null pointers at
the beginning/end is convenient for several reasons. For one, it lets us
store only one pointer in List, and it simplifies the implementation of
several functions.
Unfortunately, the `remove` function that removes an arbitrary element
from the list has to be unsafe. This is needed because there is no way
to handle the case where you pass an element from the wrong list. For
example, if it is the first element of some other list, then that other
list's `first` pointer would not be updated. Similarly, it could be a
data race if you try to remove it from two different lists in parallel.
(There's no problem with passing `remove` an item that's not in any
list. Additionally, other removal methods such as `pop_front` need not
be unsafe, as they can't be used to remove items from another list.)
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 329 +++++++++++++++++++++++++++++++++++++++++++++++-
rust/kernel/list/arc.rs | 6 +-
2 files changed, 330 insertions(+), 5 deletions(-)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index f2eca542e090..d0ff29a3e5d1 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -6,6 +6,7 @@
use crate::init::PinInit;
use crate::types::Opaque;
+use core::marker::PhantomData;
use core::ptr;
mod impl_list_item_mod;
@@ -16,7 +17,40 @@
impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
};
-/// Implemented by types where a [`ListArc<Self>`] can be inserted into a `List`.
+/// A linked list.
+///
+/// All elements in this linked list will be [`ListArc`] references to the value. Since a value can
+/// only have one `ListArc` (for each pair of prev/next pointers), this ensures that the same
+/// prev/next pointers are not used for several linked lists.
+///
+/// # Invariants
+///
+/// * If the list is empty, then `first` is null. Otherwise, `first` points at the links field of
+/// the first element in the list.
+/// * All prev/next pointers of items in the list are valid and form a cycle.
+pub struct List<T: ?Sized + ListItem<ID>, const ID: u64 = 0> {
+ first: *mut ListLinksFields,
+ _ty: PhantomData<ListArc<T, ID>>,
+}
+
+// SAFETY: This is a container of `ListArc<T, ID>`, and access to the container allows the same
+// type of access to the `ListArc<T, ID>` elements.
+unsafe impl<T, const ID: u64> Send for List<T, ID>
+where
+ ListArc<T, ID>: Send,
+ T: ?Sized + ListItem<ID>,
+{
+}
+// SAFETY: This is a container of `ListArc<T, ID>`, and access to the container allows the same
+// type of access to the `ListArc<T, ID>` elements.
+unsafe impl<T, const ID: u64> Sync for List<T, ID>
+where
+ ListArc<T, ID>: Sync,
+ T: ?Sized + ListItem<ID>,
+{
+}
+
+/// Implemented by types where a [`ListArc<Self>`] can be inserted into a [`List`].
///
/// # Safety
///
@@ -57,7 +91,7 @@ pub unsafe trait ListItem<const ID: u64 = 0>: ListArcSafe<ID> {
/// been called.
unsafe fn view_value(me: *mut ListLinks<ID>) -> *const Self;
- /// This is called when an item is inserted into a `List`.
+ /// This is called when an item is inserted into a [`List`].
///
/// # Guarantees
///
@@ -104,7 +138,6 @@ struct ListLinksFields {
/// The fields are null if and only if this item is not in a list.
#[repr(transparent)]
pub struct ListLinks<const ID: u64 = 0> {
- #[allow(dead_code)]
inner: Opaque<ListLinksFields>,
}
@@ -126,4 +159,294 @@ pub fn new() -> impl PinInit<Self> {
}),
}
}
+
+ /// # Safety
+ ///
+ /// `me` must be dereferencable.
+ #[inline]
+ unsafe fn fields(me: *mut Self) -> *mut ListLinksFields {
+ // SAFETY: The caller promises that the pointer is valid.
+ unsafe { Opaque::raw_get(ptr::addr_of!((*me).inner)) }
+ }
+
+ /// # Safety
+ ///
+ /// `me` must be dereferencable.
+ #[inline]
+ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
+ me.cast()
+ }
+}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
+ /// Creates a new empty list.
+ pub const fn new() -> Self {
+ Self {
+ first: ptr::null_mut(),
+ _ty: PhantomData,
+ }
+ }
+
+ /// Returns whether this list is empty.
+ pub fn is_empty(&self) -> bool {
+ self.first.is_null()
+ }
+
+ /// Add the provided item to the back of the list.
+ pub fn push_back(&mut self, item: ListArc<T, ID>) {
+ let raw_item = ListArc::into_raw(item);
+ // SAFETY:
+ // * We just got `raw_item` from a `ListArc`, so it's in an `Arc`.
+ // * If this requirement is violated, then the previous caller of `prepare_to_insert`
+ // violated the safety requirement that they can't give up ownership of the `ListArc`
+ // until they call `post_remove`.
+ // * We own the `ListArc`.
+ // * Removing items from this list is always done using `remove_internal_inner`, which
+ // calls `post_remove` before giving up ownership.
+ let list_links = unsafe { T::prepare_to_insert(raw_item) };
+ // SAFETY: We have not yet called `post_remove`, so `list_links` is still valid.
+ let item = unsafe { ListLinks::fields(list_links) };
+
+ if self.first.is_null() {
+ self.first = item;
+ // SAFETY: The caller just gave us ownership of these fields.
+ // INVARIANT: A linked list with one item should be cyclic.
+ unsafe {
+ (*item).next = item;
+ (*item).prev = item;
+ }
+ } else {
+ let next = self.first;
+ // SAFETY: By the type invariant, this pointer is valid or null. We just checked that
+ // it's not null, so it must be valid.
+ let prev = unsafe { (*next).prev };
+ // SAFETY: Pointers in a linked list are never dangling, and the caller just gave us
+ // ownership of the fields on `item`.
+ // INVARIANT: This correctly inserts `item` between `prev` and `next`.
+ unsafe {
+ (*item).next = next;
+ (*item).prev = prev;
+ (*prev).next = item;
+ (*next).prev = item;
+ }
+ }
+ }
+
+ /// Add the provided item to the front of the list.
+ pub fn push_front(&mut self, item: ListArc<T, ID>) {
+ let raw_item = ListArc::into_raw(item);
+ // SAFETY:
+ // * We just got `raw_item` from a `ListArc`, so it's in an `Arc`.
+ // * If this requirement is violated, then the previous caller of `prepare_to_insert`
+ // violated the safety requirement that they can't give up ownership of the `ListArc`
+ // until they call `post_remove`.
+ // * We own the `ListArc`.
+ // * Removing items from this list is always done using `remove_internal_inner`, which
+ // calls `post_remove` before giving up ownership.
+ let list_links = unsafe { T::prepare_to_insert(raw_item) };
+ // SAFETY: We have not yet called `post_remove`, so `list_links` is still valid.
+ let item = unsafe { ListLinks::fields(list_links) };
+
+ if self.first.is_null() {
+ // SAFETY: The caller just gave us ownership of these fields.
+ // INVARIANT: A linked list with one item should be cyclic.
+ unsafe {
+ (*item).next = item;
+ (*item).prev = item;
+ }
+ } else {
+ let next = self.first;
+ // SAFETY: We just checked that `next` is non-null.
+ let prev = unsafe { (*next).prev };
+ // SAFETY: Pointers in a linked list are never dangling, and the caller just gave us
+ // ownership of the fields on `item`.
+ // INVARIANT: This correctly inserts `item` between `prev` and `next`.
+ unsafe {
+ (*item).next = next;
+ (*item).prev = prev;
+ (*prev).next = item;
+ (*next).prev = item;
+ }
+ }
+ self.first = item;
+ }
+
+ /// Removes the last item from this list.
+ pub fn pop_back(&mut self) -> Option<ListArc<T, ID>> {
+ if self.first.is_null() {
+ return None;
+ }
+
+ // SAFETY: We just checked that the list is not empty.
+ let last = unsafe { (*self.first).prev };
+ // SAFETY: The last item of this list is in this list.
+ Some(unsafe { self.remove_internal(last) })
+ }
+
+ /// Removes the first item from this list.
+ pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
+ if self.first.is_null() {
+ return None;
+ }
+
+ // SAFETY: The first item of this list is in this list.
+ Some(unsafe { self.remove_internal(self.first) })
+ }
+
+ /// Removes the provided item from this list and returns it.
+ ///
+ /// This returns `None` if the item is not in the list. (Note that by the safety requirements,
+ /// this means that the item is not in any list.)
+ ///
+ /// # Safety
+ ///
+ /// The provided item must not be in a different linked list (with the same id).
+ pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
+ let mut item = unsafe { ListLinks::fields(T::view_links(item)) };
+ // SAFETY: The user provided a reference, and reference are never dangling.
+ //
+ // As for why this is not a data race, there are two cases:
+ //
+ // * If `item` is not in any list, then these fields are read-only and null.
+ // * If `item` is in this list, then we have exclusive access to these fields since we
+ // have a mutable reference to the list.
+ //
+ // In either case, there's no race.
+ let ListLinksFields { next, prev } = unsafe { *item };
+
+ debug_assert_eq!(next.is_null(), prev.is_null());
+ if !next.is_null() {
+ // This is really a no-op, but this ensures that `item` is a raw pointer that was
+ // obtained without going through a pointer->reference->pointer conversion rountrip.
+ // This ensures that the list is valid under the more restrictive strict provenance
+ // ruleset.
+ //
+ // SAFETY: We just checked that `next` is not null, and it's not dangling by the
+ // list invariants.
+ unsafe {
+ debug_assert_eq!(item, (*next).prev);
+ item = (*next).prev;
+ }
+
+ // SAFETY: We just checked that `item` is in a list, so the caller guarantees that it
+ // is in this list. The pointers are in the right order.
+ Some(unsafe { self.remove_internal_inner(item, next, prev) })
+ } else {
+ None
+ }
+ }
+
+ /// Removes the provided item from the list.
+ ///
+ /// # Safety
+ ///
+ /// The pointer must point at an item in this list.
+ unsafe fn remove_internal(&mut self, item: *mut ListLinksFields) -> ListArc<T, ID> {
+ // SAFETY: The caller promises that this pointer is not dangling, and there's no data race
+ // since we have a mutable reference to the list containing `item`.
+ let ListLinksFields { next, prev } = unsafe { *item };
+ // SAFETY: The pointers are ok and in the right order.
+ unsafe { self.remove_internal_inner(item, next, prev) }
+ }
+
+ /// Removes the provided item from the list.
+ ///
+ /// # Safety
+ ///
+ /// The `item` pointer must point at an item in this list, and we must have `(*item).next ==
+ /// next` and `(*item).prev == prev`.
+ unsafe fn remove_internal_inner(
+ &mut self,
+ item: *mut ListLinksFields,
+ next: *mut ListLinksFields,
+ prev: *mut ListLinksFields,
+ ) -> ListArc<T, ID> {
+ // SAFETY: We have exclusive access to the pointers of items in the list, and the prev/next
+ // pointers are always valid for items in a list.
+ //
+ // INVARIANT: There are three cases:
+ // * If the list has at least three items, then after removing the item, `prev` and `next`
+ // will be next to each other.
+ // * If the list has two items, then the remaining item will point at itself.
+ // * If the list has one item, then `next == prev == item`, so these writes have no
+ // effect. The list remains unchanged and `item` is still in the list for now.
+ unsafe {
+ (*next).prev = prev;
+ (*prev).next = next;
+ }
+ // SAFETY: We have exclusive access to items in the list.
+ // INVARIANT: `item` is being removed, so the pointers should be null.
+ unsafe {
+ (*item).prev = ptr::null_mut();
+ (*item).next = ptr::null_mut();
+ }
+ // INVARIANT: There are three cases:
+ // * If `item` was not the first item, then `self.first` should remain unchanged.
+ // * If `item` was the first item and there is another item, then we just updated
+ // `prev->next` to `next`, which is the new first item, and setting `item->next` to null
+ // did not modify `prev->next`.
+ // * If `item` was the only item in the list, then `prev == item`, and we just set
+ // `item->next` to null, so this correctly sets `first` to null now that the list is
+ // empty.
+ if self.first == item {
+ // SAFETY: The `prev` pointer is the value that `item->prev` had when it was in this
+ // list, so it must be valid. There is no race since `prev` is still in the list and we
+ // still have exclusive access to the list.
+ self.first = unsafe { (*prev).next };
+ }
+
+ // SAFETY: `item` used to be in the list, so it is dereferencable by the type invariants
+ // of `List`.
+ let list_links = unsafe { ListLinks::from_fields(item) };
+ // SAFETY: Any pointer in the list originates from a `prepare_to_insert` call.
+ let raw_item = unsafe { T::post_remove(list_links) };
+ // SAFETY: The above call to `post_remove` guarantees that we can recreate the `ListArc`.
+ unsafe { ListArc::from_raw(raw_item) }
+ }
+
+ /// Moves all items from `other` into `self`.
+ ///
+ /// The items of `other` are added to the back of `self`, so the last item of `other` becomes
+ /// the last item of `self`.
+ pub fn push_all_back(&mut self, other: &mut List<T, ID>) {
+ // First, we insert the elements into `self`. At the end, we make `other` empty.
+ if self.is_empty() {
+ // INVARIANT: All of the elements in `other` become elements of `self`.
+ self.first = other.first;
+ } else if !other.is_empty() {
+ let other_first = other.first;
+ // SAFETY: The other list is not empty, so this pointer is valid.
+ let other_last = unsafe { (*other_first).prev };
+ let self_first = self.first;
+ // SAFETY: The self list is not empty, so this pointer is valid.
+ let self_last = unsafe { (*self_first).prev };
+
+ // SAFETY: We have exclusive access to both lists, so we can update the pointers.
+ // INVARIANT: This correctly sets the pointers to merge both lists. We do not need to
+ // update `self.first` because the first element of `self` does not change.
+ unsafe {
+ (*self_first).prev = other_last;
+ (*other_last).next = self_first;
+ (*self_last).next = other_first;
+ (*other_first).prev = self_last;
+ }
+ }
+
+ // INVARIANT: The other list is now empty, so update its pointer.
+ other.first = ptr::null_mut();
+ }
+}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> Default for List<T, ID> {
+ fn default() -> Self {
+ List::new()
+ }
+}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> Drop for List<T, ID> {
+ fn drop(&mut self) {
+ while let Some(item) = self.pop_front() {
+ drop(item);
+ }
+ }
}
diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs
index 4c2e33f40597..180ec8902705 100644
--- a/rust/kernel/list/arc.rs
+++ b/rust/kernel/list/arc.rs
@@ -101,8 +101,8 @@ fn try_new_list_arc(&self) -> bool {
/// The `ListArc` type can be thought of as a special reference to a refcounted object that owns the
/// permission to manipulate the `next`/`prev` pointers stored in the refcounted object. By ensuring
/// that each object has only one `ListArc` reference, the owner of that reference is assured
-/// exclusive access to the `next`/`prev` pointers. When a `ListArc` is inserted into a `List`, the
-/// `List` takes ownership of the `ListArc` reference.
+/// exclusive access to the `next`/`prev` pointers. When a `ListArc` is inserted into a [`List`],
+/// the [`List`] takes ownership of the `ListArc` reference.
///
/// There are various strategies to ensuring that a value has only one `ListArc` reference. The
/// simplest is to convert a [`UniqueArc`] into a `ListArc`. However, the refcounted object could
@@ -121,6 +121,8 @@ fn try_new_list_arc(&self) -> bool {
///
/// * Each reference counted object has at most one `ListArc` for each value of `ID`.
/// * The tracking inside `T` is aware that a `ListArc` reference exists.
+///
+/// [`List`]: crate::list::List
#[repr(transparent)]
pub struct ListArc<T, const ID: u64 = 0>
where
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 5/9] rust: list: add List
2024-05-06 9:53 ` [PATCH v2 5/9] rust: list: add List Alice Ryhl
@ 2024-05-27 10:25 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 10:25 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> Add the actual linked list itself.
>
> The linked list uses the following design: The List type itself just has
> a single pointer to the first element of the list. And the actual list
> items then form a cycle. So the last item is `first->prev`.
>
> This is slightly different from the usual kernel linked list. Matching
> that exactly would amount to giving List two pointers, and having it be
> part of the cycle of items. This alternate design has the advantage that
> the cycle is never completely empty, which can reduce the number of
> branches in some cases. However, it also has the disadvantage that List
> must be pinned, which this design is trying to avoid.
>
> Having the list items form a cycle rather than having null pointers at
> the beginning/end is convenient for several reasons. For one, it lets us
> store only one pointer in List, and it simplifies the implementation of
> several functions.
>
> Unfortunately, the `remove` function that removes an arbitrary element
> from the list has to be unsafe. This is needed because there is no way
> to handle the case where you pass an element from the wrong list. For
> example, if it is the first element of some other list, then that other
> list's `first` pointer would not be updated. Similarly, it could be a
> data race if you try to remove it from two different lists in parallel.
> (There's no problem with passing `remove` an item that's not in any
> list. Additionally, other removal methods such as `pop_front` need not
> be unsafe, as they can't be used to remove items from another list.)
I would also mention that later in this patch series you introduce
cursors for the list, which can be used to safely remove arbitrary items
(although you need to iterate the list).
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/list.rs | 329 +++++++++++++++++++++++++++++++++++++++++++++++-
> rust/kernel/list/arc.rs | 6 +-
> 2 files changed, 330 insertions(+), 5 deletions(-)
>
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index f2eca542e090..d0ff29a3e5d1 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -6,6 +6,7 @@
>
> use crate::init::PinInit;
> use crate::types::Opaque;
> +use core::marker::PhantomData;
> use core::ptr;
>
> mod impl_list_item_mod;
> @@ -16,7 +17,40 @@
> impl_list_arc_safe, AtomicListArcTracker, ListArc, ListArcSafe, TryNewListArc,
> };
>
> -/// Implemented by types where a [`ListArc<Self>`] can be inserted into a `List`.
> +/// A linked list.
> +///
> +/// All elements in this linked list will be [`ListArc`] references to the value. Since a value can
> +/// only have one `ListArc` (for each pair of prev/next pointers), this ensures that the same
> +/// prev/next pointers are not used for several linked lists.
> +///
> +/// # Invariants
> +///
> +/// * If the list is empty, then `first` is null. Otherwise, `first` points at the links field of
> +/// the first element in the list.
> +/// * All prev/next pointers of items in the list are valid and form a cycle.
I think that you additionally need "The list has exclusive access to all
`prev`/`next` pointers of items in the list." or "For every item in the
list, the list owns the associated `ListArc<T, ID>`."
> +pub struct List<T: ?Sized + ListItem<ID>, const ID: u64 = 0> {
> + first: *mut ListLinksFields,
> + _ty: PhantomData<ListArc<T, ID>>,
> +}
[...]
> + /// Add the provided item to the back of the list.
> + pub fn push_back(&mut self, item: ListArc<T, ID>) {
> + let raw_item = ListArc::into_raw(item);
> + // SAFETY:
> + // * We just got `raw_item` from a `ListArc`, so it's in an `Arc`.
> + // * If this requirement is violated, then the previous caller of `prepare_to_insert`
> + // violated the safety requirement that they can't give up ownership of the `ListArc`
> + // until they call `post_remove`.
> + // * We own the `ListArc`.
> + // * Removing items from this list is always done using `remove_internal_inner`, which
> + // calls `post_remove` before giving up ownership.
> + let list_links = unsafe { T::prepare_to_insert(raw_item) };
> + // SAFETY: We have not yet called `post_remove`, so `list_links` is still valid.
> + let item = unsafe { ListLinks::fields(list_links) };
> +
> + if self.first.is_null() {
> + self.first = item;
> + // SAFETY: The caller just gave us ownership of these fields.
> + // INVARIANT: A linked list with one item should be cyclic.
> + unsafe {
> + (*item).next = item;
> + (*item).prev = item;
> + }
> + } else {
> + let next = self.first;
> + // SAFETY: By the type invariant, this pointer is valid or null. We just checked that
> + // it's not null, so it must be valid.
> + let prev = unsafe { (*next).prev };
> + // SAFETY: Pointers in a linked list are never dangling, and the caller just gave us
> + // ownership of the fields on `item`.
Here you need that new invariant: the list needs exclusive access to all
of the `next`/`prev` pointers.
> + // INVARIANT: This correctly inserts `item` between `prev` and `next`.
> + unsafe {
> + (*item).next = next;
> + (*item).prev = prev;
> + (*prev).next = item;
> + (*next).prev = item;
> + }
> + }
> + }
> +
> + /// Add the provided item to the front of the list.
> + pub fn push_front(&mut self, item: ListArc<T, ID>) {
> + let raw_item = ListArc::into_raw(item);
> + // SAFETY:
> + // * We just got `raw_item` from a `ListArc`, so it's in an `Arc`.
> + // * If this requirement is violated, then the previous caller of `prepare_to_insert`
> + // violated the safety requirement that they can't give up ownership of the `ListArc`
> + // until they call `post_remove`.
> + // * We own the `ListArc`.
> + // * Removing items from this list is always done using `remove_internal_inner`, which
> + // calls `post_remove` before giving up ownership.
> + let list_links = unsafe { T::prepare_to_insert(raw_item) };
> + // SAFETY: We have not yet called `post_remove`, so `list_links` is still valid.
> + let item = unsafe { ListLinks::fields(list_links) };
> +
> + if self.first.is_null() {
> + // SAFETY: The caller just gave us ownership of these fields.
> + // INVARIANT: A linked list with one item should be cyclic.
> + unsafe {
> + (*item).next = item;
> + (*item).prev = item;
> + }
> + } else {
> + let next = self.first;
> + // SAFETY: We just checked that `next` is non-null.
> + let prev = unsafe { (*next).prev };
> + // SAFETY: Pointers in a linked list are never dangling, and the caller just gave us
> + // ownership of the fields on `item`.
> + // INVARIANT: This correctly inserts `item` between `prev` and `next`.
> + unsafe {
> + (*item).next = next;
> + (*item).prev = prev;
> + (*prev).next = item;
> + (*next).prev = item;
> + }
> + }
This code is the same as in `push_back`, can you refactor it?
> + self.first = item;
> + }
> +
> + /// Removes the last item from this list.
> + pub fn pop_back(&mut self) -> Option<ListArc<T, ID>> {
> + if self.first.is_null() {
> + return None;
> + }
> +
> + // SAFETY: We just checked that the list is not empty.
> + let last = unsafe { (*self.first).prev };
> + // SAFETY: The last item of this list is in this list.
> + Some(unsafe { self.remove_internal(last) })
> + }
> +
> + /// Removes the first item from this list.
> + pub fn pop_front(&mut self) -> Option<ListArc<T, ID>> {
> + if self.first.is_null() {
> + return None;
> + }
> +
> + // SAFETY: The first item of this list is in this list.
> + Some(unsafe { self.remove_internal(self.first) })
> + }
> +
> + /// Removes the provided item from this list and returns it.
> + ///
> + /// This returns `None` if the item is not in the list. (Note that by the safety requirements,
> + /// this means that the item is not in any list.)
> + ///
> + /// # Safety
> + ///
> + /// The provided item must not be in a different linked list (with the same id).
"`item` must not be ..." also other instances below.
---
Cheers,
Benno
> + pub unsafe fn remove(&mut self, item: &T) -> Option<ListArc<T, ID>> {
> + let mut item = unsafe { ListLinks::fields(T::view_links(item)) };
> + // SAFETY: The user provided a reference, and reference are never dangling.
> + //
> + // As for why this is not a data race, there are two cases:
> + //
> + // * If `item` is not in any list, then these fields are read-only and null.
> + // * If `item` is in this list, then we have exclusive access to these fields since we
> + // have a mutable reference to the list.
> + //
> + // In either case, there's no race.
> + let ListLinksFields { next, prev } = unsafe { *item };
> +
> + debug_assert_eq!(next.is_null(), prev.is_null());
> + if !next.is_null() {
> + // This is really a no-op, but this ensures that `item` is a raw pointer that was
> + // obtained without going through a pointer->reference->pointer conversion rountrip.
> + // This ensures that the list is valid under the more restrictive strict provenance
> + // ruleset.
> + //
> + // SAFETY: We just checked that `next` is not null, and it's not dangling by the
> + // list invariants.
> + unsafe {
> + debug_assert_eq!(item, (*next).prev);
> + item = (*next).prev;
> + }
> +
> + // SAFETY: We just checked that `item` is in a list, so the caller guarantees that it
> + // is in this list. The pointers are in the right order.
> + Some(unsafe { self.remove_internal_inner(item, next, prev) })
> + } else {
> + None
> + }
> + }
[...]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 6/9] rust: list: add iterators
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
` (4 preceding siblings ...)
2024-05-06 9:53 ` [PATCH v2 5/9] rust: list: add List Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 10:31 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 7/9] rust: list: add cursor Alice Ryhl
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
Rust Binder has lists containing stuff such as all contexts or all
processes, and sometimes need to iterate over them. This patch enables
Rust Binder to do that using a normal for loop.
The iterator returns the ArcBorrow type, so it is possible to grab a
refcount to values while iterating.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index d0ff29a3e5d1..e36afc7ee44a 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -5,7 +5,9 @@
//! A linked list implementation.
use crate::init::PinInit;
+use crate::sync::ArcBorrow;
use crate::types::Opaque;
+use core::iter::{DoubleEndedIterator, FusedIterator};
use core::marker::PhantomData;
use core::ptr;
@@ -435,6 +437,17 @@ pub fn push_all_back(&mut self, other: &mut List<T, ID>) {
// INVARIANT: The other list is now empty, so update its pointer.
other.first = ptr::null_mut();
}
+
+ /// Creates an iterator over the list.
+ pub fn iter(&self) -> Iter<'_, T, ID> {
+ // INVARIANT: If the list is empty, both pointers are null. Otherwise, both pointers point
+ // at the first element of the same list.
+ Iter {
+ current: self.first,
+ stop: self.first,
+ _ty: PhantomData,
+ }
+ }
}
impl<T: ?Sized + ListItem<ID>, const ID: u64> Default for List<T, ID> {
@@ -450,3 +463,92 @@ fn drop(&mut self) {
}
}
}
+
+/// An iterator into a [`List`].
+///
+/// # Invariants
+///
+/// * There must be a [`List`] that is immutably borrowed for the duration of `'a`.
+/// * The `current` pointer is null or points at a value in that [`List`].
+/// * The `stop` pointer is equal to the `first` field of the [`List`].
+#[derive(Clone)]
+pub struct Iter<'a, T: ?Sized + ListItem<ID>, const ID: u64 = 0> {
+ current: *mut ListLinksFields,
+ stop: *mut ListLinksFields,
+ _ty: PhantomData<&'a ListArc<T, ID>>,
+}
+
+impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> Iterator for Iter<'a, T, ID> {
+ type Item = ArcBorrow<'a, T>;
+
+ fn next(&mut self) -> Option<ArcBorrow<'a, T>> {
+ if self.current.is_null() {
+ return None;
+ }
+
+ let current = self.current;
+
+ // SAFETY: We just checked that `current` is not null, so it is in a list, and hence not
+ // dangling. There's no race because the iterator holds an immutable borrow to the list.
+ let next = unsafe { (*current).next };
+ // INVARIANT: If `current` was the last element of the list, then this updates it to null.
+ // Otherwise, we update it to the next element.
+ self.current = if next != self.stop {
+ next
+ } else {
+ ptr::null_mut()
+ };
+
+ // SAFETY: The `current` pointer points at a value in the list.
+ let item = unsafe { T::view_value(ListLinks::from_fields(current)) };
+ // SAFETY:
+ // * All values in a list are stored in an `Arc`.
+ // * The value cannot be removed from the list for the duration of the lifetime annotated
+ // on the returned `ArcBorrow`, because removing it from the list would require mutable
+ // access to the list. However, the `ArcBorrow` is annotated with the iterator's
+ // lifetime, and the list is immutably borrowed for that lifetime.
+ // * Values in a list never have a `UniqueArc` reference.
+ Some(unsafe { ArcBorrow::from_raw(item) })
+ }
+}
+
+impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> FusedIterator for Iter<'a, T, ID> {}
+
+impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> IntoIterator for &'a List<T, ID> {
+ type IntoIter = Iter<'a, T, ID>;
+ type Item = ArcBorrow<'a, T>;
+
+ fn into_iter(self) -> Iter<'a, T, ID> {
+ self.iter()
+ }
+}
+
+/// An owning iterator into a [`List`].
+pub struct IntoIter<T: ?Sized + ListItem<ID>, const ID: u64 = 0> {
+ list: List<T, ID>,
+}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> Iterator for IntoIter<T, ID> {
+ type Item = ListArc<T, ID>;
+
+ fn next(&mut self) -> Option<ListArc<T, ID>> {
+ self.list.pop_front()
+ }
+}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> FusedIterator for IntoIter<T, ID> {}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> DoubleEndedIterator for IntoIter<T, ID> {
+ fn next_back(&mut self) -> Option<ListArc<T, ID>> {
+ self.list.pop_back()
+ }
+}
+
+impl<T: ?Sized + ListItem<ID>, const ID: u64> IntoIterator for List<T, ID> {
+ type IntoIter = IntoIter<T, ID>;
+ type Item = ListArc<T, ID>;
+
+ fn into_iter(self) -> IntoIter<T, ID> {
+ IntoIter { list: self }
+ }
+}
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 6/9] rust: list: add iterators
2024-05-06 9:53 ` [PATCH v2 6/9] rust: list: add iterators Alice Ryhl
@ 2024-05-27 10:31 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 10:31 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> Rust Binder has lists containing stuff such as all contexts or all
> processes, and sometimes need to iterate over them. This patch enables
typo: need -> needs
> Rust Binder to do that using a normal for loop.
>
> The iterator returns the ArcBorrow type, so it is possible to grab a
> refcount to values while iterating.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Two documentation nits below, with those fixed:
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> ---
> rust/kernel/list.rs | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 102 insertions(+)
>
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index d0ff29a3e5d1..e36afc7ee44a 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -5,7 +5,9 @@
> //! A linked list implementation.
>
> use crate::init::PinInit;
> +use crate::sync::ArcBorrow;
> use crate::types::Opaque;
> +use core::iter::{DoubleEndedIterator, FusedIterator};
> use core::marker::PhantomData;
> use core::ptr;
>
> @@ -435,6 +437,17 @@ pub fn push_all_back(&mut self, other: &mut List<T, ID>) {
> // INVARIANT: The other list is now empty, so update its pointer.
> other.first = ptr::null_mut();
> }
> +
> + /// Creates an iterator over the list.
> + pub fn iter(&self) -> Iter<'_, T, ID> {
> + // INVARIANT: If the list is empty, both pointers are null. Otherwise, both pointers point
> + // at the first element of the same list.
> + Iter {
> + current: self.first,
> + stop: self.first,
> + _ty: PhantomData,
> + }
> + }
> }
>
> impl<T: ?Sized + ListItem<ID>, const ID: u64> Default for List<T, ID> {
> @@ -450,3 +463,92 @@ fn drop(&mut self) {
> }
> }
> }
> +
> +/// An iterator into a [`List`].
I would use "over" instead of "into".
> +///
> +/// # Invariants
> +///
> +/// * There must be a [`List`] that is immutably borrowed for the duration of `'a`.
> +/// * The `current` pointer is null or points at a value in that [`List`].
> +/// * The `stop` pointer is equal to the `first` field of the [`List`].
the -> that
---
Cheers,
Benno
> +#[derive(Clone)]
> +pub struct Iter<'a, T: ?Sized + ListItem<ID>, const ID: u64 = 0> {
> + current: *mut ListLinksFields,
> + stop: *mut ListLinksFields,
> + _ty: PhantomData<&'a ListArc<T, ID>>,
> +}
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 7/9] rust: list: add cursor
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
` (5 preceding siblings ...)
2024-05-06 9:53 ` [PATCH v2 6/9] rust: list: add iterators Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 10:37 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 8/9] rust: list: support heterogeneous lists Alice Ryhl
2024-05-06 9:53 ` [PATCH v2 9/9] rust: list: add ListArcField Alice Ryhl
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
The cursor is very similar to the list iterator, but it has one
important feature that the iterator doesn't: it can be used to remove
items from the linked list.
This feature cannot be added to the iterator because the references you
get from the iterator are considered borrows of the original list,
rather than borrows of the iterator. This means that there's no way to
prevent code like this:
let item = iter.next();
iter.remove();
use(item);
If `iter` was a cursor instead of an iterator, then `item` will be
considered a borrow of `iter`. Since `remove` destroys `iter`, this
means that the borrow-checker will prevent uses of `item` after the call
to `remove`.
So there is a trade-off between supporting use in traditional for loops,
and supporting removal of elements as you iterate. Iterators and cursors
represents two different choices on that spectrum.
Rust Binder needs cursors for the list of death notifications that a
process is currently handling. When userspace tells Binder that it has
finished processing the death notification, Binder will iterate the list
to search for the relevant item and remove it.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index e36afc7ee44a..641b434e3841 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -438,6 +438,20 @@ pub fn push_all_back(&mut self, other: &mut List<T, ID>) {
other.first = ptr::null_mut();
}
+ /// Returns a cursor to the first element of the list.
+ ///
+ /// If the list is empty, this returns `None`.
+ pub fn cursor_front(&mut self) -> Option<Cursor<'_, T, ID>> {
+ if self.first.is_null() {
+ None
+ } else {
+ Some(Cursor {
+ current: self.first,
+ list: self,
+ })
+ }
+ }
+
/// Creates an iterator over the list.
pub fn iter(&self) -> Iter<'_, T, ID> {
// INVARIANT: If the list is empty, both pointers are null. Otherwise, both pointers point
@@ -512,6 +526,74 @@ fn next(&mut self) -> Option<ArcBorrow<'a, T>> {
}
}
+/// A cursor into a [`List`].
+///
+/// # Invariants
+///
+/// The `current` pointer points a value in `list`.
+pub struct Cursor<'a, T: ?Sized + ListItem<ID>, const ID: u64 = 0> {
+ current: *mut ListLinksFields,
+ list: &'a mut List<T, ID>,
+}
+
+impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> Cursor<'a, T, ID> {
+ /// Access the current element of this cursor.
+ pub fn current(&self) -> ArcBorrow<'_, T> {
+ // SAFETY: The `current` pointer points a value in the list.
+ let me = unsafe { T::view_value(ListLinks::from_fields(self.current)) };
+ // SAFETY:
+ // * All values in a list are stored in an `Arc`.
+ // * The value cannot be removed from the list for the duration of the lifetime annotated
+ // on the returned `ArcBorrow`, because removing it from the list would require mutable
+ // access to the cursor or the list. However, the `ArcBorrow` holds an immutable borrow
+ // on the cursor, which in turn holds a mutable borrow on the list, so any such
+ // mutable access requires first releasing the immutable borrow on the cursor.
+ // * Values in a list never have a `UniqueArc` reference, because the list has a `ListArc`
+ // reference, and `UniqueArc` references must be unique.
+ unsafe { ArcBorrow::from_raw(me) }
+ }
+
+ /// Move the cursor to the next element.
+ pub fn next(self) -> Option<Cursor<'a, T, ID>> {
+ // SAFETY: The `current` field is always in a list.
+ let next = unsafe { (*self.current).next };
+
+ if next == self.list.first {
+ None
+ } else {
+ // INVARIANT: Since `self.current` is in the `list`, its `next` pointer is also in the
+ // `list`.
+ Some(Cursor {
+ current: next,
+ list: self.list,
+ })
+ }
+ }
+
+ /// Move the cursor to the previous element.
+ pub fn prev(self) -> Option<Cursor<'a, T, ID>> {
+ // SAFETY: The `current` field is always in a list.
+ let prev = unsafe { (*self.current).prev };
+
+ if self.current == self.list.first {
+ None
+ } else {
+ // INVARIANT: Since `self.current` is in the `list`, its `prev` pointer is also in the
+ // `list`.
+ Some(Cursor {
+ current: prev,
+ list: self.list,
+ })
+ }
+ }
+
+ /// Remove the current element from the list.
+ pub fn remove(self) -> ListArc<T, ID> {
+ // SAFETY: The `current` pointer always points at a member of the list.
+ unsafe { self.list.remove_internal(self.current) }
+ }
+}
+
impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> FusedIterator for Iter<'a, T, ID> {}
impl<'a, T: ?Sized + ListItem<ID>, const ID: u64> IntoIterator for &'a List<T, ID> {
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 7/9] rust: list: add cursor
2024-05-06 9:53 ` [PATCH v2 7/9] rust: list: add cursor Alice Ryhl
@ 2024-05-27 10:37 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 10:37 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> The cursor is very similar to the list iterator, but it has one
> important feature that the iterator doesn't: it can be used to remove
> items from the linked list.
>
> This feature cannot be added to the iterator because the references you
> get from the iterator are considered borrows of the original list,
> rather than borrows of the iterator. This means that there's no way to
> prevent code like this:
>
> let item = iter.next();
> iter.remove();
> use(item);
>
> If `iter` was a cursor instead of an iterator, then `item` will be
> considered a borrow of `iter`. Since `remove` destroys `iter`, this
> means that the borrow-checker will prevent uses of `item` after the call
> to `remove`.
>
> So there is a trade-off between supporting use in traditional for loops,
> and supporting removal of elements as you iterate. Iterators and cursors
> represents two different choices on that spectrum.
>
> Rust Binder needs cursors for the list of death notifications that a
> process is currently handling. When userspace tells Binder that it has
> finished processing the death notification, Binder will iterate the list
> to search for the relevant item and remove it.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/list.rs | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 82 insertions(+)
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 8/9] rust: list: support heterogeneous lists
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
` (6 preceding siblings ...)
2024-05-06 9:53 ` [PATCH v2 7/9] rust: list: add cursor Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 10:46 ` Benno Lossin
2024-05-06 9:53 ` [PATCH v2 9/9] rust: list: add ListArcField Alice Ryhl
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
Support linked lists that can have many different structs at once. This
is generally done using trait objects. The main challenge is figuring
what the struct is given only a pointer to the ListLinks.
We do this by storing a pointer to the struct next to the ListLinks
field. The container_of operation will then just read that pointer. When
the type is a trait object, that pointer will be a fat pointer whose
metadata is a vtable that tells you what kind of struct it is.
Heterogeneous lists are heavily used by Rust Binder. There are a lot of
so-called todo lists containing various events that need to be delivered
to userspace next time userspace calls into the driver. And there are
quite a few different todo item types: incoming transaction, changes to
refcounts, death notifications, and more.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 47 ++++++++++++++-
rust/kernel/list/impl_list_item_mod.rs | 105 +++++++++++++++++++++++++++++++++
2 files changed, 151 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index 641b434e3841..3e687401c6d3 100644
--- a/rust/kernel/list.rs
+++ b/rust/kernel/list.rs
@@ -7,12 +7,16 @@
use crate::init::PinInit;
use crate::sync::ArcBorrow;
use crate::types::Opaque;
+use core::cell::UnsafeCell;
use core::iter::{DoubleEndedIterator, FusedIterator};
use core::marker::PhantomData;
+use core::mem::MaybeUninit;
use core::ptr;
mod impl_list_item_mod;
-pub use self::impl_list_item_mod::{impl_has_list_links, impl_list_item, HasListLinks};
+pub use self::impl_list_item_mod::{
+ impl_has_list_links, impl_has_list_links_self_ptr, impl_list_item, HasListLinks, HasSelfPtr,
+};
mod arc;
pub use self::arc::{
@@ -180,6 +184,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
}
}
+/// Similar to [`ListLinks`], but also contains a pointer to the full value.
+///
+/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
+#[repr(C)]
+pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
+ /// The `ListLinks` field inside this value.
+ ///
+ /// This is public so that it can be used with `impl_has_list_links!`.
+ pub inner: ListLinks<ID>,
+ self_ptr: UnsafeCell<MaybeUninit<*const T>>,
+}
+
+// SAFETY: The fields of a ListLinksSelfPtr can be moved across thread boundaries.
+unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
+// SAFETY: The type is opaque so immutable references to a ListLinksSelfPtr are useless. Therefore,
+// it's okay to have immutable access to a ListLinks from several threads at once.
+//
+// Note that `inner` being a public field does not prevent this type from being opaque, since
+// `inner` is a opaque type.
+unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
+
+impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
+ /// The offset from the [`ListLinks`] to the self pointer field.
+ pub const LIST_LINKS_SELF_PTR_OFFSET: usize = core::mem::offset_of!(Self, self_ptr);
+
+ /// Creates a new initializer for this type.
+ pub fn new() -> impl PinInit<Self> {
+ // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
+ // not be constructed in an `Arc` that already has a `ListArc`.
+ Self {
+ inner: ListLinks {
+ inner: Opaque::new(ListLinksFields {
+ prev: ptr::null_mut(),
+ next: ptr::null_mut(),
+ }),
+ },
+ self_ptr: UnsafeCell::new(MaybeUninit::zeroed()),
+ }
+ }
+}
+
impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
/// Creates a new empty list.
pub const fn new() -> Self {
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index 3ff483be89d1..96e90c0ec587 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -62,6 +62,49 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
}
pub use impl_has_list_links;
+/// Declares that the `ListLinks<ID>` field in this struct is inside a `ListLinksSelfPtr<T, ID>`.
+///
+/// # Safety
+///
+/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
+/// inside a `ListLinksSelfPtr<T, ID>`.
+pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
+where
+ Self: HasListLinks<ID>,
+{
+}
+
+/// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
+#[macro_export]
+macro_rules! impl_has_list_links_self_ptr {
+ ($(impl$({$($implarg:tt)*})?
+ HasSelfPtr<$item_type:ty $(, $id:tt)?>
+ for $self:ident $(<$($selfarg:ty),*>)?
+ { self.$field:ident }
+ )*) => {$(
+ // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
+ // right type.
+ unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for
+ $self $(<$($selfarg),*>)?
+ {}
+
+ unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for
+ $self $(<$($selfarg),*>)?
+ {
+ const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
+
+ #[inline]
+ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
+ // SAFETY: The caller promises that the pointer is not dangling.
+ let ptr: *mut $crate::list::ListLinksSelfPtr<$item_type $(, $id)?> =
+ unsafe { ::core::ptr::addr_of_mut!((*ptr).$field) };
+ ptr.cast()
+ }
+ }
+ )*};
+}
+pub use impl_has_list_links_self_ptr;
+
/// Implements the [`ListItem`] trait for the given type.
///
/// Assumes that the type implements [`HasListLinks`].
@@ -95,5 +138,67 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
}
}
};
+
+ (
+ impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
+ using ListLinksSelfPtr;
+ } $($rest:tt)*
+ ) => {
+ unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
+ unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
+ // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
+ let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
+
+ let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
+ // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
+ // the pointer stays in bounds of the allocation.
+ let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
+ as *const ::core::cell::UnsafeCell<*const Self>;
+ let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
+
+ // SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
+ // `post_remove`, or `view_value`. By the safety requirements of those methods,
+ // none of these three methods may be called in parallel with this call to
+ // `prepare_to_insert`, so this write will not race with any other access to the
+ // value.
+ unsafe { ::core::ptr::write(cell_inner, me) };
+
+ links_field
+ }
+
+ unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
+ // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
+ unsafe { <Self as HasListLinks<$num>>::raw_get_list_links(me.cast_mut()) }
+ }
+
+ // This function is also used as the implementation of `post_remove`, so the caller
+ // may choose to satisfy the safety requirements of `post_remove` instead of the safety
+ // requirements for `view_value`.
+ unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
+ let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
+ // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
+ // the pointer stays in bounds of the allocation.
+ let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
+ as *const ::core::cell::UnsafeCell<*const Self>;
+ let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
+ // This returns the same pointer as the one passes to the previous call to
+ // `prepare_to_insert` since that previous call wrote that pointer to this
+ // location, and the value has not been modified since.
+ //
+ // SAFETY: This is not a data race, because the only function that writes to this
+ // value is `prepare_to_insert`, but by the safety requirements the
+ // `prepare_to_insert` method may not be called in parallel with `view_value` or
+ // `post_remove`.
+ unsafe { ::core::ptr::read(cell_inner) }
+ }
+
+ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
+ // SAFETY: This specific implementation of `view_value` allows the caller to
+ // promise the safety requirements of `post_remove` instead of the safety
+ // requirements for `view_value`.
+ unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
+ }
+ }
+ };
}
pub use impl_list_item;
--
2.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 8/9] rust: list: support heterogeneous lists
2024-05-06 9:53 ` [PATCH v2 8/9] rust: list: support heterogeneous lists Alice Ryhl
@ 2024-05-27 10:46 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 10:46 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, Alice Ryhl wrote:
> Support linked lists that can have many different structs at once. This
have -> hold
> is generally done using trait objects. The main challenge is figuring
> what the struct is given only a pointer to the ListLinks.
>
> We do this by storing a pointer to the struct next to the ListLinks
> field. The container_of operation will then just read that pointer. When
> the type is a trait object, that pointer will be a fat pointer whose
> metadata is a vtable that tells you what kind of struct it is.
>
> Heterogeneous lists are heavily used by Rust Binder. There are a lot of
> so-called todo lists containing various events that need to be delivered
> to userspace next time userspace calls into the driver. And there are
> quite a few different todo item types: incoming transaction, changes to
> refcounts, death notifications, and more.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/list.rs | 47 ++++++++++++++-
> rust/kernel/list/impl_list_item_mod.rs | 105 +++++++++++++++++++++++++++++++++
> 2 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
> index 641b434e3841..3e687401c6d3 100644
> --- a/rust/kernel/list.rs
> +++ b/rust/kernel/list.rs
> @@ -7,12 +7,16 @@
> use crate::init::PinInit;
> use crate::sync::ArcBorrow;
> use crate::types::Opaque;
> +use core::cell::UnsafeCell;
> use core::iter::{DoubleEndedIterator, FusedIterator};
> use core::marker::PhantomData;
> +use core::mem::MaybeUninit;
> use core::ptr;
>
> mod impl_list_item_mod;
> -pub use self::impl_list_item_mod::{impl_has_list_links, impl_list_item, HasListLinks};
> +pub use self::impl_list_item_mod::{
> + impl_has_list_links, impl_has_list_links_self_ptr, impl_list_item, HasListLinks, HasSelfPtr,
> +};
>
> mod arc;
> pub use self::arc::{
> @@ -180,6 +184,47 @@ unsafe fn from_fields(me: *mut ListLinksFields) -> *mut Self {
> }
> }
>
> +/// Similar to [`ListLinks`], but also contains a pointer to the full value.
> +///
> +/// This type can be used instead of [`ListLinks`] to support lists with trait objects.
> +#[repr(C)]
> +pub struct ListLinksSelfPtr<T: ?Sized, const ID: u64 = 0> {
> + /// The `ListLinks` field inside this value.
> + ///
> + /// This is public so that it can be used with `impl_has_list_links!`.
> + pub inner: ListLinks<ID>,
> + self_ptr: UnsafeCell<MaybeUninit<*const T>>,
Why don't you use `Opaque` instead?
> +}
> +
> +// SAFETY: The fields of a ListLinksSelfPtr can be moved across thread boundaries.
> +unsafe impl<T: ?Sized + Send, const ID: u64> Send for ListLinksSelfPtr<T, ID> {}
> +// SAFETY: The type is opaque so immutable references to a ListLinksSelfPtr are useless. Therefore,
> +// it's okay to have immutable access to a ListLinks from several threads at once.
> +//
> +// Note that `inner` being a public field does not prevent this type from being opaque, since
> +// `inner` is a opaque type.
> +unsafe impl<T: ?Sized + Sync, const ID: u64> Sync for ListLinksSelfPtr<T, ID> {}
> +
> +impl<T: ?Sized, const ID: u64> ListLinksSelfPtr<T, ID> {
> + /// The offset from the [`ListLinks`] to the self pointer field.
> + pub const LIST_LINKS_SELF_PTR_OFFSET: usize = core::mem::offset_of!(Self, self_ptr);
> +
> + /// Creates a new initializer for this type.
> + pub fn new() -> impl PinInit<Self> {
> + // INVARIANT: Pin-init initializers can't be used on an existing `Arc`, so this value will
> + // not be constructed in an `Arc` that already has a `ListArc`.
> + Self {
> + inner: ListLinks {
> + inner: Opaque::new(ListLinksFields {
> + prev: ptr::null_mut(),
> + next: ptr::null_mut(),
> + }),
> + },
> + self_ptr: UnsafeCell::new(MaybeUninit::zeroed()),
> + }
> + }
> +}
> +
> impl<T: ?Sized + ListItem<ID>, const ID: u64> List<T, ID> {
> /// Creates a new empty list.
> pub const fn new() -> Self {
> diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
> index 3ff483be89d1..96e90c0ec587 100644
> --- a/rust/kernel/list/impl_list_item_mod.rs
> +++ b/rust/kernel/list/impl_list_item_mod.rs
> @@ -62,6 +62,49 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$
> }
> pub use impl_has_list_links;
>
> +/// Declares that the `ListLinks<ID>` field in this struct is inside a `ListLinksSelfPtr<T, ID>`.
> +///
> +/// # Safety
> +///
> +/// The `ListLinks<ID>` field of this struct at the offset `HasListLinks<ID>::OFFSET` must be
> +/// inside a `ListLinksSelfPtr<T, ID>`.
> +pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0>
> +where
> + Self: HasListLinks<ID>,
> +{
> +}
> +
> +/// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type.
> +#[macro_export]
> +macro_rules! impl_has_list_links_self_ptr {
> + ($(impl$({$($implarg:tt)*})?
> + HasSelfPtr<$item_type:ty $(, $id:tt)?>
> + for $self:ident $(<$($selfarg:ty),*>)?
> + { self.$field:ident }
> + )*) => {$(
> + // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the
> + // right type.
> + unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for
> + $self $(<$($selfarg),*>)?
> + {}
> +
> + unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for
Missing SAFETY comment.
> + $self $(<$($selfarg),*>)?
> + {
> + const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
> +
> + #[inline]
> + unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut $crate::list::ListLinks$(<$id>)? {
> + // SAFETY: The caller promises that the pointer is not dangling.
> + let ptr: *mut $crate::list::ListLinksSelfPtr<$item_type $(, $id)?> =
> + unsafe { ::core::ptr::addr_of_mut!((*ptr).$field) };
> + ptr.cast()
> + }
> + }
> + )*};
> +}
> +pub use impl_has_list_links_self_ptr;
> +
> /// Implements the [`ListItem`] trait for the given type.
> ///
> /// Assumes that the type implements [`HasListLinks`].
> @@ -95,5 +138,67 @@ unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> }
> }
> };
> +
> + (
> + impl$({$($generics:tt)*})? ListItem<$num:tt> for $t:ty {
> + using ListLinksSelfPtr;
> + } $($rest:tt)*
> + ) => {
> + unsafe impl$(<$($generics)*>)? $crate::list::ListItem<$num> for $t {
Missing SAFETY comment.
---
Cheers,
Benno
> + unsafe fn prepare_to_insert(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> + // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
> + let links_field = unsafe { <Self as $crate::list::ListItem<$num>>::view_links(me) };
> +
> + let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
> + // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
> + // the pointer stays in bounds of the allocation.
> + let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
> + as *const ::core::cell::UnsafeCell<*const Self>;
> + let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
> +
> + // SAFETY: This value is not accessed in any other places than `prepare_to_insert`,
> + // `post_remove`, or `view_value`. By the safety requirements of those methods,
> + // none of these three methods may be called in parallel with this call to
> + // `prepare_to_insert`, so this write will not race with any other access to the
> + // value.
> + unsafe { ::core::ptr::write(cell_inner, me) };
> +
> + links_field
> + }
> +
> + unsafe fn view_links(me: *const Self) -> *mut $crate::list::ListLinks<$num> {
> + // SAFETY: The caller promises that `me` points at a valid value of type `Self`.
> + unsafe { <Self as HasListLinks<$num>>::raw_get_list_links(me.cast_mut()) }
> + }
> +
> + // This function is also used as the implementation of `post_remove`, so the caller
> + // may choose to satisfy the safety requirements of `post_remove` instead of the safety
> + // requirements for `view_value`.
> + unsafe fn view_value(links_field: *mut $crate::list::ListLinks<$num>) -> *const Self {
> + let spoff = $crate::list::ListLinksSelfPtr::<Self, $num>::LIST_LINKS_SELF_PTR_OFFSET;
> + // SAFETY: The constant is equal to `offset_of!(ListLinksSelfPtr, self_ptr)`, so
> + // the pointer stays in bounds of the allocation.
> + let self_ptr = unsafe { (links_field as *const u8).add(spoff) }
> + as *const ::core::cell::UnsafeCell<*const Self>;
> + let cell_inner = ::core::cell::UnsafeCell::raw_get(self_ptr);
> + // This returns the same pointer as the one passes to the previous call to
> + // `prepare_to_insert` since that previous call wrote that pointer to this
> + // location, and the value has not been modified since.
> + //
> + // SAFETY: This is not a data race, because the only function that writes to this
> + // value is `prepare_to_insert`, but by the safety requirements the
> + // `prepare_to_insert` method may not be called in parallel with `view_value` or
> + // `post_remove`.
> + unsafe { ::core::ptr::read(cell_inner) }
> + }
> +
> + unsafe fn post_remove(me: *mut $crate::list::ListLinks<$num>) -> *const Self {
> + // SAFETY: This specific implementation of `view_value` allows the caller to
> + // promise the safety requirements of `post_remove` instead of the safety
> + // requirements for `view_value`.
> + unsafe { <Self as $crate::list::ListItem<$num>>::view_value(me) }
> + }
> + }
> + };
> }
> pub use impl_list_item;
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 9/9] rust: list: add ListArcField
2024-05-06 9:53 [PATCH v2 0/9] Add Rust linked list for reference counted values Alice Ryhl
` (7 preceding siblings ...)
2024-05-06 9:53 ` [PATCH v2 8/9] rust: list: support heterogeneous lists Alice Ryhl
@ 2024-05-06 9:53 ` Alice Ryhl
2024-05-27 10:50 ` Benno Lossin
8 siblings, 1 reply; 20+ messages in thread
From: Alice Ryhl @ 2024-05-06 9:53 UTC (permalink / raw)
To: Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Marco Elver,
Kees Cook, Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar,
Jakub Kicinski, Wei Yang, Matthew Wilcox, linux-kernel,
rust-for-linux, Alice Ryhl
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.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/list.rs | 3 ++
rust/kernel/list/arc_field.rs | 96 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+)
diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs
index 3e687401c6d3..d88a4c0f5c31 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..2330f673427a
--- /dev/null
+++ b/rust/kernel/list/arc_field.rs
@@ -0,0 +1,96 @@
+// 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`].
+///
+/// [`ListArc`]: crate::list::ListArc
+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 getters for a [`ListArcField`].
+#[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.45.0.rc1.225.g2a3ae87e7f-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v2 9/9] rust: list: add ListArcField
2024-05-06 9:53 ` [PATCH v2 9/9] rust: list: add ListArcField Alice Ryhl
@ 2024-05-27 10:50 ` Benno Lossin
0 siblings, 0 replies; 20+ messages in thread
From: Benno Lossin @ 2024-05-27 10:50 UTC (permalink / raw)
To: Alice Ryhl, Miguel Ojeda, Andrew Morton
Cc: Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Marco Elver, Kees Cook,
Coly Li, Paolo Abeni, Pierre Gondois, Ingo Molnar, Jakub Kicinski,
Wei Yang, Matthew Wilcox, linux-kernel, rust-for-linux
On 06.05.24 11:53, 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.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/list.rs | 3 ++
> rust/kernel/list/arc_field.rs | 96 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 99 insertions(+)
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 20+ messages in thread