Re: [PATCH RFC 1/2] rust: sync: introduce a way to create lock class from caller

["Gary Guo" <gary@garyguo.net>]

On Fri Jul 3, 2026 at 3:01 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Creating a reference with interior mutability (`UnsafeCell`) from a read-only reference without interior mutability is Undefined Behavior.
> - [High] Distinct locks initialized within the same macro expansion will incorrectly share the same lock class, causing false positive deadlock reports.
> - [Medium] The `#[track_caller]` attribute unconditionally generates `Location` statics even when `CONFIG_LOCKDEP` is disabled, defeating the stated intent.
> --
>
> commit dd5e5262e2385533a2fc8caffa1b94a916bed484
> Author: Gary Guo <gary@garyguo.net>
>
> rust: sync: introduce a way to create lock class from caller
>
> This patch introduces a mechanism to implicitly create lock classes using
> Rust's track_caller feature. It adds LockClassKey::from_caller to generate a
> unique lock class key from the caller's location, simplifying lock
> initialization by removing the need for macros.
>
>> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
>> --- a/rust/kernel/sync.rs
>> +++ b/rust/kernel/sync.rs
>
> [ ... ]
>
>> @@ -66,6 +70,31 @@ impl LockClassKey {
>> +    #[inline]
>> +    #[track_caller]
>> +    pub const fn from_caller() -> Pin<&'static Self> {
>
> [Severity: Medium]
> Since the track_caller attribute unconditionally forces the caller to generate
> and pass a Location static, does this defeat the intent of the branch below
> that avoids generating Location when CONFIG_LOCKDEP is disabled?

Not true.

>
> Could this use cfg_attr(CONFIG_LOCKDEP, track_caller) to prevent the
> generation at the language level when lockdep is off?
>
>> +        static_assert!(size_of::<Location<'_>>() >= size_of::<LockClassKey>());
>> +
>> +        #[cfg(CONFIG_LOCKDEP)]
>> +        {
>> +            let caller = Location::caller();
>
> [Severity: High]
> If multiple locks are initialized within the same macro expansion, will they
> incorrectly share the same lock class?
>
> Because track_caller assigns the macro invocation's span to all tokens
> generated by it, Location::caller() will return the exact same memory address
> for all locks instantiated by that macro. Could this cause lockdep to falsely
> report recursive deadlocks when acquiring distinct locks from the same macro?

The only macro that needs this is DelayedWork. Two lock classes can be created
per `Location`, which should be sufficient (see cover letter).

>
>> +            // SAFETY: For static objects, lockdep does not touch the underlying memory, and only
>> +            // the address matters. `Location::caller()` is in rodata so it meets the requirement.
>> +            // The size check above makes sure that it does not overlap with other lock class keys.
>> +            unsafe { Pin::new_unchecked(&*core::ptr::from_ref(caller).cast::<Self>()) }
>
> [Severity: High]
> Can this cast trigger undefined behavior under Rust's aliasing rules?
>
> Location is a read-only struct in .rodata with no interior mutability, while
> LockClassKey contains an Opaque that wraps UnsafeCell. Upgrading a read-only
> provenance to a shared read-write provenance (which UnsafeCell implies) is
> immediate undefined behavior, even if the memory is never mutated.

This is https://github.com/rust-lang/unsafe-code-guidelines/issues/303 and it's
allowed by tree borrow.

>
>> +        }
>> +
>> +        // A separate version if lockdep is disabled to avoid unnecessary `Location` being
>> +        // generated.
>> +        #[cfg(not(CONFIG_LOCKDEP))]
>> +        static_lock_class!()
>> +    }
>
> [ ... ]
>
>> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
>> --- a/rust/kernel/sync/lock.rs
>> +++ b/rust/kernel/sync/lock.rs
>
> [ ... ]
>
>> @@ -127,8 +127,17 @@ 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.
>> -    pub fn new(
>> +    /// Constructs a new lock initialiser with a custom name.
>> +    #[inline]
>> +    #[track_caller]
>> +    pub fn new_with_name(t: impl PinInit<T>, name: &'static CStr) -> impl PinInit<Self> {
>
> [Severity: Medium]
> Similar to the issue in from_caller(), does this unconditionally propagate
> Location generation up to the user even when CONFIG_LOCKDEP is disabled?
>
> Could this also be conditionally compiled with cfg_attr?

False positive.

Best,
Gary

>
>> +        let key = LockClassKey::from_caller();
>> +        Self::new_with_lock_class(t, name, key)
>> +    }