Skip to content

Commit aac3193

Browse files
nbdd0121jannau
authored andcommitted
rust: block: convert block::mq to use Refcount
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. [boqun: Adopt the LKMM atomic API] Signed-off-by: Gary Guo <gary@garyguo.net> Signed-off-by: Boqun Feng <boqun.feng@gmail.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Benno Lossin <lossin@kernel.org> Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev> Acked-by: Andreas Hindborg <a.hindborg@kernel.org> Tested-by: David Gow <davidgow@google.com> Link: https://lore.kernel.org/r/20250723233312.3304339-5-gary@kernel.org
1 parent 2098b9a commit aac3193

3 files changed

Lines changed: 40 additions & 55 deletions

File tree

rust/kernel/block/mq/operations.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@ use crate::{
1010
block::mq::Request,
1111
error::{from_result, Result},
1212
prelude::*,
13+
sync::Refcount,
1314
types::ARef,
1415
};
15-
use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
16+
use core::marker::PhantomData;
1617

1718
/// Implement this trait to interface blk-mq as block devices.
1819
///
@@ -78,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
7879
let request = unsafe { &*(*bd).rq.cast::<Request<T>>() };
7980

8081
// One refcount for the ARef, one for being in flight
81-
request.wrapper_ref().refcount().store(2, Ordering::Relaxed);
82+
request.wrapper_ref().refcount().set(2);
8283

8384
// SAFETY:
8485
// - We own a refcount that we took above. We pass that to `ARef`.
@@ -187,7 +188,7 @@ impl<T: Operations> OperationsVTable<T> {
187188

188189
// SAFETY: The refcount field is allocated but not initialized, so
189190
// it is valid for writes.
190-
unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(AtomicU64::new(0)) };
191+
unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Refcount::new(0)) };
191192

192193
Ok(0)
193194
})

rust/kernel/block/mq/request.rs

Lines changed: 21 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,10 @@ use crate::{
88
bindings,
99
block::mq::Operations,
1010
error::Result,
11+
sync::{atomic::Relaxed, Refcount},
1112
types::{ARef, AlwaysRefCounted, Opaque},
1213
};
13-
use core::{
14-
marker::PhantomData,
15-
ptr::NonNull,
16-
sync::atomic::{AtomicU64, Ordering},
17-
};
14+
use core::{marker::PhantomData, ptr::NonNull};
1815

1916
/// A wrapper around a blk-mq [`struct request`]. This represents an IO request.
2017
///
@@ -37,6 +34,9 @@ use core::{
3734
/// We need to track 3 and 4 to ensure that it is safe to end the request and hand
3835
/// back ownership to the block layer.
3936
///
37+
/// Note that the driver can still obtain new `ARef` even if there is no `ARef`s in existence by
38+
/// using `tag_to_rq`, hence the need to distinguish B and C.
39+
///
4040
/// The states are tracked through the private `refcount` field of
4141
/// `RequestDataWrapper`. This structure lives in the private data area of the C
4242
/// [`struct request`].
@@ -98,13 +98,16 @@ impl<T: Operations> Request<T> {
9898
///
9999
/// [`struct request`]: srctree/include/linux/blk-mq.h
100100
fn try_set_end(this: ARef<Self>) -> Result<*mut bindings::request, ARef<Self>> {
101-
// We can race with `TagSet::tag_to_rq`
102-
if let Err(_old) = this.wrapper_ref().refcount().compare_exchange(
103-
2,
104-
0,
105-
Ordering::Relaxed,
106-
Ordering::Relaxed,
107-
) {
101+
// To hand back the ownership, we need the current refcount to be 2.
102+
// Since we can race with `TagSet::tag_to_rq`, this needs to atomically reduce
103+
// refcount to 0. `Refcount` does not provide a way to do this, so use the underlying
104+
// atomics directly.
105+
if let Err(_old) = this
106+
.wrapper_ref()
107+
.refcount()
108+
.as_atomic()
109+
.cmpxchg(2, 0, Relaxed)
110+
{
108111
return Err(this);
109112
}
110113

@@ -173,13 +176,13 @@ pub(crate) struct RequestDataWrapper {
173176
/// - 0: The request is owned by C block layer.
174177
/// - 1: The request is owned by Rust abstractions but there are no [`ARef`] references to it.
175178
/// - 2+: There are [`ARef`] references to the request.
176-
refcount: AtomicU64,
179+
refcount: Refcount,
177180
}
178181

179182
impl RequestDataWrapper {
180183
/// Return a reference to the refcount of the request that is embedding
181184
/// `self`.
182-
pub(crate) fn refcount(&self) -> &AtomicU64 {
185+
pub(crate) fn refcount(&self) -> &Refcount {
183186
&self.refcount
184187
}
185188

@@ -189,7 +192,7 @@ impl RequestDataWrapper {
189192
/// # Safety
190193
///
191194
/// - `this` must point to a live allocation of at least the size of `Self`.
192-
pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut AtomicU64 {
195+
pub(crate) unsafe fn refcount_ptr(this: *mut Self) -> *mut Refcount {
193196
// SAFETY: Because of the safety requirements of this function, the
194197
// field projection is safe.
195198
unsafe { &raw mut (*this).refcount }
@@ -205,47 +208,13 @@ unsafe impl<T: Operations> Send for Request<T> {}
205208
// mutate `self` are internally synchronized`
206209
unsafe impl<T: Operations> Sync for Request<T> {}
207210

208-
/// Store the result of `op(target.load())` in target, returning new value of
209-
/// target.
210-
fn atomic_relaxed_op_return(target: &AtomicU64, op: impl Fn(u64) -> u64) -> u64 {
211-
let old = target.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| Some(op(x)));
212-
213-
// SAFETY: Because the operation passed to `fetch_update` above always
214-
// return `Some`, `old` will always be `Ok`.
215-
let old = unsafe { old.unwrap_unchecked() };
216-
217-
op(old)
218-
}
219-
220-
/// Store the result of `op(target.load)` in `target` if `target.load() !=
221-
/// pred`, returning [`true`] if the target was updated.
222-
fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) -> u64, pred: u64) -> bool {
223-
target
224-
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| {
225-
if x == pred {
226-
None
227-
} else {
228-
Some(op(x))
229-
}
230-
})
231-
.is_ok()
232-
}
233-
234211
// SAFETY: All instances of `Request<T>` are reference counted. This
235212
// implementation of `AlwaysRefCounted` ensure that increments to the ref count
236213
// keeps the object alive in memory at least until a matching reference count
237214
// decrement is executed.
238215
unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
239216
fn inc_ref(&self) {
240-
let refcount = &self.wrapper_ref().refcount();
241-
242-
#[cfg_attr(not(CONFIG_DEBUG_MISC), allow(unused_variables))]
243-
let updated = atomic_relaxed_op_unless(refcount, |x| x + 1, 0);
244-
245-
#[cfg(CONFIG_DEBUG_MISC)]
246-
if !updated {
247-
panic!("Request refcount zero on clone")
248-
}
217+
self.wrapper_ref().refcount().inc();
249218
}
250219

251220
unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
@@ -257,10 +226,10 @@ unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
257226
let refcount = unsafe { &*RequestDataWrapper::refcount_ptr(wrapper_ptr) };
258227

259228
#[cfg_attr(not(CONFIG_DEBUG_MISC), allow(unused_variables))]
260-
let new_refcount = atomic_relaxed_op_return(refcount, |x| x - 1);
229+
let is_zero = refcount.dec_and_test();
261230

262231
#[cfg(CONFIG_DEBUG_MISC)]
263-
if new_refcount == 0 {
232+
if is_zero {
264233
panic!("Request reached refcount zero in Rust abstractions");
265234
}
266235
}

rust/kernel/sync/refcount.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//! C header: [`include/linux/refcount.h`](srctree/include/linux/refcount.h)
66
77
use crate::build_assert;
8+
use crate::sync::atomic::Atomic;
89
use crate::types::Opaque;
910

1011
/// Atomic reference counter.
@@ -34,6 +35,20 @@ impl Refcount {
3435
self.0.get()
3536
}
3637

38+
/// Get the underlying atomic counter that backs the refcount.
39+
///
40+
/// NOTE: Usage of this function is discouraged as it can circumvent the protections offered by
41+
/// `refcount.h`. If there is no way to achieve the result using APIs in `refcount.h`, then
42+
/// this function can be used. Otherwise consider adding a binding for the required API.
43+
#[inline]
44+
pub fn as_atomic(&self) -> &Atomic<i32> {
45+
let ptr = self.0.get().cast();
46+
// SAFETY: `refcount_t` is a transparent wrapper of `atomic_t`, which is an atomic 32-bit
47+
// integer that is layout-wise compatible with `Atomic<i32>`. All values are valid for
48+
// `refcount_t`, despite some of the values being considered saturated and "bad".
49+
unsafe { &*ptr }
50+
}
51+
3752
/// Set a refcount's value.
3853
#[inline]
3954
pub fn set(&self, value: i32) {

0 commit comments

Comments
 (0)