implement `kernel::sync::Refcount` and convert users

From: Gary Guo <[email protected]>

This is a wrapping layer of include/linux/refcount.h. Currently the kernel refcount has already been used in Arc, however it calls into FFI directly.

Reviewed-by Alice Ryhl <[email protected]>

Reviewed-by Boqun Feng <[email protected]>

Reviewed-by Fiona Behrens <[email protected]>

Signed-off-by Gary Guo <[email protected]>

rust/kernel/sync/refcount.rs new file +10 -0

+        self.0.get()
+    }
+
 
+    /// Decrement a refcount and test if it is 0.
+    ///
+    /// It will `WARN` on underflow and fail to decrement when saturated.
+    ///
+    /// Provides release memory ordering, such that prior loads and stores are done
+    /// before, and provides an acquire ordering on success such that memory deallocation
+    /// must come after.

Quoted 10 lines · Gary Guo

From: Gary Guo <[email protected]>

This is a wrapping layer of include/linux/refcount.h. Currently the kernel refcount has already been used in Arc, however it calls into FFI directly.

Reviewed-by Alice Ryhl <[email protected]>

Reviewed-by Boqun Feng <[email protected]>

Reviewed-by Fiona Behrens <[email protected]>

Signed-off-by Gary Guo <[email protected]>

Reviewed-by Benno Lossin <[email protected]>

Quoted 6 lines

---
 rust/helpers/refcount.c      | 10 ++++
 rust/kernel/sync.rs          |  2 +
 rust/kernel/sync/refcount.rs | 98 ++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 rust/kernel/sync/refcount.rs  

Review

Quoted 2 lines

(quoted diff) +2 -0

+    #[inline]
+    fn as_ptr(&self) -> *mut bindings::refcount_t {  

I think we should make this pub(crate) from the get-go.

Quoted 12 lines

rust/kernel/sync/refcount.rs new file +10 -0

+        self.0.get()
+    }
+  
 
+    /// Decrement a refcount and test if it is 0.
+    ///
+    /// It will `WARN` on underflow and fail to decrement when saturated.
+    ///
+    /// Provides release memory ordering, such that prior loads and stores are done
+    /// before, and provides an acquire ordering on success such that memory deallocation
+    /// must come after.

rust/kernel/sync/refcount.rs line 85

Quoted 10 lines

rust/kernel/sync/refcount.rs new file +10 -0

+    ///
+    /// Returns true if the resulting refcount is 0, false otherwise.
+    ///
+    /// # Notes
+    ///
+    /// A common pattern of using `Refcount` is to free memory when the reference count reaches
+    /// zero. This means that the reference to `Refcount` could become invalid after calling this
+    /// function. This is fine as long as the reference to `Refcount` is no longer used when this
+    /// function returns `false`. It is not necessary to use raw pointers in this scenario, see
+    /// https://github.com/rust-lang/rust/issues/55005.  

Missing <> around link.

---
Cheers,
Benno

From: Gary Guo <[email protected]>

Make Arc::into_unique_or_drop to become a mere associated function instead of a method (i.e. removing the self receiver).

It's a general convention for Rust smart pointers to avoid having methods defined on them, because if the pointee type has a method of the same name, then it is shadowed. This is normally for avoiding semver breakage, which isn't an issue for kernel codebase, but it's still generally a good practice to follow this rule, so that ptr.foo() would always be calling a method on the pointee type.

Reviewed-by Benno Lossin <[email protected]>

Signed-off-by Gary Guo <[email protected]>

From: Gary Guo <[email protected]>

With Refcount type created, Arc can use Refcount instead of calling into FFI directly.

Signed-off-by Gary Guo <[email protected]>

From: Gary Guo <[email protected]>

With Refcount type created, Arc can use Refcount instead of calling into FFI directly.

Signed-off-by Gary Guo <[email protected]>

---
 rust/kernel/sync/arc.rs | 45 +++++++++++++----------------------------
 1 file changed, 14 insertions(+), 31 deletions(-)

From: Gary Guo <[email protected]>

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 <[email protected]>

Acked-by Andreas Hindborg <[email protected]>

Signed-off-by Gary Guo <[email protected]>

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

rust/kernel/block/mq/operations.rs +3 -2

index c2b98f507bcbd..c0f95a9419c4e 100644
@@ -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`.

Quoted 48 lines · Gary Guo

From: Gary Guo <[email protected]>

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 <[email protected]>

Acked-by Andreas Hindborg <[email protected]>

Signed-off-by Gary Guo <[email protected]>

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

rust/kernel/block/mq/operations.rs +3 -2

index c2b98f507bcbd..c0f95a9419c4e 100644
@@ -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`.

rust/kernel/block/mq/operations.rs impl<T: Operations> OperationsVTable<T> { · line 191

Quoted 5 lines

rust/kernel/block/mq/operations.rs +1 -1

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

From: Gary Guo <[email protected]>

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 <[email protected]>

Acked-by Andreas Hindborg <[email protected]>

Signed-off-by Gary Guo <[email protected]>

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

rust/kernel/sync/refcount.rs +2 -0

index 3ff4585326b41..a9b24c6b2f8a7 100644
@@ -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;
 

rust/kernel/sync/refcount.rs +9 -0

+    #[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) {

Quoted 8 lines · Gary Guo

From: Gary Guo <[email protected]>

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.

Quoted 8 lines

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 <[email protected]>

Acked-by Andreas Hindborg <[email protected]>

Signed-off-by Gary Guo <[email protected]>

Reviewed-by Benno Lossin <[email protected]>

Quoted 23 lines

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

rust/kernel/sync/refcount.rs +2 -0

index 3ff4585326b41..a9b24c6b2f8a7 100644
@@ -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()
     }
 

rust/kernel/sync/refcount.rs fn as_ptr(&self) -> *mut bindings::refcount_t { · line 41

Quoted 3 lines

rust/kernel/sync/refcount.rs +3 -0

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

Quoted 12 lines

rust/kernel/sync/refcount.rs +9 -0

+    #[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) {  

Quoted 5 lines · On Tue, 12 Aug 2025 10:17:44 +0200 "Benno Lossin" <[email protected]> wrote:

Quoted 4 lines · Gary Guo

rust/kernel/sync/refcount.rs +0 -0

@@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
         self.0.get()
     }
 

Quoted 7 lines

rust/kernel/sync/refcount.rs fn as_ptr(&self) -> *mut bindings::refcount_t { · line 41

Quoted 3 lines

rust/kernel/sync/refcount.rs +3 -0

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

Quoted 15 lines · Gary Guo

Quoted 5 lines · On Tue, 12 Aug 2025 10:17:44 +0200 "Benno Lossin" <[email protected]> wrote:

Quoted 4 lines · Gary Guo

rust/kernel/sync/refcount.rs +0 -0

@@ -34,6 +36,18 @@ fn as_ptr(&self) -> *mut bindings::refcount_t {
         self.0.get()
     }
 

Quoted 7 lines

rust/kernel/sync/refcount.rs fn as_ptr(&self) -> *mut bindings::refcount_t { · line 41

Quoted 3 lines

rust/kernel/sync/refcount.rs +3 -0

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

Quoted 6 lines

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