Re: [PATCH RFC 2/2] lockdep: delegate Rust lock class printing to Rust code

[lyude@redhat.com]

This looks great, one comment below:

On Fri, 2026-07-03 at 14:47 +0100, Gary Guo wrote:
> Add a special "(rust)" name which means that the lock class comes
> from Rust
> `#[track_caller]` and thus should be delegated to Rust code to print
> the
> name.
> 
> This allows locks to be created with usual constructor syntax and
> does not
> need macros anymore.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  kernel/locking/lockdep.c           |  7 ++++++-
>  kernel/locking/lockdep_internals.h |  3 +++
>  rust/kernel/sync.rs                | 24 ++++++++++++++++++++++++
>  rust/kernel/sync/lock.rs           |  8 ++++++++
>  rust/kernel/sync/lock/mutex.rs     |  8 ++++----
>  rust/kernel/sync/lock/spinlock.rs  |  8 ++++----
>  6 files changed, 49 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 2d4c5bab5af8..1e05d51e710f 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -724,7 +724,12 @@ static void __print_lock_name(struct held_lock
> *hlock, struct lock_class *class)
>  		name = __get_key_name(class->key, str);
>  		printk(KERN_CONT "%s", name);
>  	} else {
> -		printk(KERN_CONT "%s", name);
> +		if (CONFIG_RUST && class->name == lockdep_rust_name)
> +			lockdep_print_rust_name((struct
> lock_class_key *)(
> +				class->key - class->subclass
> +			));
> +		else
> +			printk(KERN_CONT "%s", name);
>  		if (class->name_version > 1)
>  			printk(KERN_CONT "#%d", class-
> >name_version);
>  		if (class->subclass)
> diff --git a/kernel/locking/lockdep_internals.h
> b/kernel/locking/lockdep_internals.h
> index 0e5e6ffe91a3..d18d50f7fd99 100644
> --- a/kernel/locking/lockdep_internals.h
> +++ b/kernel/locking/lockdep_internals.h
> @@ -134,6 +134,9 @@ extern void get_usage_chars(struct lock_class
> *class,
>  extern const char *__get_key_name(const struct lockdep_subclass_key
> *key,
>  				  char *str);
>  
> +extern const char lockdep_rust_name[];
> +extern void lockdep_print_rust_name(struct lock_class_key *key);
> +
>  struct lock_class *lock_chain_get_class(struct lock_chain *chain,
> int i);
>  
>  extern unsigned long nr_lock_classes;
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index cf76fb37c460..cf35858469d1 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -8,6 +8,7 @@
>  use core::panic::Location;
>  
>  use crate::{
> +    pr_cont,
>      prelude::*,
>      types::Opaque, //
>  };
> @@ -150,6 +151,29 @@ fn drop(self: Pin<&mut Self>) {
>      }
>  }
>  
> +// We want this to be completely unique so lockdep can identify when
> lock classes are generated with
> +// `new_static` mechanism, hence making this `static` and attach a
> `link_section` so it cannot be
> +// merged with other constants.
> +#[export_name = "lockdep_rust_name"]
> +#[link_section = ".rodata"]
> +static LOCKDEP_RUST_NAME_BYTES: [u8; 7] = *b"(rust)\0";
> +
> +// This should only be used in conjunction of
> `LockClassKey::from_caller`.
> +const LOCKDEP_RUST_NAME: &CStr = match
> CStr::from_bytes_with_nul(&LOCKDEP_RUST_NAME_BYTES) {
> +    Ok(v) => v,
> +    Err(_) => unreachable!(),
> +};
> +
> +#[cfg(CONFIG_LOCKDEP)]
> +#[expect(clippy::missing_safety_doc)]
> +#[no_mangle]
> +unsafe extern "C" fn lockdep_print_rust_name(key: *mut
> bindings::lock_class_key) {
> +    // SAFETY: `rust_print_lockdep_name` is called when the lock
> name is a reserved value indicating
> +    // that the lock_class_key is static and backed by a `Location`.
> +    let location = unsafe { &*key.cast::<Location<'_>>() };
> +    pr_cont!("{}:{}", location.file(), location.line());
> +}
> +
>  /// Defines a new static lock class and returns a pointer to it.
>  ///
>  /// # Examples
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index 447fec291cdf..66d51c860483 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -127,6 +127,14 @@ unsafe impl<T: ?Sized + Send, B: Backend> Send
> for Lock<T, B> {}
>  unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
>  
>  impl<T, B: Backend> Lock<T, B> {
> +    /// Constructs a new lock initialiser with a custom name.

I think this rustdoc is a bit confusing: we say "with a custom name"
but this function doesn't provide any way of actually specifying a
name. Maybe this would make more sense?

"Constructs a new lock initialiser using an auto-generated name."

With that fixed:

Reviewed-by: Lyude Paul <lyude@redhat.com>

> +    #[inline]
> +    #[track_caller]
> +    pub fn new(t: impl PinInit<T>) -> impl PinInit<Self> {
> +        let key = LockClassKey::from_caller();
> +        Self::new_with_lock_class(t, super::LOCKDEP_RUST_NAME, key)
> +    }
> +
>      /// Constructs a new lock initialiser with a custom name.
>      #[inline]
>      #[track_caller]
> diff --git a/rust/kernel/sync/lock/mutex.rs
> b/rust/kernel/sync/lock/mutex.rs
> index 3675ce244e08..c730196b9d2d 100644
> --- a/rust/kernel/sync/lock/mutex.rs
> +++ b/rust/kernel/sync/lock/mutex.rs
> @@ -24,8 +24,8 @@ macro_rules! new_mutex {
>  ///
>  /// Since it may block, [`Mutex`] needs to be used with care in
> atomic contexts.
>  ///
> -/// Instances of [`Mutex`] need a lock class and to be pinned. The
> recommended way to create such
> -/// instances is with the [`pin_init`](pin_init::pin_init) and
> [`new_mutex`] macros.
> +/// Instances of [`Mutex`] need to be pinned. You can create such
> instances with the
> +/// [`pin_init`](pin_init::pin_init).
>  ///
>  /// # Examples
>  ///
> @@ -33,7 +33,7 @@ macro_rules! new_mutex {
>  /// contains an inner struct (`Inner`) that is protected by a mutex.
>  ///
>  /// ```
> -/// use kernel::sync::{new_mutex, Mutex};
> +/// use kernel::sync::Mutex;
>  ///
>  /// struct Inner {
>  ///     a: u32,
> @@ -51,7 +51,7 @@ macro_rules! new_mutex {
>  ///     fn new() -> impl PinInit<Self> {
>  ///         pin_init!(Self {
>  ///             c: 10,
> -///             d <- new_mutex!(Inner { a: 20, b: 30 }),
> +///             d <- Mutex::new(Inner { a: 20, b: 30 }),
>  ///         })
>  ///     }
>  /// }
> diff --git a/rust/kernel/sync/lock/spinlock.rs
> b/rust/kernel/sync/lock/spinlock.rs
> index 091167ceda66..f8bec26f1cee 100644
> --- a/rust/kernel/sync/lock/spinlock.rs
> +++ b/rust/kernel/sync/lock/spinlock.rs
> @@ -22,8 +22,8 @@ macro_rules! new_spinlock {
>  /// one at a time is allowed to progress, the others will block
> (spinning) until the spinlock is
>  /// unlocked, at which point another CPU will be allowed to make
> progress.
>  ///
> -/// Instances of [`SpinLock`] need a lock class and to be pinned.
> The recommended way to create such
> -/// instances is with the [`pin_init`](pin_init::pin_init) and
> [`new_spinlock`] macros.
> +/// Instances of [`SpinLock`] need to be pinned. You can create such
> instances with the
> +/// [`pin_init`](pin_init::pin_init).
>  ///
>  /// # Examples
>  ///
> @@ -31,7 +31,7 @@ macro_rules! new_spinlock {
>  /// contains an inner struct (`Inner`) that is protected by a
> spinlock.
>  ///
>  /// ```
> -/// use kernel::sync::{new_spinlock, SpinLock};
> +/// use kernel::sync::SpinLock;
>  ///
>  /// struct Inner {
>  ///     a: u32,
> @@ -49,7 +49,7 @@ macro_rules! new_spinlock {
>  ///     fn new() -> impl PinInit<Self> {
>  ///         pin_init!(Self {
>  ///             c: 10,
> -///             d <- new_spinlock!(Inner { a: 20, b: 30 }),
> +///             d <- SpinLock::new(Inner { a: 20, b: 30 }),
>  ///         })
>  ///     }
>  /// }