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

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
>

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

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

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