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

Andreas Hindborg posted 1 patch 15 hours ago
rust/helpers/xarray.c |  5 -----
rust/kernel/xarray.rs | 43 +++++++++++++++++++++++++++++++++++++------
2 files changed, 37 insertions(+), 11 deletions(-)
[PATCH] rust: xarray: fix false positive lockdep warnings
Posted by Andreas Hindborg 15 hours 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.

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")
Acked-by: Tamir Duberstein <tamird@gmail.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/helpers/xarray.c |  5 -----
 rust/kernel/xarray.rs | 43 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
index 60b299f11451d..644b8b35b1bee 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 a49d6db288458..62a2b2ebfb0bf 100644
--- a/rust/kernel/xarray.rs
+++ b/rust/kernel/xarray.rs
@@ -8,11 +8,31 @@
     alloc, bindings, build_assert,
     error::{Error, Result},
     ffi::c_void,
+    str::CStr,
+    sync::LockClassKey,
     types::{ForeignOwnable, NotThreadSafe, Opaque},
 };
-use core::{iter, marker::PhantomData, pin::Pin, ptr::NonNull};
+use core::{
+    iter,
+    marker::PhantomData,
+    pin::Pin,
+    ptr::{null_mut, 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
@@ -27,9 +47,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)?;
@@ -86,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,
@@ -95,8 +120,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,
         })

---
base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
change-id: 20260206-xarray-lockdep-fix-10f1cc68e5d7

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>
Re: [PATCH] rust: xarray: fix false positive lockdep warnings
Posted by Tamir Duberstein 15 hours ago
On Fri, Feb 6, 2026 at 12:11 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.
>
> 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")
> Acked-by: Tamir Duberstein <tamird@gmail.com>

Please replace with:

Acked-by: Tamir Duberstein <tamird@kernel.org>

> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/helpers/xarray.c |  5 -----
>  rust/kernel/xarray.rs | 43 +++++++++++++++++++++++++++++++++++++------
>  2 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
> index 60b299f11451d..644b8b35b1bee 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);
> -}

I like Alice's suggestion in the previous version of doing the
initialization here. It gives a natural place to explain why
xa_init_flags cannot be used directly and avoids introducing a second
user of `bindings:: __spin_lock_init` which seems like something we
should discourage direct use of.

> -
>  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 a49d6db288458..62a2b2ebfb0bf 100644
> --- a/rust/kernel/xarray.rs
> +++ b/rust/kernel/xarray.rs
> @@ -8,11 +8,31 @@
>      alloc, bindings, build_assert,
>      error::{Error, Result},
>      ffi::c_void,
> +    str::CStr,
> +    sync::LockClassKey,
>      types::{ForeignOwnable, NotThreadSafe, Opaque},
>  };
> -use core::{iter, marker::PhantomData, pin::Pin, ptr::NonNull};
> +use core::{
> +    iter,
> +    marker::PhantomData,
> +    pin::Pin,
> +    ptr::{null_mut, 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
> @@ -27,9 +47,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)?;
> @@ -86,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,
> @@ -95,8 +120,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,
>          })
>
> ---
> base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
> change-id: 20260206-xarray-lockdep-fix-10f1cc68e5d7
>
> Best regards,
> --
> Andreas Hindborg <a.hindborg@kernel.org>
>
>
Re: [PATCH] rust: xarray: fix false positive lockdep warnings
Posted by Gary Guo 15 hours ago
On Fri Feb 6, 2026 at 5:19 PM GMT, Tamir Duberstein wrote:
> On Fri, Feb 6, 2026 at 12:11 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.
>>
>> 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")
>> Acked-by: Tamir Duberstein <tamird@gmail.com>
>
> Please replace with:
>
> Acked-by: Tamir Duberstein <tamird@kernel.org>
>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>>  rust/helpers/xarray.c |  5 -----
>>  rust/kernel/xarray.rs | 43 +++++++++++++++++++++++++++++++++++++------
>>  2 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c
>> index 60b299f11451d..644b8b35b1bee 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);
>> -}
>
> I like Alice's suggestion in the previous version of doing the
> initialization here. It gives a natural place to explain why
> xa_init_flags cannot be used directly and avoids introducing a second
> user of `bindings:: __spin_lock_init` which seems like something we
> should discourage direct use of.

Or perhaps we should just add __xa_init_flags which accepts explicit lock
classes, and replace `xa_init_flags` to use it with a fresh lock class key.

I think C users benefit from having a fresh lock class key per xarray rather
than a single one per compilation unit, too.

Cc: Matthew Wilcox <willy@infradead.org>

Matthew, for context, the current xa_init_flags have an issue where, because it
is static inline function instead of macro, all users inside a single
compilation unit ended up get a single lock class key. For C users this means
that each .c file gets a single key, and this is more problematic for Rust as we
invoke via helpers, so all of Rust users share a single lockdep key.

Link: https://lore.kernel.org/all/DFUI6LMASTMK.2WI00LMY5FTSP@garyguo.net/

Best,
Gary

>
>> -
>>  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 a49d6db288458..62a2b2ebfb0bf 100644
>> --- a/rust/kernel/xarray.rs
>> +++ b/rust/kernel/xarray.rs
>> @@ -8,11 +8,31 @@
>>      alloc, bindings, build_assert,
>>      error::{Error, Result},
>>      ffi::c_void,
>> +    str::CStr,
>> +    sync::LockClassKey,
>>      types::{ForeignOwnable, NotThreadSafe, Opaque},
>>  };
>> -use core::{iter, marker::PhantomData, pin::Pin, ptr::NonNull};
>> +use core::{
>> +    iter,
>> +    marker::PhantomData,
>> +    pin::Pin,
>> +    ptr::{null_mut, 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
>> @@ -27,9 +47,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)?;
>> @@ -86,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,
>> @@ -95,8 +120,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,
>>          })
>>
>> ---
>> base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
>> change-id: 20260206-xarray-lockdep-fix-10f1cc68e5d7
>>
>> Best regards,
>> --
>> Andreas Hindborg <a.hindborg@kernel.org>
>>
>>