[PATCH 0/2] Clean up Rust LockClassKey

[PATCH 0/2] Clean up Rust LockClassKey

[Alice Ryhl]

This series applies the suggestion from Benno [1] and various other
improvements I found when looking over the LockClassKey implementation.

Based on rust-next.

[1]: https://lore.kernel.org/all/DBIJLR7XNI6U.21PMPODHE83DZ@kernel.org/

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (2):
      rust: sync: refactor static_lock_class!() macro
      rust: sync: clean up LockClassKey and its docs

 rust/kernel/sync.rs | 77 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 20 deletions(-)
---
base-commit: dff64b072708ffef23c117fa1ee1ea59eb417807
change-id: 20250723-lock-class-key-cleanup-a3baf53b123a

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

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

[Alice Ryhl]

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.

That the destructor must never run is a sufficient safety requirement
for new_static() because to actually use it you must also pin it.
This implies that the memory location remains valid forever, which is
what is needed when using a statically allocated lock class key.

Suggested-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/sync.rs | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 00f9b558a3ade19e442b32b46d05885b67e1d830..9545bedf47b67976ab8c22d8368991cf1f382e42 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -39,6 +39,20 @@ 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
+    ///
+    /// 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 +109,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, so its destructor will
+        // not run.
+        static CLASS: $crate::sync::LockClassKey = unsafe {
+            $crate::sync::LockClassKey::new_static()
+        };
         $crate::prelude::Pin::static_ref(&CLASS)
     }};
 }

-- 
2.50.0.727.gbf7dc18ff4-goog

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

[Benno Lossin]

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
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);
    let _ = lock.lock();

Because if lockdep then expects this to be initialized, we need to
change the requirement to only be used from statics.

---
Cheers,
Benno

> +    #[doc(hidden)]
> +    pub const unsafe fn new_static() -> Self {
> +        LockClassKey {
> +            inner: Opaque::uninit(),
> +        }
> +    }

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

[Alice Ryhl]

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
> 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);
>     let _ = lock.lock();
>
> Because if lockdep then expects this to be initialized, we need to
> change the requirement to only be used from statics.

My understanding is that it has to with correctly handling reuse of
the same key. In your scenario it's not reused.

Alice

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

[Boqun Feng]

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.

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

Regards,
Boqun

> >     let _ = lock.lock();
> >
> > Because if lockdep then expects this to be initialized, we need to
> > change the requirement to only be used from statics.
> 
> My understanding is that it has to with correctly handling reuse of
> the same key. In your scenario it's not reused.
> 
> Alice

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

[Benno Lossin]

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?)

>> > 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?

---
Cheers,
Benno

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

[Boqun Feng]

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.

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

Regards,
Boqun

> ---
> Cheers,
> Benno

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

[Benno Lossin]

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

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

[Boqun Feng]

On Wed, Jul 23, 2025 at 10:41:18PM +0200, Benno Lossin wrote:
[...]
> >> >> > 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?
> 

Yes.

> And how does it detect that the class isn't registered? By checking for
> the address in the hash list?

Right, in is_dynamice_key(), the hash list is scanned to see the key has
been registered.

> Otherwise it would be UB, right? Could there be a hash collision that
> induces UB?
> 

I don't think a hash collision will cause an UB, because the checking
only uses the address of the key, so even the key is uninitialized, it's
fine.

Regards,
Boqun

> ---
> Cheers,
> Benno

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

[Alice Ryhl]

On Wed, Jul 23, 2025 at 6:20 PM Boqun Feng <boqun.feng@gmail.com> 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.

Oh, ok. In that case I will adjust the safety comment.

Alice

[PATCH 2/2] rust: sync: clean up LockClassKey and its docs

[Alice Ryhl]

Several aspects of the code and documentation for this type is
incomplete. Also several things are hidden from the docs. Thus, clean it
up and make it easier to read the rendered html docs.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/sync.rs | 55 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 9545bedf47b67976ab8c22d8368991cf1f382e42..5019a0bc95446fe30bad02ce040a1cbbe6d9ad5b 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -26,7 +26,9 @@
 pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
 pub use locked_by::LockedBy;
 
-/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
+/// Represents a lockdep class.
+///
+/// Wraps the kernel's `struct lock_class_key`.
 #[repr(transparent)]
 #[pin_data(PinnedDrop)]
 pub struct LockClassKey {
@@ -34,6 +36,10 @@ pub struct LockClassKey {
     inner: Opaque<bindings::lock_class_key>,
 }
 
+// SAFETY: Unregistering a lock class key from a different thread than where it was registered is
+// allowed.
+unsafe impl Send for LockClassKey {}
+
 // SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
 // provides its own synchronization.
 unsafe impl Sync for LockClassKey {}
@@ -41,28 +47,30 @@ 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.
+    /// This is usually used indirectly through the [`static_lock_class!`] macro. See its
+    /// documentation for more information.
     ///
     /// # Safety
     ///
     /// 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.
+    /// 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.
     ///
     /// # Examples
+    ///
     /// ```
-    /// # use kernel::c_str;
-    /// # use kernel::alloc::KBox;
-    /// # use kernel::types::ForeignOwnable;
-    /// # use kernel::sync::{LockClassKey, SpinLock};
-    /// # use pin_init::stack_pin_init;
+    /// use kernel::c_str;
+    /// use kernel::types::ForeignOwnable;
+    /// use kernel::sync::{LockClassKey, SpinLock};
+    /// use pin_init::stack_pin_init;
     ///
     /// let key = KBox::pin_init(LockClassKey::new_dynamic(), GFP_KERNEL)?;
     /// let key_ptr = key.into_foreign();
@@ -80,7 +88,6 @@ impl LockClassKey {
     /// // SAFETY: We dropped `num`, the only use of the key, so the result of the previous
     /// // `borrow` has also been dropped. Thus, it's safe to use from_foreign.
     /// unsafe { drop(<Pin<KBox<LockClassKey>> as ForeignOwnable>::from_foreign(key_ptr)) };
-    ///
     /// # Ok::<(), Error>(())
     /// ```
     pub fn new_dynamic() -> impl PinInit<Self> {
@@ -90,7 +97,10 @@ pub fn new_dynamic() -> impl PinInit<Self> {
         })
     }
 
-    pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
+    /// Returns a raw pointer to the inner C struct.
+    ///
+    /// It is up to the caller to use the raw pointer correctly.
+    pub fn as_ptr(&self) -> *mut bindings::lock_class_key {
         self.inner.get()
     }
 }
@@ -98,14 +108,28 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
 #[pinned_drop]
 impl PinnedDrop for LockClassKey {
     fn drop(self: Pin<&mut Self>) {
-        // SAFETY: self.as_ptr was registered with lockdep and self is pinned, so the address
-        // hasn't changed. Thus, it's safe to pass to unregister.
+        // SAFETY: `self.as_ptr()` was registered with lockdep and `self` is pinned, so the address
+        // hasn't changed. Thus, it's safe to pass it to unregister.
         unsafe { bindings::lockdep_unregister_key(self.as_ptr()) }
     }
 }
 
 /// Defines a new static lock class and returns a pointer to it.
-#[doc(hidden)]
+///
+/// # Examples
+///
+/// ```
+/// use kernel::c_str;
+/// use kernel::sync::{static_lock_class, Arc, SpinLock};
+///
+/// fn new_locked_int() -> Result<Arc<SpinLock<u32>>> {
+///     Arc::pin_init(SpinLock::new(
+///         42,
+///         c_str!("new_locked_int"),
+///         static_lock_class!(),
+///     ), GFP_KERNEL)
+/// }
+/// ```
 #[macro_export]
 macro_rules! static_lock_class {
     () => {{
@@ -117,6 +141,7 @@ macro_rules! static_lock_class {
         $crate::prelude::Pin::static_ref(&CLASS)
     }};
 }
+pub use static_lock_class;
 
 /// Returns the given string, if one is provided, otherwise generates one based on the source code
 /// location.

-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH 2/2] rust: sync: clean up LockClassKey and its docs

[Boqun Feng]

On Wed, Jul 23, 2025 at 11:49:34AM +0000, Alice Ryhl wrote:
> Several aspects of the code and documentation for this type is
> incomplete. Also several things are hidden from the docs. Thus, clean it
> up and make it easier to read the rendered html docs.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---

This looks good to me. One thing below:

>  rust/kernel/sync.rs | 55 ++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 40 insertions(+), 15 deletions(-)
> 
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 9545bedf47b67976ab8c22d8368991cf1f382e42..5019a0bc95446fe30bad02ce040a1cbbe6d9ad5b 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -26,7 +26,9 @@
>  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
>  pub use locked_by::LockedBy;
>  
> -/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> +/// Represents a lockdep class.
> +///
> +/// Wraps the kernel's `struct lock_class_key`.
>  #[repr(transparent)]
>  #[pin_data(PinnedDrop)]
>  pub struct LockClassKey {
> @@ -34,6 +36,10 @@ pub struct LockClassKey {
>      inner: Opaque<bindings::lock_class_key>,
>  }
>  
> +// SAFETY: Unregistering a lock class key from a different thread than where it was registered is
> +// allowed.
> +unsafe impl Send for LockClassKey {}
> +
>  // SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
>  // provides its own synchronization.
>  unsafe impl Sync for LockClassKey {}
> @@ -41,28 +47,30 @@ 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.
> +    /// This is usually used indirectly through the [`static_lock_class!`] macro. See its
> +    /// documentation for more information.
>      ///
>      /// # Safety
>      ///
>      /// 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.
> +    /// 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.
>      ///
>      /// # Examples
> +    ///
>      /// ```
> -    /// # use kernel::c_str;
> -    /// # use kernel::alloc::KBox;
> -    /// # use kernel::types::ForeignOwnable;
> -    /// # use kernel::sync::{LockClassKey, SpinLock};
> -    /// # use pin_init::stack_pin_init;
> +    /// use kernel::c_str;

We can probably change the use `optional_name!()` to make
core::ffi::CStr -> kernel::str::CStr more smooth.

> +    /// use kernel::types::ForeignOwnable;
> +    /// use kernel::sync::{LockClassKey, SpinLock};
> +    /// use pin_init::stack_pin_init;
>      ///
>      /// let key = KBox::pin_init(LockClassKey::new_dynamic(), GFP_KERNEL)?;
>      /// let key_ptr = key.into_foreign();
> @@ -80,7 +88,6 @@ impl LockClassKey {
>      /// // SAFETY: We dropped `num`, the only use of the key, so the result of the previous
>      /// // `borrow` has also been dropped. Thus, it's safe to use from_foreign.
>      /// unsafe { drop(<Pin<KBox<LockClassKey>> as ForeignOwnable>::from_foreign(key_ptr)) };
> -    ///
>      /// # Ok::<(), Error>(())
>      /// ```
>      pub fn new_dynamic() -> impl PinInit<Self> {
> @@ -90,7 +97,10 @@ pub fn new_dynamic() -> impl PinInit<Self> {
>          })
>      }
>  
> -    pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> +    /// Returns a raw pointer to the inner C struct.
> +    ///
> +    /// It is up to the caller to use the raw pointer correctly.
> +    pub fn as_ptr(&self) -> *mut bindings::lock_class_key {
>          self.inner.get()
>      }
>  }
> @@ -98,14 +108,28 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
>  #[pinned_drop]
>  impl PinnedDrop for LockClassKey {
>      fn drop(self: Pin<&mut Self>) {
> -        // SAFETY: self.as_ptr was registered with lockdep and self is pinned, so the address
> -        // hasn't changed. Thus, it's safe to pass to unregister.
> +        // SAFETY: `self.as_ptr()` was registered with lockdep and `self` is pinned, so the address
> +        // hasn't changed. Thus, it's safe to pass it to unregister.
>          unsafe { bindings::lockdep_unregister_key(self.as_ptr()) }
>      }
>  }
>  
>  /// Defines a new static lock class and returns a pointer to it.
> -#[doc(hidden)]
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::c_str;
> +/// use kernel::sync::{static_lock_class, Arc, SpinLock};
> +///
> +/// fn new_locked_int() -> Result<Arc<SpinLock<u32>>> {
> +///     Arc::pin_init(SpinLock::new(
> +///         42,
> +///         c_str!("new_locked_int"),

We could use `optional_name!()` here to avoid another usage of
`c_str!()`.

That said, I'm not sure whether we should replace `c_str!()` in the
example of `new_dynamic()` right now in this series, I think that
depends on two things: 1) whether this series goes into tip or rust-next
for 6.18 and 2) when we are expecting to land the replacement of
`c_str!()`.

Miguel and Tamir, any thought?

Regards,
Boqun

> +///         static_lock_class!(),
> +///     ), GFP_KERNEL)
> +/// }
> +/// ```
>  #[macro_export]
>  macro_rules! static_lock_class {
>      () => {{
> @@ -117,6 +141,7 @@ macro_rules! static_lock_class {
>          $crate::prelude::Pin::static_ref(&CLASS)
>      }};
>  }
> +pub use static_lock_class;
>  
>  /// Returns the given string, if one is provided, otherwise generates one based on the source code
>  /// location.
> 
> -- 
> 2.50.0.727.gbf7dc18ff4-goog
> 

Re: [PATCH 2/2] rust: sync: clean up LockClassKey and its docs

[Tamir Duberstein]

On Wed, Jul 23, 2025 at 9:49 AM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Wed, Jul 23, 2025 at 11:49:34AM +0000, Alice Ryhl wrote:
> > Several aspects of the code and documentation for this type is
> > incomplete. Also several things are hidden from the docs. Thus, clean it
> > up and make it easier to read the rendered html docs.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
>
> This looks good to me. One thing below:
>
> >  rust/kernel/sync.rs | 55 ++++++++++++++++++++++++++++++++++++++---------------
> >  1 file changed, 40 insertions(+), 15 deletions(-)
> >
> > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > index 9545bedf47b67976ab8c22d8368991cf1f382e42..5019a0bc95446fe30bad02ce040a1cbbe6d9ad5b 100644
> > --- a/rust/kernel/sync.rs
> > +++ b/rust/kernel/sync.rs
> > @@ -26,7 +26,9 @@
> >  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
> >  pub use locked_by::LockedBy;
> >
> > -/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> > +/// Represents a lockdep class.
> > +///
> > +/// Wraps the kernel's `struct lock_class_key`.
> >  #[repr(transparent)]
> >  #[pin_data(PinnedDrop)]
> >  pub struct LockClassKey {
> > @@ -34,6 +36,10 @@ pub struct LockClassKey {
> >      inner: Opaque<bindings::lock_class_key>,
> >  }
> >
> > +// SAFETY: Unregistering a lock class key from a different thread than where it was registered is
> > +// allowed.
> > +unsafe impl Send for LockClassKey {}
> > +
> >  // SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
> >  // provides its own synchronization.
> >  unsafe impl Sync for LockClassKey {}
> > @@ -41,28 +47,30 @@ 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.
> > +    /// This is usually used indirectly through the [`static_lock_class!`] macro. See its
> > +    /// documentation for more information.
> >      ///
> >      /// # Safety
> >      ///
> >      /// 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.
> > +    /// 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.
> >      ///
> >      /// # Examples
> > +    ///
> >      /// ```
> > -    /// # use kernel::c_str;
> > -    /// # use kernel::alloc::KBox;
> > -    /// # use kernel::types::ForeignOwnable;
> > -    /// # use kernel::sync::{LockClassKey, SpinLock};
> > -    /// # use pin_init::stack_pin_init;
> > +    /// use kernel::c_str;
>
> We can probably change the use `optional_name!()` to make
> core::ffi::CStr -> kernel::str::CStr more smooth.
>
> > +    /// use kernel::types::ForeignOwnable;
> > +    /// use kernel::sync::{LockClassKey, SpinLock};
> > +    /// use pin_init::stack_pin_init;
> >      ///
> >      /// let key = KBox::pin_init(LockClassKey::new_dynamic(), GFP_KERNEL)?;
> >      /// let key_ptr = key.into_foreign();
> > @@ -80,7 +88,6 @@ impl LockClassKey {
> >      /// // SAFETY: We dropped `num`, the only use of the key, so the result of the previous
> >      /// // `borrow` has also been dropped. Thus, it's safe to use from_foreign.
> >      /// unsafe { drop(<Pin<KBox<LockClassKey>> as ForeignOwnable>::from_foreign(key_ptr)) };
> > -    ///
> >      /// # Ok::<(), Error>(())
> >      /// ```
> >      pub fn new_dynamic() -> impl PinInit<Self> {
> > @@ -90,7 +97,10 @@ pub fn new_dynamic() -> impl PinInit<Self> {
> >          })
> >      }
> >
> > -    pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > +    /// Returns a raw pointer to the inner C struct.
> > +    ///
> > +    /// It is up to the caller to use the raw pointer correctly.
> > +    pub fn as_ptr(&self) -> *mut bindings::lock_class_key {
> >          self.inner.get()
> >      }
> >  }
> > @@ -98,14 +108,28 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> >  #[pinned_drop]
> >  impl PinnedDrop for LockClassKey {
> >      fn drop(self: Pin<&mut Self>) {
> > -        // SAFETY: self.as_ptr was registered with lockdep and self is pinned, so the address
> > -        // hasn't changed. Thus, it's safe to pass to unregister.
> > +        // SAFETY: `self.as_ptr()` was registered with lockdep and `self` is pinned, so the address
> > +        // hasn't changed. Thus, it's safe to pass it to unregister.
> >          unsafe { bindings::lockdep_unregister_key(self.as_ptr()) }
> >      }
> >  }
> >
> >  /// Defines a new static lock class and returns a pointer to it.
> > -#[doc(hidden)]
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::c_str;
> > +/// use kernel::sync::{static_lock_class, Arc, SpinLock};
> > +///
> > +/// fn new_locked_int() -> Result<Arc<SpinLock<u32>>> {
> > +///     Arc::pin_init(SpinLock::new(
> > +///         42,
> > +///         c_str!("new_locked_int"),
>
> We could use `optional_name!()` here to avoid another usage of
> `c_str!()`.
>
> That said, I'm not sure whether we should replace `c_str!()` in the
> example of `new_dynamic()` right now in this series, I think that
> depends on two things: 1) whether this series goes into tip or rust-next
> for 6.18 and 2) when we are expecting to land the replacement of
> `c_str!()`.
>
> Miguel and Tamir, any thought?
>
> Regards,
> Boqun

I don't think this patch meaningfully changes the complexity of the
cstr migration. The changes are just a few tokens.

>
> > +///         static_lock_class!(),
> > +///     ), GFP_KERNEL)
> > +/// }
> > +/// ```
> >  #[macro_export]
> >  macro_rules! static_lock_class {
> >      () => {{
> > @@ -117,6 +141,7 @@ macro_rules! static_lock_class {
> >          $crate::prelude::Pin::static_ref(&CLASS)
> >      }};
> >  }
> > +pub use static_lock_class;
> >
> >  /// Returns the given string, if one is provided, otherwise generates one based on the source code
> >  /// location.
> >
> > --
> > 2.50.0.727.gbf7dc18ff4-goog
> >

Re: [PATCH 2/2] rust: sync: clean up LockClassKey and its docs

[Boqun Feng]

On Thu, Jul 24, 2025 at 02:14:59PM -0400, Tamir Duberstein wrote:
> On Wed, Jul 23, 2025 at 9:49 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Wed, Jul 23, 2025 at 11:49:34AM +0000, Alice Ryhl wrote:
> > > Several aspects of the code and documentation for this type is
> > > incomplete. Also several things are hidden from the docs. Thus, clean it
> > > up and make it easier to read the rendered html docs.
> > >
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> >
> > This looks good to me. One thing below:
> >
> > >  rust/kernel/sync.rs | 55 ++++++++++++++++++++++++++++++++++++++---------------
> > >  1 file changed, 40 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > > index 9545bedf47b67976ab8c22d8368991cf1f382e42..5019a0bc95446fe30bad02ce040a1cbbe6d9ad5b 100644
> > > --- a/rust/kernel/sync.rs
> > > +++ b/rust/kernel/sync.rs
> > > @@ -26,7 +26,9 @@
> > >  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
> > >  pub use locked_by::LockedBy;
> > >
> > > -/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> > > +/// Represents a lockdep class.
> > > +///
> > > +/// Wraps the kernel's `struct lock_class_key`.
> > >  #[repr(transparent)]
> > >  #[pin_data(PinnedDrop)]
> > >  pub struct LockClassKey {
> > > @@ -34,6 +36,10 @@ pub struct LockClassKey {
> > >      inner: Opaque<bindings::lock_class_key>,
> > >  }
> > >
> > > +// SAFETY: Unregistering a lock class key from a different thread than where it was registered is
> > > +// allowed.
> > > +unsafe impl Send for LockClassKey {}
> > > +
> > >  // SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
> > >  // provides its own synchronization.
> > >  unsafe impl Sync for LockClassKey {}
> > > @@ -41,28 +47,30 @@ 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.
> > > +    /// This is usually used indirectly through the [`static_lock_class!`] macro. See its
> > > +    /// documentation for more information.
> > >      ///
> > >      /// # Safety
> > >      ///
> > >      /// 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.
> > > +    /// 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.
> > >      ///
> > >      /// # Examples
> > > +    ///
> > >      /// ```
> > > -    /// # use kernel::c_str;
> > > -    /// # use kernel::alloc::KBox;
> > > -    /// # use kernel::types::ForeignOwnable;
> > > -    /// # use kernel::sync::{LockClassKey, SpinLock};
> > > -    /// # use pin_init::stack_pin_init;
> > > +    /// use kernel::c_str;
> >
> > We can probably change the use `optional_name!()` to make
> > core::ffi::CStr -> kernel::str::CStr more smooth.
> >
> > > +    /// use kernel::types::ForeignOwnable;
> > > +    /// use kernel::sync::{LockClassKey, SpinLock};
> > > +    /// use pin_init::stack_pin_init;
> > >      ///
> > >      /// let key = KBox::pin_init(LockClassKey::new_dynamic(), GFP_KERNEL)?;
> > >      /// let key_ptr = key.into_foreign();
> > > @@ -80,7 +88,6 @@ impl LockClassKey {
> > >      /// // SAFETY: We dropped `num`, the only use of the key, so the result of the previous
> > >      /// // `borrow` has also been dropped. Thus, it's safe to use from_foreign.
> > >      /// unsafe { drop(<Pin<KBox<LockClassKey>> as ForeignOwnable>::from_foreign(key_ptr)) };
> > > -    ///
> > >      /// # Ok::<(), Error>(())
> > >      /// ```
> > >      pub fn new_dynamic() -> impl PinInit<Self> {
> > > @@ -90,7 +97,10 @@ pub fn new_dynamic() -> impl PinInit<Self> {
> > >          })
> > >      }
> > >
> > > -    pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > > +    /// Returns a raw pointer to the inner C struct.
> > > +    ///
> > > +    /// It is up to the caller to use the raw pointer correctly.
> > > +    pub fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > >          self.inner.get()
> > >      }
> > >  }
> > > @@ -98,14 +108,28 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > >  #[pinned_drop]
> > >  impl PinnedDrop for LockClassKey {
> > >      fn drop(self: Pin<&mut Self>) {
> > > -        // SAFETY: self.as_ptr was registered with lockdep and self is pinned, so the address
> > > -        // hasn't changed. Thus, it's safe to pass to unregister.
> > > +        // SAFETY: `self.as_ptr()` was registered with lockdep and `self` is pinned, so the address
> > > +        // hasn't changed. Thus, it's safe to pass it to unregister.
> > >          unsafe { bindings::lockdep_unregister_key(self.as_ptr()) }
> > >      }
> > >  }
> > >
> > >  /// Defines a new static lock class and returns a pointer to it.
> > > -#[doc(hidden)]
> > > +///
> > > +/// # Examples
> > > +///
> > > +/// ```
> > > +/// use kernel::c_str;
> > > +/// use kernel::sync::{static_lock_class, Arc, SpinLock};
> > > +///
> > > +/// fn new_locked_int() -> Result<Arc<SpinLock<u32>>> {
> > > +///     Arc::pin_init(SpinLock::new(
> > > +///         42,
> > > +///         c_str!("new_locked_int"),
> >
> > We could use `optional_name!()` here to avoid another usage of
> > `c_str!()`.
> >
> > That said, I'm not sure whether we should replace `c_str!()` in the
> > example of `new_dynamic()` right now in this series, I think that
> > depends on two things: 1) whether this series goes into tip or rust-next
> > for 6.18 and 2) when we are expecting to land the replacement of
> > `c_str!()`.
> >
> > Miguel and Tamir, any thought?
> >
> > Regards,
> > Boqun
> 
> I don't think this patch meaningfully changes the complexity of the
> cstr migration. The changes are just a few tokens.
> 

Ok, so you're fine if I or someone else take this patch as it is
(including the new user of `c_str!()`), and get it merged via the tip
tree [1] in v6.18 merge window? That means if we remove `c_str!()` in
v6.18 merge window, there would be a non-trivial merge resolution to do.

Of course, it'll be less problematic if this goes into rust tree instead
of tip.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/

Regards,
Boqun

> >
> > > +///         static_lock_class!(),
> > > +///     ), GFP_KERNEL)
> > > +/// }
> > > +/// ```
> > >  #[macro_export]
> > >  macro_rules! static_lock_class {
> > >      () => {{
> > > @@ -117,6 +141,7 @@ macro_rules! static_lock_class {
> > >          $crate::prelude::Pin::static_ref(&CLASS)
> > >      }};
> > >  }
> > > +pub use static_lock_class;
> > >
> > >  /// Returns the given string, if one is provided, otherwise generates one based on the source code
> > >  /// location.
> > >
> > > --
> > > 2.50.0.727.gbf7dc18ff4-goog
> > >

Re: [PATCH 2/2] rust: sync: clean up LockClassKey and its docs

[Tamir Duberstein]

On Thu, Jul 24, 2025 at 2:46 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Thu, Jul 24, 2025 at 02:14:59PM -0400, Tamir Duberstein wrote:
> > On Wed, Jul 23, 2025 at 9:49 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Wed, Jul 23, 2025 at 11:49:34AM +0000, Alice Ryhl wrote:
> > > > Several aspects of the code and documentation for this type is
> > > > incomplete. Also several things are hidden from the docs. Thus, clean it
> > > > up and make it easier to read the rendered html docs.
> > > >
> > > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > > ---
> > >
> > > This looks good to me. One thing below:
> > >
> > > >  rust/kernel/sync.rs | 55 ++++++++++++++++++++++++++++++++++++++---------------
> > > >  1 file changed, 40 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> > > > index 9545bedf47b67976ab8c22d8368991cf1f382e42..5019a0bc95446fe30bad02ce040a1cbbe6d9ad5b 100644
> > > > --- a/rust/kernel/sync.rs
> > > > +++ b/rust/kernel/sync.rs
> > > > @@ -26,7 +26,9 @@
> > > >  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
> > > >  pub use locked_by::LockedBy;
> > > >
> > > > -/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> > > > +/// Represents a lockdep class.
> > > > +///
> > > > +/// Wraps the kernel's `struct lock_class_key`.
> > > >  #[repr(transparent)]
> > > >  #[pin_data(PinnedDrop)]
> > > >  pub struct LockClassKey {
> > > > @@ -34,6 +36,10 @@ pub struct LockClassKey {
> > > >      inner: Opaque<bindings::lock_class_key>,
> > > >  }
> > > >
> > > > +// SAFETY: Unregistering a lock class key from a different thread than where it was registered is
> > > > +// allowed.
> > > > +unsafe impl Send for LockClassKey {}
> > > > +
> > > >  // SAFETY: `bindings::lock_class_key` is designed to be used concurrently from multiple threads and
> > > >  // provides its own synchronization.
> > > >  unsafe impl Sync for LockClassKey {}
> > > > @@ -41,28 +47,30 @@ 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.
> > > > +    /// This is usually used indirectly through the [`static_lock_class!`] macro. See its
> > > > +    /// documentation for more information.
> > > >      ///
> > > >      /// # Safety
> > > >      ///
> > > >      /// 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.
> > > > +    /// 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.
> > > >      ///
> > > >      /// # Examples
> > > > +    ///
> > > >      /// ```
> > > > -    /// # use kernel::c_str;
> > > > -    /// # use kernel::alloc::KBox;
> > > > -    /// # use kernel::types::ForeignOwnable;
> > > > -    /// # use kernel::sync::{LockClassKey, SpinLock};
> > > > -    /// # use pin_init::stack_pin_init;
> > > > +    /// use kernel::c_str;
> > >
> > > We can probably change the use `optional_name!()` to make
> > > core::ffi::CStr -> kernel::str::CStr more smooth.
> > >
> > > > +    /// use kernel::types::ForeignOwnable;
> > > > +    /// use kernel::sync::{LockClassKey, SpinLock};
> > > > +    /// use pin_init::stack_pin_init;
> > > >      ///
> > > >      /// let key = KBox::pin_init(LockClassKey::new_dynamic(), GFP_KERNEL)?;
> > > >      /// let key_ptr = key.into_foreign();
> > > > @@ -80,7 +88,6 @@ impl LockClassKey {
> > > >      /// // SAFETY: We dropped `num`, the only use of the key, so the result of the previous
> > > >      /// // `borrow` has also been dropped. Thus, it's safe to use from_foreign.
> > > >      /// unsafe { drop(<Pin<KBox<LockClassKey>> as ForeignOwnable>::from_foreign(key_ptr)) };
> > > > -    ///
> > > >      /// # Ok::<(), Error>(())
> > > >      /// ```
> > > >      pub fn new_dynamic() -> impl PinInit<Self> {
> > > > @@ -90,7 +97,10 @@ pub fn new_dynamic() -> impl PinInit<Self> {
> > > >          })
> > > >      }
> > > >
> > > > -    pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > > > +    /// Returns a raw pointer to the inner C struct.
> > > > +    ///
> > > > +    /// It is up to the caller to use the raw pointer correctly.
> > > > +    pub fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > > >          self.inner.get()
> > > >      }
> > > >  }
> > > > @@ -98,14 +108,28 @@ pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key {
> > > >  #[pinned_drop]
> > > >  impl PinnedDrop for LockClassKey {
> > > >      fn drop(self: Pin<&mut Self>) {
> > > > -        // SAFETY: self.as_ptr was registered with lockdep and self is pinned, so the address
> > > > -        // hasn't changed. Thus, it's safe to pass to unregister.
> > > > +        // SAFETY: `self.as_ptr()` was registered with lockdep and `self` is pinned, so the address
> > > > +        // hasn't changed. Thus, it's safe to pass it to unregister.
> > > >          unsafe { bindings::lockdep_unregister_key(self.as_ptr()) }
> > > >      }
> > > >  }
> > > >
> > > >  /// Defines a new static lock class and returns a pointer to it.
> > > > -#[doc(hidden)]
> > > > +///
> > > > +/// # Examples
> > > > +///
> > > > +/// ```
> > > > +/// use kernel::c_str;
> > > > +/// use kernel::sync::{static_lock_class, Arc, SpinLock};
> > > > +///
> > > > +/// fn new_locked_int() -> Result<Arc<SpinLock<u32>>> {
> > > > +///     Arc::pin_init(SpinLock::new(
> > > > +///         42,
> > > > +///         c_str!("new_locked_int"),
> > >
> > > We could use `optional_name!()` here to avoid another usage of
> > > `c_str!()`.
> > >
> > > That said, I'm not sure whether we should replace `c_str!()` in the
> > > example of `new_dynamic()` right now in this series, I think that
> > > depends on two things: 1) whether this series goes into tip or rust-next
> > > for 6.18 and 2) when we are expecting to land the replacement of
> > > `c_str!()`.
> > >
> > > Miguel and Tamir, any thought?
> > >
> > > Regards,
> > > Boqun
> >
> > I don't think this patch meaningfully changes the complexity of the
> > cstr migration. The changes are just a few tokens.
> >
>
> Ok, so you're fine if I or someone else take this patch as it is
> (including the new user of `c_str!()`), and get it merged via the tip
> tree [1] in v6.18 merge window? That means if we remove `c_str!()` in
> v6.18 merge window, there would be a non-trivial merge resolution to do.

The merge conflict would only arise at the very end of the migration,
when we're using C-string literals everywhere *and* the macro has been
renamed. I haven't even sent that final series yet. Regardless, the
resolution would be quite trivial, I think:
- remove the import that doesn't exist
- replace `c_str!("new_locked_int")` with `c"new_locked_int"`

In other words: yep, I'm fine with however this is taken.

>
> Of course, it'll be less problematic if this goes into rust tree instead
> of tip.

This would be ideal, yep.

>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/
>
> Regards,
> Boqun
>
> > >
> > > > +///         static_lock_class!(),
> > > > +///     ), GFP_KERNEL)
> > > > +/// }
> > > > +/// ```
> > > >  #[macro_export]
> > > >  macro_rules! static_lock_class {
> > > >      () => {{
> > > > @@ -117,6 +141,7 @@ macro_rules! static_lock_class {
> > > >          $crate::prelude::Pin::static_ref(&CLASS)
> > > >      }};
> > > >  }
> > > > +pub use static_lock_class;
> > > >
> > > >  /// Returns the given string, if one is provided, otherwise generates one based on the source code
> > > >  /// location.
> > > >
> > > > --
> > > > 2.50.0.727.gbf7dc18ff4-goog
> > > >

Re: [PATCH 0/2] Clean up Rust LockClassKey

[Daniel Almeida]

Hi Alice,

> On 23 Jul 2025, at 08:49, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> This series applies the suggestion from Benno [1] and various other
> improvements I found when looking over the LockClassKey implementation.
> 
> Based on rust-next.
> 
> [1]: https://lore.kernel.org/all/DBIJLR7XNI6U.21PMPODHE83DZ@kernel.org/
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Alice Ryhl (2):
>      rust: sync: refactor static_lock_class!() macro
>      rust: sync: clean up LockClassKey and its docs
> 
> rust/kernel/sync.rs | 77 +++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 57 insertions(+), 20 deletions(-)
> ---
> base-commit: dff64b072708ffef23c117fa1ee1ea59eb417807
> change-id: 20250723-lock-class-key-cleanup-a3baf53b123a
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@google.com>
> 
> 


Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

— Daniel