Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH v3] rust: xarray: fix false positive lockdep warnings
@ 2026-11-03 16:16 Andreas Hindborg
  0 siblings, 0 replies; only message in thread
From: Andreas Hindborg @ 2026-11-03 16:16 UTC (permalink / raw)
  To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Tamir Duberstein,
	Boqun Feng, Daniel Almeida, Alexandre Courbot, Onur Özkan,
	Boqun Feng
  Cc: rust-for-linux, linux-kernel, Andreas Hindborg

`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>



^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-11-03 16:16 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 16:16 [PATCH v3] rust: xarray: fix false positive lockdep warnings Andreas Hindborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox