[PATCH 10/10] rust: xarray: fix false positive lockdep warnings

Andreas Hindborg posted 10 patches 2 months ago
There is a newer version of this series
[PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Andreas Hindborg 2 months ago
Replace the `xa_init_flags` helper with direct initialization of XArray
structures using `__spin_lock_init`. This allows each XArray to have
its own lock class key for lockdep, preventing false positive warnings
about lock ordering violations.

Add a `new_xarray!` macro that automatically generates a unique lock
class key for each XArray instantiation site. The macro accepts an
optional name parameter and uses the `optional_name!` and
`static_lock_class!` macros to generate appropriate names and lock
classes.

Update the `XArray::new` method signature to accept explicit name and
lock class key parameters. This enables proper lockdep tracking while
maintaining flexibility for advanced use cases.

Remove `rust_helper_xa_init_flags` as it is now dead code.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/helpers/xarray.c       |  5 ---
 rust/kernel/xarray.rs       | 74 +++++++++++++++++++++++++++++++++------------
 rust/kernel/xarray/entry.rs | 55 ++++++++++++++++++---------------
 3 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
index 425a6cc494734..2852f63388eea 100644
--- a/rust/helpers/xarray.c
+++ b/rust/helpers/xarray.c
@@ -7,11 +7,6 @@ int rust_helper_xa_err(void *entry)
 	return xa_err(entry);
 }
 
-void rust_helper_xa_init_flags(struct xarray *xa, gfp_t flags)
-{
-	return xa_init_flags(xa, flags);
-}
-
 int rust_helper_xa_trylock(struct xarray *xa)
 {
 	return xa_trylock(xa);
diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
index a405f2b6fdcad..c393d4c01af2a 100644
--- a/rust/kernel/xarray.rs
+++ b/rust/kernel/xarray.rs
@@ -29,6 +29,8 @@
         Result, //
     },
     ffi::c_void,
+    str::CStr,
+    sync::LockClassKey,
     types::{
         ForeignOwnable,
         NotThreadSafe,
@@ -48,6 +50,19 @@
 
 mod preload;
 
+/// 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
@@ -62,9 +77,10 @@
 ///
 /// ```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: Pin<KBox<XArray<KBox<u32>>>> =
+///     KBox::pin_init(new_xarray!(AllocKind::Alloc1), GFP_KERNEL)?;
 ///
 /// let dead = KBox::new(0xdead, GFP_KERNEL)?;
 /// let beef = KBox::new(0xbeef, GFP_KERNEL)?;
@@ -124,7 +140,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,
@@ -133,8 +153,14 @@ pub fn new(kind: AllocKind) -> impl PinInit<Self> {
             // SAFETY: `xa` is valid while the closure is called.
             //
             // INVARIANT: `xa` is initialized here to an empty, valid [`bindings::xarray`].
-            xa <- Opaque::ffi_init(|xa| unsafe {
-                bindings::xa_init_flags(xa, flags)
+            xa <- Opaque::ffi_init(|xa: *mut bindings::xarray| unsafe {
+                bindings::__spin_lock_init(
+                    &raw mut (*xa).xa_lock,
+                    name.as_ptr().cast(),
+                    key.as_ptr(),
+                );
+                (*xa).xa_flags = flags;
+                (*xa).xa_head = null_mut();
             }),
             _p: PhantomData,
         })
@@ -236,8 +262,12 @@ fn load(&self, index: usize) -> Option<NonNull<c_void>> {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{alloc::{flags::GFP_KERNEL, kbox::KBox}, xarray::{AllocKind, XArray}};
-    /// let xa = KBox::pin_init(XArray::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{
+    /// #     alloc::{flags::GFP_KERNEL, kbox::KBox},
+    /// #     xarray::{new_xarray, AllocKind, XArray},
+    /// # };
+    /// let xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     ///
     /// let mut guard = xa.lock();
     /// assert_eq!(guard.contains_index(42), false);
@@ -272,8 +302,9 @@ pub fn get_mut(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// assert_eq!(guard.contains_index(42), false);
@@ -309,8 +340,9 @@ fn load_next(&self, index: usize) -> Option<(usize, NonNull<c_void>)> {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -339,8 +371,9 @@ pub fn find_next(&self, index: usize) -> Option<(usize, T::Borrowed<'_>)> {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -366,8 +399,9 @@ pub fn find_next_mut(&mut self, index: usize) -> Option<(usize, T::BorrowedMut<'
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -401,8 +435,9 @@ pub fn find_next_entry<'b>(&'b mut self, index: usize) -> Option<OccupiedEntry<'
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(100, KBox::new(42u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -505,8 +540,9 @@ pub fn store(
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// assert_eq!(guard.get(42), None);
diff --git a/rust/kernel/xarray/entry.rs b/rust/kernel/xarray/entry.rs
index 2d6ef4781f47d..6e370252309fc 100644
--- a/rust/kernel/xarray/entry.rs
+++ b/rust/kernel/xarray/entry.rs
@@ -26,8 +26,9 @@ impl<T: ForeignOwnable> Entry<'_, '_, T> {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// let entry = guard.get_entry(42);
@@ -100,17 +101,17 @@ fn insert_internal(
     ///
     /// Returns a reference to the newly inserted value.
     ///
-    /// - This method will fail if the nodes on the path to the index
-    ///   represented by this entry are not present in the XArray and no memory
-    ///   is available via the `preload` argument.
+    /// - This method will fail if the nodes on the path to the index represented by this entry are
+    ///   not present in the XArray and no memory is available via the `preload` argument.
     /// - This method will not drop the XArray lock.
     ///
     ///
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// assert_eq!(guard.get(42), None);
@@ -139,16 +140,16 @@ pub fn insert(
 
     /// Inserts a value and returns an occupied entry representing the newly inserted value.
     ///
-    /// - This method will fail if the nodes on the path to the index
-    ///   represented by this entry are not present in the XArray and no memory
-    ///   is available via the `preload` argument.
+    /// - This method will fail if the nodes on the path to the index represented by this entry are
+    ///   not present in the XArray and no memory is available via the `preload` argument.
     /// - This method will not drop the XArray lock.
     ///
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// assert_eq!(guard.get(42), None);
@@ -182,8 +183,9 @@ pub fn insert_entry(
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// assert_eq!(guard.get(42), None);
@@ -221,8 +223,9 @@ pub(crate) fn new(guard: &'b mut Guard<'a, T>, index: usize, ptr: NonNull<c_void
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -266,8 +269,9 @@ pub fn remove(mut self) -> T {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -287,8 +291,9 @@ pub fn index(&self) -> usize {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -334,8 +339,9 @@ pub fn insert(&mut self, value: T) -> T {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
@@ -361,8 +367,9 @@ pub fn into_mut(self) -> T::BorrowedMut<'b> {
     /// # Examples
     ///
     /// ```
-    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
-    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
+    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
+    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
+    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
     /// let mut guard = xa.lock();
     ///
     /// guard.store(42, KBox::new(100u32, GFP_KERNEL)?, GFP_KERNEL)?;

-- 
2.51.2
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Alice Ryhl 2 weeks, 1 day ago
On Wed, Dec 03, 2025 at 11:26:40PM +0100, Andreas Hindborg wrote:
> Replace the `xa_init_flags` helper with direct initialization of XArray
> structures using `__spin_lock_init`. This allows each XArray to have
> its own lock class key for lockdep, preventing false positive warnings
> about lock ordering violations.
> 
> Add a `new_xarray!` macro that automatically generates a unique lock
> class key for each XArray instantiation site. The macro accepts an
> optional name parameter and uses the `optional_name!` and
> `static_lock_class!` macros to generate appropriate names and lock
> classes.
> 
> Update the `XArray::new` method signature to accept explicit name and
> lock class key parameters. This enables proper lockdep tracking while
> maintaining flexibility for advanced use cases.
> 
> Remove `rust_helper_xa_init_flags` as it is now dead code.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

IMO this should have a Fixes: tag and probably land separately from the
rest of this series.

There is a similar situation with workqueue, which was solved by using a
custom helper. See rust/helpers/workqueue.c for details.

Alice
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Andreas Hindborg 13 hours ago
Alice Ryhl <aliceryhl@google.com> writes:

> On Wed, Dec 03, 2025 at 11:26:40PM +0100, Andreas Hindborg wrote:
>> Replace the `xa_init_flags` helper with direct initialization of XArray
>> structures using `__spin_lock_init`. This allows each XArray to have
>> its own lock class key for lockdep, preventing false positive warnings
>> about lock ordering violations.
>> 
>> Add a `new_xarray!` macro that automatically generates a unique lock
>> class key for each XArray instantiation site. The macro accepts an
>> optional name parameter and uses the `optional_name!` and
>> `static_lock_class!` macros to generate appropriate names and lock
>> classes.
>> 
>> Update the `XArray::new` method signature to accept explicit name and
>> lock class key parameters. This enables proper lockdep tracking while
>> maintaining flexibility for advanced use cases.
>> 
>> Remove `rust_helper_xa_init_flags` as it is now dead code.
>> 
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> IMO this should have a Fixes: tag and probably land separately from the
> rest of this series.

Ok.

>
> There is a similar situation with workqueue, which was solved by using a
> custom helper. See rust/helpers/workqueue.c for details.

I don't think coding a custom helper in C is better than open codng the
initalization in rust?


Best regards,
Andreas Hindborg
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Gary Guo 2 weeks, 2 days ago
On Wed Dec 3, 2025 at 10:26 PM GMT, Andreas Hindborg wrote:
> Replace the `xa_init_flags` helper with direct initialization of XArray
> structures using `__spin_lock_init`. This allows each XArray to have
> its own lock class key for lockdep, preventing false positive warnings
> about lock ordering violations.
>

Isn't this potentially a problem on the C side as well? `xa_init_flags` is a
static inline function, which means that the lock class is going to be the same
if a single C compilation unit initializes multiple xarrays -- unlike when you
use spin_lock_init, where each callsite gets a different lock class.

Best,
Gary

> Add a `new_xarray!` macro that automatically generates a unique lock
> class key for each XArray instantiation site. The macro accepts an
> optional name parameter and uses the `optional_name!` and
> `static_lock_class!` macros to generate appropriate names and lock
> classes.
>
> Update the `XArray::new` method signature to accept explicit name and
> lock class key parameters. This enables proper lockdep tracking while
> maintaining flexibility for advanced use cases.
>
> Remove `rust_helper_xa_init_flags` as it is now dead code.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/helpers/xarray.c       |  5 ---
>  rust/kernel/xarray.rs       | 74 +++++++++++++++++++++++++++++++++------------
>  rust/kernel/xarray/entry.rs | 55 ++++++++++++++++++---------------
>  3 files changed, 86 insertions(+), 48 deletions(-)
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Andreas Hindborg 2 weeks, 2 days ago
"Gary Guo" <gary@garyguo.net> writes:

> On Wed Dec 3, 2025 at 10:26 PM GMT, Andreas Hindborg wrote:
>> Replace the `xa_init_flags` helper with direct initialization of XArray
>> structures using `__spin_lock_init`. This allows each XArray to have
>> its own lock class key for lockdep, preventing false positive warnings
>> about lock ordering violations.
>>
>
> Isn't this potentially a problem on the C side as well? `xa_init_flags` is a
> static inline function, which means that the lock class is going to be the same
> if a single C compilation unit initializes multiple xarrays -- unlike when you
> use spin_lock_init, where each callsite gets a different lock class.
>
> Best,
> Gary
>
>> Add a `new_xarray!` macro that automatically generates a unique lock
>> class key for each XArray instantiation site. The macro accepts an
>> optional name parameter and uses the `optional_name!` and
>> `static_lock_class!` macros to generate appropriate names and lock
>> classes.

My intuition about this was that when the C static function is inlined, a
new static address is used for each place the function is inlined. Is
this not correct?


Best regards,
Andreas Hindborg
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Gary Guo 2 weeks, 2 days ago
On Wed Jan 21, 2026 at 7:01 PM GMT, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Wed Dec 3, 2025 at 10:26 PM GMT, Andreas Hindborg wrote:
>>> Replace the `xa_init_flags` helper with direct initialization of XArray
>>> structures using `__spin_lock_init`. This allows each XArray to have
>>> its own lock class key for lockdep, preventing false positive warnings
>>> about lock ordering violations.
>>>
>>
>> Isn't this potentially a problem on the C side as well? `xa_init_flags` is a
>> static inline function, which means that the lock class is going to be the same
>> if a single C compilation unit initializes multiple xarrays -- unlike when you
>> use spin_lock_init, where each callsite gets a different lock class.
>>
>> Best,
>> Gary
>>
>>> Add a `new_xarray!` macro that automatically generates a unique lock
>>> class key for each XArray instantiation site. The macro accepts an
>>> optional name parameter and uses the `optional_name!` and
>>> `static_lock_class!` macros to generate appropriate names and lock
>>> classes.
>
> My intuition about this was that when the C static function is inlined, a
> new static address is used for each place the function is inlined. Is
> this not correct?

No, the static inside static functions still have the normal static semantics.
There's a single copy regardless how many times the function is inlined.

So you have a unique copy per compilation unit.

Best,
Gary

>
> Best regards,
> Andreas Hindborg
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Andreas Hindborg 2 weeks, 2 days ago
"Gary Guo" <gary@garyguo.net> writes:

> On Wed Jan 21, 2026 at 7:01 PM GMT, Andreas Hindborg wrote:
>> "Gary Guo" <gary@garyguo.net> writes:
>>
>>> On Wed Dec 3, 2025 at 10:26 PM GMT, Andreas Hindborg wrote:
>>>> Replace the `xa_init_flags` helper with direct initialization of XArray
>>>> structures using `__spin_lock_init`. This allows each XArray to have
>>>> its own lock class key for lockdep, preventing false positive warnings
>>>> about lock ordering violations.
>>>>
>>>
>>> Isn't this potentially a problem on the C side as well? `xa_init_flags` is a
>>> static inline function, which means that the lock class is going to be the same
>>> if a single C compilation unit initializes multiple xarrays -- unlike when you
>>> use spin_lock_init, where each callsite gets a different lock class.
>>>
>>> Best,
>>> Gary
>>>
>>>> Add a `new_xarray!` macro that automatically generates a unique lock
>>>> class key for each XArray instantiation site. The macro accepts an
>>>> optional name parameter and uses the `optional_name!` and
>>>> `static_lock_class!` macros to generate appropriate names and lock
>>>> classes.
>>
>> My intuition about this was that when the C static function is inlined, a
>> new static address is used for each place the function is inlined. Is
>> this not correct?
>
> No, the static inside static functions still have the normal static semantics.
> There's a single copy regardless how many times the function is inlined.
>
> So you have a unique copy per compilation unit.

I see. A any rate I get into trouble with lockdep when I lock multiple
different xarrays concurrently in the same code gen unit. I am not sure
how they solve that in C, or if it will also be a problem there.


Best regards,
Andreas Hindborg
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Tamir Duberstein 1 month, 1 week ago
On Wed, Dec 3, 2025 at 5:28 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Replace the `xa_init_flags` helper with direct initialization of XArray
> structures using `__spin_lock_init`. This allows each XArray to have
> its own lock class key for lockdep, preventing false positive warnings
> about lock ordering violations.
>
> Add a `new_xarray!` macro that automatically generates a unique lock
> class key for each XArray instantiation site. The macro accepts an
> optional name parameter and uses the `optional_name!` and
> `static_lock_class!` macros to generate appropriate names and lock
> classes.
>
> Update the `XArray::new` method signature to accept explicit name and
> lock class key parameters. This enables proper lockdep tracking while
> maintaining flexibility for advanced use cases.
>
> Remove `rust_helper_xa_init_flags` as it is now dead code.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/helpers/xarray.c       |  5 ---
>  rust/kernel/xarray.rs       | 74 +++++++++++++++++++++++++++++++++------------
>  rust/kernel/xarray/entry.rs | 55 ++++++++++++++++++---------------
>  3 files changed, 86 insertions(+), 48 deletions(-)
>
> diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
> index 425a6cc494734..2852f63388eea 100644
> --- a/rust/helpers/xarray.c
> +++ b/rust/helpers/xarray.c
> @@ -7,11 +7,6 @@ int rust_helper_xa_err(void *entry)
>         return xa_err(entry);
>  }
>
> -void rust_helper_xa_init_flags(struct xarray *xa, gfp_t flags)
> -{
> -       return xa_init_flags(xa, flags);
> -}
> -
>  int rust_helper_xa_trylock(struct xarray *xa)
>  {
>         return xa_trylock(xa);
> diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs
> index a405f2b6fdcad..c393d4c01af2a 100644
> --- a/rust/kernel/xarray.rs
> +++ b/rust/kernel/xarray.rs
> @@ -29,6 +29,8 @@
>          Result, //
>      },
>      ffi::c_void,
> +    str::CStr,
> +    sync::LockClassKey,
>      types::{
>          ForeignOwnable,
>          NotThreadSafe,
> @@ -48,6 +50,19 @@
>
>  mod preload;
>
> +/// 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
> @@ -62,9 +77,10 @@
>  ///
>  /// ```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: Pin<KBox<XArray<KBox<u32>>>> =
> +///     KBox::pin_init(new_xarray!(AllocKind::Alloc1), GFP_KERNEL)?;
>  ///
>  /// let dead = KBox::new(0xdead, GFP_KERNEL)?;
>  /// let beef = KBox::new(0xbeef, GFP_KERNEL)?;
> @@ -124,7 +140,11 @@ pub enum AllocKind {
>
>  impl<T: ForeignOwnable> XArray<T> {
>      /// Creates a new initializer for this type.

Should this be #[doc(hidden)] now?

> -    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,
> @@ -133,8 +153,14 @@ pub fn new(kind: AllocKind) -> impl PinInit<Self> {
>              // SAFETY: `xa` is valid while the closure is called.
>              //
>              // INVARIANT: `xa` is initialized here to an empty, valid [`bindings::xarray`].
> -            xa <- Opaque::ffi_init(|xa| unsafe {
> -                bindings::xa_init_flags(xa, flags)
> +            xa <- Opaque::ffi_init(|xa: *mut bindings::xarray| unsafe {
> +                bindings::__spin_lock_init(
> +                    &raw mut (*xa).xa_lock,
> +                    name.as_ptr().cast(),
> +                    key.as_ptr(),
> +                );
> +                (*xa).xa_flags = flags;
> +                (*xa).xa_head = null_mut();
>              }),
>              _p: PhantomData,
>          })
> @@ -236,8 +262,12 @@ fn load(&self, index: usize) -> Option<NonNull<c_void>> {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{alloc::{flags::GFP_KERNEL, kbox::KBox}, xarray::{AllocKind, XArray}};
> -    /// let xa = KBox::pin_init(XArray::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{
> +    /// #     alloc::{flags::GFP_KERNEL, kbox::KBox},
> +    /// #     xarray::{new_xarray, AllocKind, XArray},
> +    /// # };
> +    /// let xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      ///
>      /// let mut guard = xa.lock();
>      /// assert_eq!(guard.contains_index(42), false);
> @@ -272,8 +302,9 @@ pub fn get_mut(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// assert_eq!(guard.contains_index(42), false);
> @@ -309,8 +340,9 @@ fn load_next(&self, index: usize) -> Option<(usize, NonNull<c_void>)> {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -339,8 +371,9 @@ pub fn find_next(&self, index: usize) -> Option<(usize, T::Borrowed<'_>)> {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -366,8 +399,9 @@ pub fn find_next_mut(&mut self, index: usize) -> Option<(usize, T::BorrowedMut<'
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(10, KBox::new(10u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -401,8 +435,9 @@ pub fn find_next_entry<'b>(&'b mut self, index: usize) -> Option<OccupiedEntry<'
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(100, KBox::new(42u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -505,8 +540,9 @@ pub fn store(
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// assert_eq!(guard.get(42), None);
> diff --git a/rust/kernel/xarray/entry.rs b/rust/kernel/xarray/entry.rs
> index 2d6ef4781f47d..6e370252309fc 100644
> --- a/rust/kernel/xarray/entry.rs
> +++ b/rust/kernel/xarray/entry.rs
> @@ -26,8 +26,9 @@ impl<T: ForeignOwnable> Entry<'_, '_, T> {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// let entry = guard.get_entry(42);
> @@ -100,17 +101,17 @@ fn insert_internal(
>      ///
>      /// Returns a reference to the newly inserted value.
>      ///
> -    /// - This method will fail if the nodes on the path to the index
> -    ///   represented by this entry are not present in the XArray and no memory
> -    ///   is available via the `preload` argument.
> +    /// - This method will fail if the nodes on the path to the index represented by this entry are
> +    ///   not present in the XArray and no memory is available via the `preload` argument.
>      /// - This method will not drop the XArray lock.
>      ///
>      ///
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// assert_eq!(guard.get(42), None);
> @@ -139,16 +140,16 @@ pub fn insert(
>
>      /// Inserts a value and returns an occupied entry representing the newly inserted value.
>      ///
> -    /// - This method will fail if the nodes on the path to the index
> -    ///   represented by this entry are not present in the XArray and no memory
> -    ///   is available via the `preload` argument.
> +    /// - This method will fail if the nodes on the path to the index represented by this entry are
> +    ///   not present in the XArray and no memory is available via the `preload` argument.
>      /// - This method will not drop the XArray lock.
>      ///
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// assert_eq!(guard.get(42), None);
> @@ -182,8 +183,9 @@ pub fn insert_entry(
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// assert_eq!(guard.get(42), None);
> @@ -221,8 +223,9 @@ pub(crate) fn new(guard: &'b mut Guard<'a, T>, index: usize, ptr: NonNull<c_void
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -266,8 +269,9 @@ pub fn remove(mut self) -> T {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -287,8 +291,9 @@ pub fn index(&self) -> usize {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -334,8 +339,9 @@ pub fn insert(&mut self, value: T) -> T {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(42, KBox::new(0x1337u32, GFP_KERNEL)?, GFP_KERNEL)?;
> @@ -361,8 +367,9 @@ pub fn into_mut(self) -> T::BorrowedMut<'b> {
>      /// # Examples
>      ///
>      /// ```
> -    /// # use kernel::{prelude::*, xarray::{AllocKind, XArray, Entry}};
> -    /// let mut xa = KBox::pin_init(XArray::<KBox<u32>>::new(AllocKind::Alloc), GFP_KERNEL)?;
> +    /// # use kernel::{prelude::*, xarray::{new_xarray, AllocKind, XArray, Entry}};
> +    /// let mut xa: Pin<KBox<XArray<KBox<u32>>>> =
> +    ///     KBox::pin_init(new_xarray!(AllocKind::Alloc), GFP_KERNEL)?;
>      /// let mut guard = xa.lock();
>      ///
>      /// guard.store(42, KBox::new(100u32, GFP_KERNEL)?, GFP_KERNEL)?;
>
> --
> 2.51.2
>
>

Acked-by: Tamir Duberstein <tamird@gmail.com>
Re: [PATCH 10/10] rust: xarray: fix false positive lockdep warnings
Posted by Andreas Hindborg 13 hours ago
Tamir Duberstein <tamird@gmail.com> writes:

> On Wed, Dec 3, 2025 at 5:28 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>

<cut>

>> @@ -62,9 +77,10 @@
>>  ///
>>  /// ```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: Pin<KBox<XArray<KBox<u32>>>> =
>> +///     KBox::pin_init(new_xarray!(AllocKind::Alloc1), GFP_KERNEL)?;
>>  ///
>>  /// let dead = KBox::new(0xdead, GFP_KERNEL)?;
>>  /// let beef = KBox::new(0xbeef, GFP_KERNEL)?;
>> @@ -124,7 +140,11 @@ pub enum AllocKind {
>>
>>  impl<T: ForeignOwnable> XArray<T> {
>>      /// Creates a new initializer for this type.
>
> Should this be #[doc(hidden)] now?

No, I think users should be free to vall this if they want to.


Best regards,
Andreas Hindborg