Re: [PATCH 1/2] rust: sync: refactor static_lock_class!() macro

["Benno Lossin" <lossin@kernel.org>]

On Wed Jul 23, 2025 at 10:07 PM CEST, Boqun Feng wrote:
> On Wed, Jul 23, 2025 at 09:46:03PM +0200, Benno Lossin wrote:
>> On Wed Jul 23, 2025 at 6:20 PM CEST, Boqun Feng wrote:
>> > On Wed, Jul 23, 2025 at 05:01:39PM +0200, Alice Ryhl wrote:
>> >> On Wed, Jul 23, 2025 at 4:36 PM Benno Lossin <lossin@kernel.org> wrote:
>> >> > On Wed Jul 23, 2025 at 1:49 PM CEST, Alice Ryhl wrote:
>> >> > >  impl LockClassKey {
>> >> > > +    /// Initializes a statically allocated lock class key.
>> >> > > +    ///
>> >> > > +    /// This is usually used indirectly through the [`static_lock_class!`] macro.
>> >> > > +    ///
>> >> > > +    /// # Safety
>> >> > > +    ///
>> >> > > +    /// The destructor must never run on the returned `LockClassKey`.
>> >> >
>> >> > I don't know how lockdep works, but Boqun mentioned in the other thread
>> >> > that it uses the address of static keys. But AFAIK there is no mechanism
>> >> > to differentiate them, so does lockdep just check the address and if it
>> >
>> > In lockdep, we use `static_obj()` to tell whether it's a static obj or a
>> > dynamic allocated one.
>> 
>> So the code below will go in the non-static code path. Why doesn't it
>> need to be initialized/registered? (but other cases need it?)
>> 
>
> Becasue all the dynamic lock class keys are put in a hash list (using an
> intrusive single linked list), so you have to register it before use and
> unregister after use.

Gotcha.

>> >> > is in a static segment it uses different behavior?
>> >> >
>> >> > Because from the safety requirements on this function, I could just do
>> >> > this:
>> >> >
>> >> >     // SAFETY: we leak the box below, so the destructor never runs.
>> >> >     let class = KBox::new(unsafe { LockClassKey::new_static() });
>> >> >     let class = Pin::static_ref(KBox::leak(class));
>> >> >     let lock = SpinLock::new(42, c_str!("test"), class);
>> >
>> > This will trigger a runtime error because `class` is not static, but
>> > technically, it won't trigger UB, at least lockdep should be able to
>> > handle this case.
>> 
>> Could you go into more details? What is the "technically it won't
>> trigger UB" part about?
>> 
>
> If a dynamic key is not registered, lockdep will simply just skip the
> initialization of locks, report an error and disable itself entirely. So
> it won't cause UB.

So the code above would trigger lockdep to disable itself?

And how does it detect that the class isn't registered? By checking for
the address in the hash list? Otherwise it would be UB, right? Could
there be a hash collision that induces UB?

---
Cheers,
Benno