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

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

On Mon Jul 28, 2025 at 11:42 AM CEST, Alice Ryhl wrote:
> By introducing a new_static() constructor, the macro does not need to go
> through MaybeUninit::uninit().assume_init(), which is a pattern that is
> best avoided when possible.
>
> The safety comment requires not only requires that the value is leaked,

"requires" appears twice.

> but also that it is stored in the right portion of memory. This is so
> that the lockdep static_obj() check will succeed when using this
> constructor. One could argue that lockdep detects this scenario, so the
> safety comment isn't needed. However, it simplifies matters to require
> that static_obj() will succeed and it's not a burdensome requirement on
> the caller.

I'd argue that's implementation detail and the safety requirement of
using a lockclass key is that it either is uninit in static memory or it
was registered. (otherwise we wouldn't be "allowed" to add this as a
safety requirement)

(just adding this for info, feel free to keep the paragraph above as-is)

> Suggested-by: Benno Lossin <lossin@kernel.org>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Benno Lossin <lossin@kernel.org>

> @@ -95,13 +110,11 @@ fn drop(self: Pin<&mut Self>) {
>  #[macro_export]
>  macro_rules! static_lock_class {
>      () => {{
> -        static CLASS: $crate::sync::LockClassKey =
> -            // Lockdep expects uninitialized memory when it's handed a statically allocated `struct
> -            // lock_class_key`.
> -            //
> -            // SAFETY: `LockClassKey` transparently wraps `Opaque` which permits uninitialized
> -            // memory.
> -            unsafe { ::core::mem::MaybeUninit::uninit().assume_init() };
> +        // SAFETY: The returned `LockClassKey` is stored in static memory. Drop never runs on a

You're not mentioning the "pinned in a static memory location" part
(only the static memory, so missing the pinning). A read-only static
is implicitly pinned, so we should mention that.

---
Cheers,
Benno

> +        // static global.
> +        static CLASS: $crate::sync::LockClassKey = unsafe {
> +            $crate::sync::LockClassKey::new_static()
> +        };
>          $crate::prelude::Pin::static_ref(&CLASS)
>      }};
>  }