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,
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.

Suggested-by: Benno Lossin <lossin@kernel.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/sync.rs | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 00f9b558a3ade19e442b32b46d05885b67e1d830..2c04a3806ca885637583fde336d054426f569fe5 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -39,6 +39,21 @@ pub struct LockClassKey {
 unsafe impl Sync for LockClassKey {}
 
 impl LockClassKey {
+    /// Initializes a statically allocated lock class key.
+    ///
+    /// This is usually used indirectly through the [`static_lock_class!`] macro.
+    ///
+    /// # Safety
+    ///
+    /// * Before using the returned value, it must be pinned in a static memory location.
+    /// * The destructor must never run on the returned `LockClassKey`.
+    #[doc(hidden)]
+    pub const unsafe fn new_static() -> Self {
+        LockClassKey {
+            inner: Opaque::uninit(),
+        }
+    }
+
     /// Initializes a dynamically allocated lock class key. In the common case of using a
     /// statically allocated lock class key, the static_lock_class! macro should be used instead.
     ///
@@ -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
+        // static global.
+        static CLASS: $crate::sync::LockClassKey = unsafe {
+            $crate::sync::LockClassKey::new_static()
+        };
         $crate::prelude::Pin::static_ref(&CLASS)
     }};
 }

-- 
2.50.1.470.g6ba607880d-goog

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)
>      }};
>  }