Re: [PATCH RFC 2/2] lockdep: delegate Rust lock class printing to Rust code
From: lyude
Date: Tue Jul 07 2026 - 17:25:14 EST
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@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> + #[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 }),
> /// })
> /// }
> /// }