WIP: rust: sync: atomic: Add Atomic<*mut T> Add atomic support for raw pointer values, similar to `isize` and `usize`, the representation type is selected based on CONFIG_64BIT. `*mut T` is not `Send`, however `Atomic<*mut T>` definitely needs to be a `Sync`, and that's the whole point of atomics: being able to have multiple shared references in different threads so that they can sync with each other. As a result, a pointer value will be transferred from one thread to another via `Atomic<*mut T>`: <thread 1> <thread 2> x.store(p1, Relaxed); let p = x.load(p1, Relaxed); This means a raw pointer value (`*mut T`) needs to be able to transfer across thread boundaries, which is essentially `Send`. To reflect this in the type system, and based on the fact that pointer values can be transferred safely (only using them to dereference is unsafe), as suggested by Alice, extend the `AllowAtomic` trait to include a customized `Send` semantics, that is: `impl AllowAtomic` has to be safe to be transferred across thread boundaries. Suggested-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
diff --git a/rust/kernel/sync/atomic.rs b/rust/kernel/sync/atomic.rs index e1e4075..7ff87b2 100644 --- a/rust/kernel/sync/atomic.rs +++ b/rust/kernel/sync/atomic.rs
@@ -123,6 +123,12 @@ fn delta_into_repr(d: Self::Delta) -> Self::Repr { } } +// SAFETY: `*mut T` and `*mut ()` has the same size and alignment, and `*mut T` is round-trip +// transmutable to `*mut ()`. +unsafe impl<T> generic::AllowAtomic for *mut T { + type Repr = *mut crate::ffi::c_void; +} + use crate::macros::kunit_tests; #[kunit_tests(rust_atomics)] @@ -147,6 +153,9 @@ fn atomic_basic_tests() { assert_eq!(v, x.load(Relaxed)); }); + + let x = Atomic::new(core::ptr::null_mut::<i32>()); + assert!(x.load(Relaxed).is_null()); } #[test] @@ -190,4 +199,33 @@ fn atomic_arithmetic_tests() { assert_eq!(v + 25, x.load(Relaxed)); }); } + + #[test] + fn atomic_ptr_tests() -> crate::error::Result { + use crate::alloc::{flags::GFP_KERNEL, KBox}; + use core::ptr; + + let x = Atomic::new(ptr::null_mut::<i32>()); + + assert!(x.load(Relaxed).is_null()); + + let new = KBox::new(42, GFP_KERNEL)?; + x.store(ptr::from_mut(KBox::leak(new)), Release); + + let ptr = x.load(Relaxed); + assert!(!ptr.is_null()); + + // SAFETY: `ptr` is a valid pointer from `KBox::leak()` and the address dependency + // guarantees observation of the initialization of `KBox`. + assert_eq!(42, unsafe { ptr.read_volatile() }); + + x.xchg(ptr::null_mut(), Relaxed); + assert!(x.load(Relaxed).is_null()); + + // SAFETY: `ptr` is a valid pointer from `KBox::leak()` and no one is currently referencing + // the pointer, so it's safety to convert the ownership back to a `KBox`. + drop(unsafe { KBox::from_raw(ptr) }); + + Ok(()) + } }
diff --git a/rust/kernel/sync/atomic/generic.rs b/rust/kernel/sync/atomic/generic.rs index 412a2c8..237fcec 100644 --- a/rust/kernel/sync/atomic/generic.rs +++ b/rust/kernel/sync/atomic/generic.rs
@@ -45,6 +45,10 @@ #[repr(transparent)] pub struct Atomic<T: AllowAtomic>(UnsafeCell<T>); +// SAFETY: `Atomic<T>` is safe to send between execution contexts, because `T` is `AllowAtomic` and +// `AllowAtomic`'s safety requirement guarantees that. +unsafe impl<T: AllowAtomic> Send for Atomic<T> {} + // SAFETY: `Atomic<T>` is safe to share among execution contexts because all accesses are atomic. unsafe impl<T: AllowAtomic> Sync for Atomic<T> {} @@ -77,6 +81,11 @@ unsafe impl<T: AllowAtomic> Sync for Atomic<T> {} /// /// - [`Self`] must have the same size and alignment as [`Self::Repr`]. /// - [`Self`] and [`Self::Repr`] must have the [round-trip transmutability]. +/// - The implementer must guarantee it's safe to transfer ownership from one execution context to +/// another, this means it has to be a [`Send`], but because `*mut T` is not [`Send`] and that's +/// the basic type needs to support atomic operations, so this safety requirement is added to +/// [`AllowAtomic`] trait. This safety requirement is automatically satisfied if the type is a +/// [`Send`]. /// /// # Limitations /// @@ -91,7 +100,7 @@ unsafe impl<T: AllowAtomic> Sync for Atomic<T> {} /// /// [`transmute()`]: core::mem::transmute /// [round-trip transmutability]: AllowAtomic#round-trip-transmutability -pub unsafe trait AllowAtomic: Sized + Send + Copy { +pub unsafe trait AllowAtomic: Sized + Copy { /// The backing atomic implementation type. type Repr: AtomicImpl; }