[PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Gary Guo]

From: Gary Guo <gary@garyguo.net>

Currently there's a custom reference counting in `block::mq`, which uses
`AtomicU64` Rust atomics, and this type doesn't exist on some 32-bit
architectures. We cannot just change it to use 32-bit atomics, because
doing so will make it vulnerable to refcount overflow. So switch it to
use the kernel refcount `kernel::sync::Refcount` instead.

There is an operation needed by `block::mq`, atomically decreasing
refcount from 2 to 0, which is not available through refcount.h, so
I exposed `Refcount::as_atomic` which allows accessing the refcount
directly.

Tested-by: David Gow <davidgow@google.com>
Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/block/mq/operations.rs |  7 ++--
 rust/kernel/block/mq/request.rs    | 63 ++++++++----------------------
 rust/kernel/sync/refcount.rs       | 14 +++++++
 3 files changed, 34 insertions(+), 50 deletions(-)

diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index c2b98f507bcbd..c0f95a9419c4e 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -10,9 +10,10 @@
     block::mq::Request,
     error::{from_result, Result},
     prelude::*,
+    sync::Refcount,
     types::ARef,
 };
-use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
+use core::marker::PhantomData;
 
 /// Implement this trait to interface blk-mq as block devices.
 ///
@@ -78,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
         let request = unsafe { &*(*bd).rq.cast::<Request<T>>() };
 
         // One refcount for the ARef, one for being in flight
-        request.wrapper_ref().refcount().store(2, Ordering::Relaxed);
+        request.wrapper_ref().refcount().set(2);
 
         // SAFETY:
         //  - We own a refcount that we took above. We pass that to `ARef`.
@@ -187,7 +188,7 @@ impl<T: Operations> OperationsVTable<T> {
 
             // SAFETY: The refcount field is allocated but not initialized, so
             // it is valid for writes.
-            unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0)) };
+            unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Refcount::new(0)) };
 
             Ok(0)
         })
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index fefd394f064a7..71c62143e94d6 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -8,13 +8,10 @@
     bindings,
     block::mq::Operations,
     error::Result,
+    sync::Refcount,
     types::{ARef, AlwaysRefCounted, Opaque},
 };
-use core::{
-    marker::PhantomData,
-    ptr::NonNull,
-    sync::atomic::{AtomicU64, Ordering},
-};
+use core::{marker::PhantomData, ptr::NonNull, sync::atomic::Ordering};
 
 /// A wrapper around a blk-mq [`struct request`]. This represents an IO request.
 ///
@@ -37,6 +34,9 @@
 /// We need to track 3 and 4 to ensure that it is safe to end the request and hand
 /// back ownership to the block layer.
 ///
+/// Note that the driver can still obtain new `ARef` even if there is no `ARef`s in existence by
+/// using `tag_to_rq`, hence the need to distinguish B and C.
+///
 /// The states are tracked through the private `refcount` field of
 /// `RequestDataWrapper`. This structure lives in the private data area of the C
 /// [`struct request`].
@@ -98,8 +98,11 @@ pub(crate) unsafe fn start_unchecked(this: &ARef<Self>) {
     ///
     /// [`struct request`]: srctree/include/linux/blk-mq.h
     fn try_set_end(this: ARef<Self>) -> Result<*mut bindings::request, ARef<Self>> {
-        // We can race with `TagSet::tag_to_rq`
-        if let Err(_old) = this.wrapper_ref().refcount().compare_exchange(
+        // To hand back the ownership, we need the current refcount to be 2.
+        // Since we can race with `TagSet::tag_to_rq`, this needs to atomically reduce
+        // refcount to 0. `Refcount` does not provide a way to do this, so use the underlying
+        // atomics directly.
+        if let Err(_old) = this.wrapper_ref().refcount().as_atomic().compare_exchange(
             2,
             0,
             Ordering::Relaxed,
@@ -173,13 +176,13 @@ pub(crate) struct RequestDataWrapper {
     /// - 0: The request is owned by C block layer.
     /// - 1: The request is owned by Rust abstractions but there are no [`ARef`] references to it.
     /// - 2+: There are [`ARef`] references to the request.
-    refcount: AtomicU64,
+    refcount: Refcount,
 }
 
 impl RequestDataWrapper {
     /// Return a reference to the refcount of the request that is embedding
     /// `self`.
-    pub(crate) fn refcount(&self) -> &AtomicU64 {
+    pub(crate) fn refcount(&self) -> &Refcount {
         &self.refcount
     }
 
@@ -189,7 +192,7 @@ pub(crate) fn refcount(&self) -> &AtomicU64 {
     /// # Safety
     ///
     /// - `this` must point to a live allocation of at least the size of `Self`.
-    pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut AtomicU64 {
+    pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut Refcount {
         // SAFETY: Because of the safety requirements of this function, the
         // field projection is safe.
         unsafe { &raw mut (*this).refcount }
@@ -205,47 +208,13 @@ unsafe impl<T: Operations> Send for Request<T> {}
 // mutate `self` are internally synchronized`
 unsafe impl<T: Operations> Sync for Request<T> {}
 
-/// Store the result of `op(target.load())` in target, returning new value of
-/// target.
-fn atomic_relaxed_op_return(target: &AtomicU64, op: impl Fn(u64) -> u64) -> u64 {
-    let old = target.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| Some(op(x)));
-
-    // SAFETY: Because the operation passed to `fetch_update` above always
-    // return `Some`, `old` will always be `Ok`.
-    let old = unsafe { old.unwrap_unchecked() };
-
-    op(old)
-}
-
-/// Store the result of `op(target.load)` in `target` if `target.load() !=
-/// pred`, returning [`true`] if the target was updated.
-fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) -> u64, pred: u64) -> bool {
-    target
-        .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| {
-            if x == pred {
-                None
-            } else {
-                Some(op(x))
-            }
-        })
-        .is_ok()
-}
-
 // SAFETY: All instances of `Request<T>` are reference counted. This
 // implementation of `AlwaysRefCounted` ensure that increments to the ref count
 // keeps the object alive in memory at least until a matching reference count
 // decrement is executed.
 unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
     fn inc_ref(&self) {
-        let refcount = &self.wrapper_ref().refcount();
-
-        #[cfg_attr(not(CONFIG_DEBUG_MISC), allow(unused_variables))]
-        let updated = atomic_relaxed_op_unless(refcount, |x| x + 1, 0);
-
-        #[cfg(CONFIG_DEBUG_MISC)]
-        if !updated {
-            panic!("Request refcount zero on clone")
-        }
+        self.wrapper_ref().refcount().inc();
     }
 
     unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
@@ -257,10 +226,10 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
         let refcount = unsafe { &*RequestDataWrapper::refcount_ptr(wrapper_ptr) };
 
         #[cfg_attr(not(CONFIG_DEBUG_MISC), allow(unused_variables))]
-        let new_refcount = atomic_relaxed_op_return(refcount, |x| x - 1);
+        let is_zero = refcount.dec_and_test();
 
         #[cfg(CONFIG_DEBUG_MISC)]
-        if new_refcount == 0 {
+        if is_zero {
             panic!("Request reached refcount zero in Rust abstractions");
         }
     }
diff --git a/rust/kernel/sync/refcount.rs b/rust/kernel/sync/refcount.rs
index 3ff4585326b41..a9b24c6b2f8a7 100644
--- a/rust/kernel/sync/refcount.rs
+++ b/rust/kernel/sync/refcount.rs
@@ -4,6 +4,8 @@
 //!
 //! C header: [`include/linux/refcount.h`](srctree/include/linux/refcount.h)
 
+use core::sync::atomic::AtomicI32;
+
 use crate::build_assert;
 use crate::types::Opaque;
 
@@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
         self.0.get()
     }
 
+    /// Get the underlying atomic counter that backs the refcount.
+    ///
+    /// NOTE: This will be changed to LKMM atomic in the future.
+    #[inline]
+    pub fn as_atomic(&self) -> &AtomicI32 {
+        let ptr = self.0.get().cast();
+        // SAFETY: `refcount_t` is a transparent wrapper of `atomic_t`, which is an atomic 32-bit
+        // integer that is layout-wise compatible with `AtomicI32`. All values are valid for
+        // `refcount_t`, despite some of the values being considered saturated and "bad".
+        unsafe { &*ptr }
+    }
+
     /// Set a refcount's value.
     #[inline]
     pub fn set(&self, value: i32) {
-- 
2.49.0

Re: [PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Alexandre Courbot]

On Thu Jul 24, 2025 at 8:32 AM JST, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Currently there's a custom reference counting in `block::mq`, which uses
> `AtomicU64` Rust atomics, and this type doesn't exist on some 32-bit
> architectures. We cannot just change it to use 32-bit atomics, because
> doing so will make it vulnerable to refcount overflow. So switch it to
> use the kernel refcount `kernel::sync::Refcount` instead.
>
> There is an operation needed by `block::mq`, atomically decreasing
> refcount from 2 to 0, which is not available through refcount.h, so
> I exposed `Refcount::as_atomic` which allows accessing the refcount
> directly.
>
> Tested-by: David Gow <davidgow@google.com>
> Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/block/mq/operations.rs |  7 ++--
>  rust/kernel/block/mq/request.rs    | 63 ++++++++----------------------
>  rust/kernel/sync/refcount.rs       | 14 +++++++
>  3 files changed, 34 insertions(+), 50 deletions(-)
>
> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
> index c2b98f507bcbd..c0f95a9419c4e 100644
> --- a/rust/kernel/block/mq/operations.rs
> +++ b/rust/kernel/block/mq/operations.rs
> @@ -10,9 +10,10 @@
>      block::mq::Request,
>      error::{from_result, Result},
>      prelude::*,
> +    sync::Refcount,
>      types::ARef,
>  };
> -use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
> +use core::marker::PhantomData;
>  
>  /// Implement this trait to interface blk-mq as block devices.
>  ///
> @@ -78,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
>          let request = unsafe { &*(*bd).rq.cast::<Request<T>>() };
>  
>          // One refcount for the ARef, one for being in flight
> -        request.wrapper_ref().refcount().store(2, Ordering::Relaxed);
> +        request.wrapper_ref().refcount().set(2);
>  
>          // SAFETY:
>          //  - We own a refcount that we took above. We pass that to `ARef`.
> @@ -187,7 +188,7 @@ impl<T: Operations> OperationsVTable<T> {
>  
>              // SAFETY: The refcount field is allocated but not initialized, so
>              // it is valid for writes.
> -            unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0)) };
> +            unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Refcount::new(0)) };

Ah, so that's why `0` is allowed as a valid value for `Refcount::new`.
Seeing the use that is made of atomics here, I wonder if `Refcount` is a
good choice, or if we could adapt the code to use them as expected. I am
not familiar enough with this part of the code to give informed advice
unfortunately.

But at the very least, I think the constructor should not be made unsafe
due to account for one particular user. How about doing a
`Refcount::new(1)` immediately followed by a `set(0)` so other users are
not tricked into creating an invalid Refcount?

Re: [PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Boqun Feng]

On Sat, Aug 09, 2025 at 05:21:49PM +0900, Alexandre Courbot wrote:
> On Thu Jul 24, 2025 at 8:32 AM JST, Gary Guo wrote:
> > From: Gary Guo <gary@garyguo.net>
> >
> > Currently there's a custom reference counting in `block::mq`, which uses
> > `AtomicU64` Rust atomics, and this type doesn't exist on some 32-bit
> > architectures. We cannot just change it to use 32-bit atomics, because
> > doing so will make it vulnerable to refcount overflow. So switch it to
> > use the kernel refcount `kernel::sync::Refcount` instead.
> >
> > There is an operation needed by `block::mq`, atomically decreasing
> > refcount from 2 to 0, which is not available through refcount.h, so
> > I exposed `Refcount::as_atomic` which allows accessing the refcount
> > directly.
> >
> > Tested-by: David Gow <davidgow@google.com>
> > Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
> > Signed-off-by: Gary Guo <gary@garyguo.net>
> > ---
> >  rust/kernel/block/mq/operations.rs |  7 ++--
> >  rust/kernel/block/mq/request.rs    | 63 ++++++++----------------------
> >  rust/kernel/sync/refcount.rs       | 14 +++++++
> >  3 files changed, 34 insertions(+), 50 deletions(-)
> >
> > diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
> > index c2b98f507bcbd..c0f95a9419c4e 100644
> > --- a/rust/kernel/block/mq/operations.rs
> > +++ b/rust/kernel/block/mq/operations.rs
> > @@ -10,9 +10,10 @@
> >      block::mq::Request,
> >      error::{from_result, Result},
> >      prelude::*,
> > +    sync::Refcount,
> >      types::ARef,
> >  };
> > -use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
> > +use core::marker::PhantomData;
> >  
> >  /// Implement this trait to interface blk-mq as block devices.
> >  ///
> > @@ -78,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
> >          let request = unsafe { &*(*bd).rq.cast::<Request<T>>() };
> >  
> >          // One refcount for the ARef, one for being in flight
> > -        request.wrapper_ref().refcount().store(2, Ordering::Relaxed);
> > +        request.wrapper_ref().refcount().set(2);
> >  
> >          // SAFETY:
> >          //  - We own a refcount that we took above. We pass that to `ARef`.
> > @@ -187,7 +188,7 @@ impl<T: Operations> OperationsVTable<T> {
> >  
> >              // SAFETY: The refcount field is allocated but not initialized, so
> >              // it is valid for writes.
> > -            unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0)) };
> > +            unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Refcount::new(0)) };
> 
> Ah, so that's why `0` is allowed as a valid value for `Refcount::new`.
> Seeing the use that is made of atomics here, I wonder if `Refcount` is a
> good choice, or if we could adapt the code to use them as expected. I am
> not familiar enough with this part of the code to give informed advice
> unfortunately.
> 
> But at the very least, I think the constructor should not be made unsafe
> due to account for one particular user. How about doing a

Hmm.. a refcount being 0 is not unsafe I would say, it means no one
references the object (usually in a particular type of referencing). In
general, one should use `Arc` and be done with that, but if you were to
have different types of referencing to a same object, then one of the
refcounts may end up being 0.

> `Refcount::new(1)` immediately followed by a `set(0)` so other users are
> not tricked into creating an invalid Refcount?

This I will call bad code, as it would introduce further confusion
because the `new(1)` is pretty pointless imo.

Regards,
Boqun

> 
> 

Re: [PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Benno Lossin]

On Thu Jul 24, 2025 at 1:32 AM CEST, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Currently there's a custom reference counting in `block::mq`, which uses
> `AtomicU64` Rust atomics, and this type doesn't exist on some 32-bit
> architectures. We cannot just change it to use 32-bit atomics, because
> doing so will make it vulnerable to refcount overflow. So switch it to
> use the kernel refcount `kernel::sync::Refcount` instead.
>
> There is an operation needed by `block::mq`, atomically decreasing
> refcount from 2 to 0, which is not available through refcount.h, so
> I exposed `Refcount::as_atomic` which allows accessing the refcount
> directly.
>
> Tested-by: David Gow <davidgow@google.com>
> Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: Gary Guo <gary@garyguo.net>

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

> ---
>  rust/kernel/block/mq/operations.rs |  7 ++--
>  rust/kernel/block/mq/request.rs    | 63 ++++++++----------------------
>  rust/kernel/sync/refcount.rs       | 14 +++++++
>  3 files changed, 34 insertions(+), 50 deletions(-)

> diff --git a/rust/kernel/sync/refcount.rs b/rust/kernel/sync/refcount.rs
> index 3ff4585326b41..a9b24c6b2f8a7 100644
> --- a/rust/kernel/sync/refcount.rs
> +++ b/rust/kernel/sync/refcount.rs
> @@ -4,6 +4,8 @@
>  //!
>  //! C header: [`include/linux/refcount.h`](srctree/include/linux/refcount.h)
>  
> +use core::sync::atomic::AtomicI32;
> +
>  use crate::build_assert;
>  use crate::types::Opaque;
>  
> @@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
>          self.0.get()
>      }
>  
> +    /// Get the underlying atomic counter that backs the refcount.
> +    ///
> +    /// NOTE: This will be changed to LKMM atomic in the future.

Can we discourage using this function a bit more in the docs? At least
point people to try other ways before reaching for this, since it allows
overflowing & doesn't warn on saturate etc.

---
Cheers,
Benno

> +    #[inline]
> +    pub fn as_atomic(&self) -> &AtomicI32 {
> +        let ptr = self.0.get().cast();
> +        // SAFETY: `refcount_t` is a transparent wrapper of `atomic_t`, which is an atomic 32-bit
> +        // integer that is layout-wise compatible with `AtomicI32`. All values are valid for
> +        // `refcount_t`, despite some of the values being considered saturated and "bad".
> +        unsafe { &*ptr }
> +    }
> +
>      /// Set a refcount's value.
>      #[inline]
>      pub fn set(&self, value: i32) {

Re: [PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Gary Guo]

On Tue, 12 Aug 2025 10:17:44 +0200
"Benno Lossin" <lossin@kernel.org> wrote:

> On Thu Jul 24, 2025 at 1:32 AM CEST, Gary Guo wrote:
> > From: Gary Guo <gary@garyguo.net>
> >
> > Currently there's a custom reference counting in `block::mq`, which uses
> > `AtomicU64` Rust atomics, and this type doesn't exist on some 32-bit
> > architectures. We cannot just change it to use 32-bit atomics, because
> > doing so will make it vulnerable to refcount overflow. So switch it to
> > use the kernel refcount `kernel::sync::Refcount` instead.
> >
> > There is an operation needed by `block::mq`, atomically decreasing
> > refcount from 2 to 0, which is not available through refcount.h, so
> > I exposed `Refcount::as_atomic` which allows accessing the refcount
> > directly.
> >
> > Tested-by: David Gow <davidgow@google.com>
> > Acked-by: Andreas Hindborg <a.hindborg@kernel.org>
> > Signed-off-by: Gary Guo <gary@garyguo.net>  
> 
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> 
> > ---
> >  rust/kernel/block/mq/operations.rs |  7 ++--
> >  rust/kernel/block/mq/request.rs    | 63 ++++++++----------------------
> >  rust/kernel/sync/refcount.rs       | 14 +++++++
> >  3 files changed, 34 insertions(+), 50 deletions(-)  
> 
> > diff --git a/rust/kernel/sync/refcount.rs b/rust/kernel/sync/refcount.rs
> > index 3ff4585326b41..a9b24c6b2f8a7 100644
> > --- a/rust/kernel/sync/refcount.rs
> > +++ b/rust/kernel/sync/refcount.rs
> > @@ -4,6 +4,8 @@
> >  //!
> >  //! C header: [`include/linux/refcount.h`](srctree/include/linux/refcount.h)
> >  
> > +use core::sync::atomic::AtomicI32;
> > +
> >  use crate::build_assert;
> >  use crate::types::Opaque;
> >  
> > @@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
> >          self.0.get()
> >      }
> >  
> > +    /// Get the underlying atomic counter that backs the refcount.
> > +    ///
> > +    /// NOTE: This will be changed to LKMM atomic in the future.  
> 
> Can we discourage using this function a bit more in the docs? At least
> point people to try other ways before reaching for this, since it allows
> overflowing & doesn't warn on saturate etc.

Would this additional doc comment be good enough for you?

/// NOTE: usage of this function is discouraged unless there is no way
/// to achieve the desired result using APIs in `refcount.h`. If an API
/// in `refcount.h` does not currently contain a binding, please
/// consider adding a binding for it instead.

Best,
Gary


> 
> ---
> Cheers,
> Benno
> 
> > +    #[inline]
> > +    pub fn as_atomic(&self) -> &AtomicI32 {
> > +        let ptr = self.0.get().cast();
> > +        // SAFETY: `refcount_t` is a transparent wrapper of `atomic_t`, which is an atomic 32-bit
> > +        // integer that is layout-wise compatible with `AtomicI32`. All values are valid for
> > +        // `refcount_t`, despite some of the values being considered saturated and "bad".
> > +        unsafe { &*ptr }
> > +    }
> > +
> >      /// Set a refcount's value.
> >      #[inline]
> >      pub fn set(&self, value: i32) {  
> 

Re: [PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Benno Lossin]

On Wed Aug 27, 2025 at 9:51 PM CEST, Gary Guo wrote:
> On Tue, 12 Aug 2025 10:17:44 +0200
> "Benno Lossin" <lossin@kernel.org> wrote:
>> On Thu Jul 24, 2025 at 1:32 AM CEST, Gary Guo wrote:
>> > @@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
>> >          self.0.get()
>> >      }
>> >  
>> > +    /// Get the underlying atomic counter that backs the refcount.
>> > +    ///
>> > +    /// NOTE: This will be changed to LKMM atomic in the future.  
>> 
>> Can we discourage using this function a bit more in the docs? At least
>> point people to try other ways before reaching for this, since it allows
>> overflowing & doesn't warn on saturate etc.
>
> Would this additional doc comment be good enough for you?
>
> /// NOTE: usage of this function is discouraged unless there is no way
> /// to achieve the desired result using APIs in `refcount.h`. If an API
> /// in `refcount.h` does not currently contain a binding, please
> /// consider adding a binding for it instead.

I'd like to stress that the atomic doesn't have the same protections as
the refcount type, how about:

    /// NOTE: usage of this function is discouraged as it can circumvent the protections offered by
    /// `refcount.h`. If there is no way to achieve the result using APIs in `refcount.h`, then this
    /// function can be used. Otherwise consider adding a binding for the required API.

---
Cheers,
Benno

Re: [PATCH v5 4/5] rust: block: convert `block::mq` to use `Refcount`

[Boqun Feng]

On Thu, Aug 28, 2025 at 09:18:40AM +0200, Benno Lossin wrote:
> On Wed Aug 27, 2025 at 9:51 PM CEST, Gary Guo wrote:
> > On Tue, 12 Aug 2025 10:17:44 +0200
> > "Benno Lossin" <lossin@kernel.org> wrote:
> >> On Thu Jul 24, 2025 at 1:32 AM CEST, Gary Guo wrote:
> >> > @@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
> >> >          self.0.get()
> >> >      }
> >> >  
> >> > +    /// Get the underlying atomic counter that backs the refcount.
> >> > +    ///
> >> > +    /// NOTE: This will be changed to LKMM atomic in the future.  
> >> 
> >> Can we discourage using this function a bit more in the docs? At least
> >> point people to try other ways before reaching for this, since it allows
> >> overflowing & doesn't warn on saturate etc.
> >
> > Would this additional doc comment be good enough for you?
> >
> > /// NOTE: usage of this function is discouraged unless there is no way
> > /// to achieve the desired result using APIs in `refcount.h`. If an API
> > /// in `refcount.h` does not currently contain a binding, please
> > /// consider adding a binding for it instead.
> 
> I'd like to stress that the atomic doesn't have the same protections as
> the refcount type, how about:
> 
>     /// NOTE: usage of this function is discouraged as it can circumvent the protections offered by
>     /// `refcount.h`. If there is no way to achieve the result using APIs in `refcount.h`, then this
>     /// function can be used. Otherwise consider adding a binding for the required API.
> 

Looks good to me. I will apply this and some adjustment against my
atomic patchset, and queue this whole series for v6.18, thanks!

Regards,
Boqun

> ---
> Cheers,
> Benno