All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Hindborg <a.hindborg@kernel.org>
To: "Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Boqun Feng" <boqun@kernel.org>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Andreas Hindborg <a.hindborg@kernel.org>
Subject: [PATCH v3] rust: xarray: fix false positive lockdep warnings
Date: Tue, 03 Nov 2026 17:16:28 +0100	[thread overview]
Message-ID: <20261103-xarray-lockdep-fix-v3-1-e188bec82003@kernel.org> (raw)

`xa_init_flags()` is a static inline that expands `spin_lock_init()`,
which generates a single static `lock_class_key` per compilation unit.
When used through the Rust binding helper, every Rust `XArray`
instance ends up sharing that single key. Lockdep then sees concurrent
locking of distinct Rust `XArray`s as recursive locking on the same
class and emits false-positive lock-ordering reports.

Add a custom helper `xa_init_flags_with_key()` that mirrors
`xa_init_flags()` but takes an explicit name and `struct
lock_class_key` from the caller. Update `XArray::new` to take a name
and a `LockClassKey`, and add a `new_xarray!` macro that uses
`optional_name!` and `static_lock_class!` to generate a unique class
per instantiation site.

Remove the now unused `rust_helper_xa_init_flags`.

This was previously part of the series at [1].

Link: https://lore.kernel.org/r/20251203-xarray-entry-send-v1-0-9e5ffd5e3cf0@kernel.org [1]
Fixes: 210b81578efbe ("rust: xarray: Add an abstraction for XArray")
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Tamir Duberstein <tamird@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
This now mirrors `init_work_with_key()` in `rust/helpers/workqueue.c`,
which takes a name and a `struct lock_class_key` from the caller for
exactly the same reason: the C initializer it replaces produces one static
lock class per call site, so routing every Rust caller through a single
helper would collapse them all onto one class.

Changes in v3:
- Rebase on v7.4-rc1.
- Reword the comment on `xa_init_flags_with_key()` to describe the helper
  for its caller rather than its implementation (Tamir).
- Drop the unnecessary type ascription in the `XArray` example (Tamir).
- Link to v2: https://msgid.link/20260605-xarray-lockdep-fix-v2-1-25a5509abee3@kernel.org

Changes in v2:
- Use a C helper `xa_init_flags_with_key()` that calls
  `lockdep_set_class_and_name()` to install the caller-supplied key,
  rather than calling `__spin_lock_init()` from Rust. Mirrors the
  `init_work_with_key()` pattern in `rust/helpers/workqueue.c`
  (suggested by Alice Ryhl, seconded by Tamir Duberstein).
- Add `Suggested-by: Alice Ryhl <aliceryhl@google.com>`.
- Correct Tamir's `Acked-by:` email to `tamird@kernel.org`.
- Rebase on v7.1-rc2.
- Link to v1: https://msgid.link/20260206-xarray-lockdep-fix-v1-1-13315951b836@kernel.org
---
 rust/helpers/xarray.c | 17 +++++++++++++++--
 rust/kernel/xarray.rs | 27 +++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
index 08979b304341..f6cc05b26cc0 100644
--- a/rust/helpers/xarray.c
+++ b/rust/helpers/xarray.c
@@ -7,9 +7,22 @@ __rust_helper int rust_helper_xa_err(void *entry)
 	return xa_err(entry);
 }
 
-__rust_helper void rust_helper_xa_init_flags(struct xarray *xa, gfp_t flags)
+/*
+ * Initialize `xa` with `flags` and register its internal spinlock with the
+ * caller-supplied lockdep class `key` and name `name`, both of which must
+ * live for the lifetime of `xa`.
+ *
+ * `xa_init_flags()` cannot be used instead: it expands `spin_lock_init()`,
+ * which yields one static lockdep class per compilation unit, so every
+ * caller reaching it through a Rust helper would share a single class.
+ */
+__rust_helper void rust_helper_xa_init_flags_with_key(struct xarray *xa,
+						      gfp_t flags,
+						      const char *name,
+						      struct lock_class_key *key)
 {
-	return xa_init_flags(xa, flags);
+	xa_init_flags(xa, flags);
+	lockdep_set_class_and_name(&xa->xa_lock, key, name);
 }
 
 __rust_helper int rust_helper_xa_trylock(struct xarray *xa)
diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
index 987c9c0c2198..84e7fb051612 100644
--- a/rust/kernel/xarray.rs
+++ b/rust/kernel/xarray.rs
@@ -10,6 +10,8 @@
     build_assert::build_assert,
     error::{Error, Result},
     ffi::c_void,
+    str::{CStr, CStrExt},
+    sync::LockClassKey,
     types::{
         ForeignOwnable,
         NotThreadSafe,
@@ -19,6 +21,19 @@
 use core::{iter, marker::PhantomData, pin::Pin, ptr::NonNull};
 use pin_init::{pin_data, pin_init, pinned_drop, PinInit};
 
+/// Creates a [`XArray`] initialiser with the given name and a newly-created lock class.
+///
+/// It uses the name if one is given, otherwise it generates one based on the file name and line
+/// number.
+#[macro_export]
+macro_rules! new_xarray {
+    ($kind:expr $(, $name:literal)? $(,)?) => {
+        $crate::xarray::XArray::new(
+            $kind, $crate::optional_name!($($name)?), $crate::static_lock_class!())
+    };
+}
+pub use new_xarray;
+
 /// An array which efficiently maps sparse integer indices to owned objects.
 ///
 /// This is similar to a [`crate::alloc::kvec::Vec<Option<T>>`], but more efficient when there are
@@ -33,9 +48,9 @@
 ///
 /// ```rust
 /// use kernel::alloc::KBox;
-/// use kernel::xarray::{AllocKind, XArray};
+/// use kernel::xarray::{new_xarray, AllocKind, XArray};
 ///
-/// let xa = KBox::pin_init(XArray::new(AllocKind::Alloc1), GFP_KERNEL)?;
+/// let xa = KBox::pin_init(new_xarray!(AllocKind::Alloc1), GFP_KERNEL)?;
 ///
 /// let dead = KBox::new(0xdead, GFP_KERNEL)?;
 /// let beef = KBox::new(0xbeef, GFP_KERNEL)?;
@@ -92,7 +107,11 @@ pub enum AllocKind {
 
 impl<T: ForeignOwnable> XArray<T> {
     /// Creates a new initializer for this type.
-    pub fn new(kind: AllocKind) -> impl PinInit<Self> {
+    pub fn new(
+        kind: AllocKind,
+        name: &'static CStr,
+        key: Pin<&'static LockClassKey>,
+    ) -> impl PinInit<Self> {
         let flags = match kind {
             AllocKind::Alloc => bindings::XA_FLAGS_ALLOC,
             AllocKind::Alloc1 => bindings::XA_FLAGS_ALLOC1,
@@ -102,7 +121,7 @@ pub fn new(kind: AllocKind) -> impl PinInit<Self> {
             //
             // INVARIANT: `xa` is initialized here to an empty, valid [`bindings::xarray`].
             xa <- Opaque::ffi_init(|xa| unsafe {
-                bindings::xa_init_flags(xa, flags)
+                bindings::xa_init_flags_with_key(xa, flags, name.as_char_ptr(), key.as_ptr())
             }),
             _p: PhantomData,
         })

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20260206-xarray-lockdep-fix-10f1cc68e5d7

Best regards,
--  
Andreas Hindborg <a.hindborg@kernel.org>



                 reply	other threads:[~2026-11-03 16:16 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20261103-xarray-lockdep-fix-v3-1-e188bec82003@kernel.org \
    --to=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.