From: Alice Ryhl <aliceryhl@google.com>
To: Miguel Ojeda <ojeda@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>
Cc: "Alex Gaynor" <alex.gaynor@gmail.com>,
"Wedson Almeida Filho" <wedsonaf@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Andreas Hindborg" <a.hindborg@samsung.com>,
"Marco Elver" <elver@google.com>, "Coly Li" <colyli@suse.de>,
"Paolo Abeni" <pabeni@redhat.com>,
"Pierre Gondois" <pierre.gondois@arm.com>,
"Ingo Molnar" <mingo@kernel.org>,
"Jakub Kicinski" <kuba@kernel.org>,
"Wei Yang" <richard.weiyang@gmail.com>,
"Matthew Wilcox" <willy@infradead.org>,
linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
"Alice Ryhl" <aliceryhl@google.com>,
"Kees Cook" <kees@kernel.org>
Subject: [PATCH v3 03/10] rust: list: add tracking for ListArc
Date: Tue, 23 Jul 2024 08:22:04 +0000 [thread overview]
Message-ID: <20240723-linked-list-v3-3-89db92c7dbf4@google.com> (raw)
In-Reply-To: <20240723-linked-list-v3-0-89db92c7dbf4@google.com>
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 | 162 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 163 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 3b7072e58256..97bd9d52b5cf 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.
@@ -47,9 +48,30 @@ 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`].
+/// This macro supports a few different strategies for implementing the tracking inside the type:
+///
+/// * The `untracked` strategy does not actually keep track of whether a [`ListArc`] exists. When
+/// using this strategy, the only way to create a [`ListArc`] is using a [`UniqueArc`].
+/// * The `tracked_by` strategy defers the tracking to a field of the struct. The user much specify
+/// which field to defer the tracking to. The field must implement [`ListArcSafe`].
+///
+/// The `tracked_by` strategy is usually used by deferring to a field of type
+/// [`AtomicListArcTracker`]. However, it is also possible to defer the tracking to another struct
+/// using also using this macro.
#[macro_export]
macro_rules! impl_list_arc_safe {
(impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty { untracked; } $($rest:tt)*) => {
@@ -60,6 +82,39 @@ 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>) {
+ $crate::assert_pinned!($t, $field, $fty, inline);
+
+ // SAFETY: This field is structurally pinned as per the above assertion.
+ 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;
@@ -201,6 +256,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),
+ }
+ }
+
/// Transmutes an [`Arc`] into a `ListArc` without updating the tracking inside `T`.
///
/// # Safety
@@ -346,3 +447,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.2.1089.g2a221341d9-goog
next prev parent reply other threads:[~2024-07-23 8:23 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-23 8:22 [PATCH v3 00/10] Add Rust linked list for reference counted values Alice Ryhl
2024-07-23 8:22 ` [PATCH v3 01/10] rust: init: add `assert_pinned` macro Alice Ryhl
2024-07-23 8:22 ` [PATCH v3 02/10] rust: list: add ListArc Alice Ryhl
2024-07-31 16:47 ` Benno Lossin
2024-08-06 13:16 ` Alice Ryhl
2024-08-06 14:11 ` Benno Lossin
2024-08-06 14:14 ` Miguel Ojeda
2024-07-23 8:22 ` Alice Ryhl [this message]
2024-07-31 17:17 ` [PATCH v3 03/10] rust: list: add tracking for ListArc Benno Lossin
2024-07-23 8:22 ` [PATCH v3 04/10] rust: list: add struct with prev/next pointers Alice Ryhl
2024-07-31 18:41 ` Benno Lossin
2024-08-01 9:42 ` Alice Ryhl
2024-08-01 10:45 ` Benno Lossin
2024-08-01 12:51 ` Alice Ryhl
2024-08-01 13:46 ` Benno Lossin
2024-08-01 13:47 ` Alice Ryhl
2024-07-23 8:22 ` [PATCH v3 05/10] rust: list: add macro for implementing ListItem Alice Ryhl
2024-07-31 13:03 ` Alice Ryhl
2024-07-31 20:17 ` Benno Lossin
2024-07-23 8:22 ` [PATCH v3 06/10] rust: list: add List Alice Ryhl
2024-08-01 9:11 ` Benno Lossin
2024-08-01 9:40 ` Alice Ryhl
2024-08-01 10:48 ` Benno Lossin
2024-07-23 8:22 ` [PATCH v3 07/10] rust: list: add iterators Alice Ryhl
2024-07-23 8:22 ` [PATCH v3 08/10] rust: list: add cursor Alice Ryhl
2024-07-23 8:22 ` [PATCH v3 09/10] rust: list: support heterogeneous lists Alice Ryhl
2024-08-01 9:24 ` Benno Lossin
2024-08-01 9:38 ` Alice Ryhl
2024-08-01 10:50 ` Benno Lossin
2024-08-01 12:33 ` Alice Ryhl
2024-07-23 8:22 ` [PATCH v3 10/10] rust: list: add ListArcField Alice Ryhl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240723-linked-list-v3-3-89db92c7dbf4@google.com \
--to=aliceryhl@google.com \
--cc=a.hindborg@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=alex.gaynor@gmail.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=colyli@suse.de \
--cc=elver@google.com \
--cc=gary@garyguo.net \
--cc=kees@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=ojeda@kernel.org \
--cc=pabeni@redhat.com \
--cc=pierre.gondois@arm.com \
--cc=richard.weiyang@gmail.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=wedsonaf@gmail.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).