From a21e895102dbacf4fc9a8c941fdc253d2bf3be60 Mon Sep 17 00:00:00 2001 From: Chen Chengjun Date: Thu, 10 Apr 2025 10:32:17 +0800 Subject: [PATCH] Enable RCU to read reference to stored pointers --- .../comps/virtio/src/device/console/device.rs | 9 +- kernel/src/time/softirq.rs | 5 +- ostd/src/sync/mod.rs | 2 +- ostd/src/sync/rcu/mod.rs | 125 +++++------- ostd/src/sync/rcu/non_null.rs | 190 ++++++++++++++++++ ostd/src/sync/rcu/owner_ptr.rs | 94 --------- 6 files changed, 244 insertions(+), 181 deletions(-) create mode 100644 ostd/src/sync/rcu/non_null.rs delete mode 100644 ostd/src/sync/rcu/owner_ptr.rs diff --git a/kernel/comps/virtio/src/device/console/device.rs b/kernel/comps/virtio/src/device/console/device.rs index 4c92861ad..cfe57731d 100644 --- a/kernel/comps/virtio/src/device/console/device.rs +++ b/kernel/comps/virtio/src/device/console/device.rs @@ -55,12 +55,9 @@ impl AnyConsoleDevice for ConsoleDevice { fn register_callback(&self, callback: &'static ConsoleCallback) { loop { let callbacks = self.callbacks.read(); - let mut callbacks_cloned = callbacks.clone(); + let mut callbacks_cloned = callbacks.get().clone(); callbacks_cloned.push(callback); - if callbacks - .compare_exchange(Box::new(callbacks_cloned)) - .is_ok() - { + if callbacks.compare_exchange(callbacks_cloned).is_ok() { break; } // Contention on pushing, retry. @@ -150,7 +147,7 @@ impl ConsoleDevice { self.receive_buffer.sync(0..len as usize).unwrap(); let callbacks = self.callbacks.read(); - for callback in callbacks.iter() { + for callback in callbacks.get().iter() { let mut reader = self.receive_buffer.reader().unwrap(); reader.limit(len as usize); callback(reader); diff --git a/kernel/src/time/softirq.rs b/kernel/src/time/softirq.rs index d75952b46..91e63a4ec 100644 --- a/kernel/src/time/softirq.rs +++ b/kernel/src/time/softirq.rs @@ -26,10 +26,7 @@ pub(super) fn register_callback(func: fn()) { Some(callbacks_vec) => { let mut callbacks_cloned = callbacks_vec.clone(); callbacks_cloned.push(func); - if callbacks - .compare_exchange(Some(Box::new(callbacks_cloned))) - .is_ok() - { + if callbacks.compare_exchange(Some(callbacks_cloned)).is_ok() { break; } } diff --git a/ostd/src/sync/mod.rs b/ostd/src/sync/mod.rs index 0ce178e47..c1eae29f4 100644 --- a/ostd/src/sync/mod.rs +++ b/ostd/src/sync/mod.rs @@ -15,7 +15,7 @@ pub(crate) use self::{guard::GuardTransfer, rcu::finish_grace_period}; pub use self::{ guard::{LocalIrqDisabled, PreemptDisabled, WriteIrqDisabled}, mutex::{ArcMutexGuard, Mutex, MutexGuard}, - rcu::{OwnerPtr, Rcu, RcuOption, RcuReadGuard}, + rcu::{non_null, Rcu, RcuOption, RcuOptionReadGuard, RcuReadGuard}, rwarc::{RoArc, RwArc}, rwlock::{ ArcRwLockReadGuard, ArcRwLockUpgradeableGuard, ArcRwLockWriteGuard, RwLock, diff --git a/ostd/src/sync/rcu/mod.rs b/ostd/src/sync/rcu/mod.rs index 09bacacc0..8a86db13d 100644 --- a/ostd/src/sync/rcu/mod.rs +++ b/ostd/src/sync/rcu/mod.rs @@ -4,7 +4,6 @@ use core::{ marker::PhantomData, - ops::Deref, ptr::NonNull, sync::atomic::{ AtomicPtr, @@ -12,20 +11,19 @@ use core::{ }, }; +use non_null::NonNullPtr; use spin::once::Once; use self::monitor::RcuMonitor; use crate::task::{atomic_mode::AsAtomicModeGuard, disable_preempt, DisabledPreemptGuard}; mod monitor; -mod owner_ptr; - -pub use owner_ptr::OwnerPtr; +pub mod non_null; /// A Read-Copy Update (RCU) cell for sharing a pointer between threads. /// -/// The pointer should be a owning pointer with type `P`, which implements -/// [`OwnerPtr`]. For example, `P` can be `Box` or `Arc`. +/// The pointer should be a non-null pointer with type `P`, which implements +/// [`NonNullPtr`]. For example, `P` can be `Box` or `Arc`. /// /// # Overview /// @@ -58,12 +56,12 @@ pub use owner_ptr::OwnerPtr; /// /// assert_eq!(*rcu_guard, Some(&43)); /// ``` -pub struct Rcu(RcuInner

); +pub struct Rcu(RcuInner

); /// A guard that allows access to the pointed data protected by a [`Rcu`]. #[clippy::has_significant_drop] #[must_use] -pub struct RcuReadGuard<'a, P: OwnerPtr>(RcuReadGuardInner<'a, P>); +pub struct RcuReadGuard<'a, P: NonNullPtr>(RcuReadGuardInner<'a, P>); /// A Read-Copy Update (RCU) cell for sharing a _nullable_ pointer. /// @@ -98,37 +96,32 @@ pub struct RcuReadGuard<'a, P: OwnerPtr>(RcuReadGuardInner<'a, P>); /// assert_eq!(*rcu_guard, 43); /// } /// ``` -pub struct RcuOption(RcuInner

); +pub struct RcuOption(RcuInner

); /// A guard that allows access to the pointed data protected by a [`RcuOption`]. #[clippy::has_significant_drop] #[must_use] -pub struct RcuOptionReadGuard<'a, P: OwnerPtr>(RcuReadGuardInner<'a, P>); +pub struct RcuOptionReadGuard<'a, P: NonNullPtr>(RcuReadGuardInner<'a, P>); /// The inner implementation of both [`Rcu`] and [`RcuOption`]. -struct RcuInner { - ptr: AtomicPtr<

::Target>, +struct RcuInner { + ptr: AtomicPtr<

::Target>, // We want to implement Send and Sync explicitly. // Having a pointer field prevents them from being implemented // automatically by the compiler. _marker: PhantomData<*const P::Target>, } -// SAFETY: It is apparent that if `P::Target` is `Send`, then `Rcu

` is `Send`. -unsafe impl Send for RcuInner

where

::Target: Send {} +// SAFETY: It is apparent that if `P` is `Send`, then `Rcu

` is `Send`. +unsafe impl Send for RcuInner

where P: Send {} // SAFETY: To implement `Sync` for `Rcu

`, we need to meet two conditions: -// 1. `P::Target` must be `Sync` because `Rcu::get` allows concurrent access. -// 2. `P::Target` must be `Send` because `Rcu::update` may obtain an object +// 1. `P` must be `Sync` because `Rcu::get` allows concurrent access. +// 2. `P` must be `Send` because `Rcu::update` may obtain an object // of `P` created on another thread. -unsafe impl Sync for RcuInner

-where -

::Target: Send + Sync, - P: Send, -{ -} +unsafe impl Sync for RcuInner

where P: Send + Sync {} -impl RcuInner

{ +impl RcuInner

{ const fn new_none() -> Self { Self { ptr: AtomicPtr::new(core::ptr::null_mut()), @@ -137,7 +130,7 @@ impl RcuInner

{ } fn new(pointer: P) -> Self { - let ptr =

::into_raw(pointer).as_ptr(); + let ptr =

::into_raw(pointer).as_ptr(); let ptr = AtomicPtr::new(ptr); Self { ptr, @@ -147,7 +140,7 @@ impl RcuInner

{ fn update(&self, new_ptr: Option

) { let new_ptr = if let Some(new_ptr) = new_ptr { -

::into_raw(new_ptr).as_ptr() +

::into_raw(new_ptr).as_ptr() } else { core::ptr::null_mut() }; @@ -169,10 +162,7 @@ impl RcuInner

{ } } - fn read_with<'a>( - &'a self, - guard: &'a dyn AsAtomicModeGuard, - ) -> Option<&'a

::Target> { + fn read_with<'a>(&'a self, guard: &'a dyn AsAtomicModeGuard) -> Option> { // Ensure that a real atomic-mode guard is obtained. let _atomic_mode_guard = guard.as_atomic_mode_guard(); @@ -185,17 +175,17 @@ impl RcuInner

{ // 2. The `_atomic_mode_guard` guarantees atomic mode for the duration of // lifetime `'a`, the pointer is valid because other writers won't release // the allocation until this task passes the quiescent state. - Some(unsafe { &*obj_ptr }) + NonNull::new(obj_ptr).map(|ptr| unsafe { P::raw_as_ref(ptr) }) } } -impl Drop for RcuInner

{ +impl Drop for RcuInner

{ fn drop(&mut self) { let ptr = self.ptr.load(Acquire); if let Some(p) = NonNull::new(ptr) { // SAFETY: It was previously returned by `into_raw` when creating // the RCU primitive. - let pointer = unsafe {

::from_raw(p) }; + let pointer = unsafe {

::from_raw(p) }; // It is OK not to delay the drop because the RCU primitive is // owned by nobody else. drop(pointer); @@ -204,16 +194,23 @@ impl Drop for RcuInner

{ } /// The inner implementation of both [`RcuReadGuard`] and [`RcuOptionReadGuard`]. -struct RcuReadGuardInner<'a, P: OwnerPtr> { - obj_ptr: *mut

::Target, +struct RcuReadGuardInner<'a, P: NonNullPtr> { + obj_ptr: *mut

::Target, rcu: &'a RcuInner

, _inner_guard: DisabledPreemptGuard, } -impl RcuReadGuardInner<'_, P> { +impl RcuReadGuardInner<'_, P> { + fn get(&self) -> Option> { + // SAFETY: The guard ensures that `P` will not be dropped. Thus, `P` + // outlives the lifetime of `&self`. Additionally, during this period, + // it is impossible to create a mutable reference to `P`. + NonNull::new(self.obj_ptr).map(|ptr| unsafe { P::raw_as_ref(ptr) }) + } + fn compare_exchange(self, new_ptr: Option

) -> Result<(), Option

> { let new_ptr = if let Some(new_ptr) = new_ptr { -

::into_raw(new_ptr).as_ptr() +

::into_raw(new_ptr).as_ptr() } else { core::ptr::null_mut() }; @@ -231,7 +228,7 @@ impl RcuReadGuardInner<'_, P> { // 1. It was previously returned by `into_raw`. // 2. The `compare_exchange` fails so the pointer will not // be used by other threads via reading the RCU primitive. - return Err(Some(unsafe {

::from_raw(new_ptr) })); + return Err(Some(unsafe {

::from_raw(new_ptr) })); } if let Some(p) = NonNull::new(self.obj_ptr) { @@ -243,7 +240,7 @@ impl RcuReadGuardInner<'_, P> { } } -impl Rcu

{ +impl Rcu

{ /// Creates a new RCU primitive with the given pointer. pub fn new(pointer: P) -> Self { Self(RcuInner::new(pointer)) @@ -279,15 +276,12 @@ impl Rcu

{ /// Unlike [`Self::read`], this function does not return a read guard, so /// you cannot use [`RcuReadGuard::compare_exchange`] to synchronize the /// writers. You may do it via a [`super::SpinLock`]. - pub fn read_with<'a>( - &'a self, - guard: &'a dyn AsAtomicModeGuard, - ) -> &'a

::Target { + pub fn read_with<'a>(&'a self, guard: &'a dyn AsAtomicModeGuard) -> P::Ref<'a> { self.0.read_with(guard).unwrap() } } -impl RcuOption

{ +impl RcuOption

{ /// Creates a new RCU primitive with the given pointer. pub fn new(pointer: Option

) -> Self { if let Some(pointer) = pointer { @@ -337,29 +331,17 @@ impl RcuOption

{ /// Unlike [`Self::read`], this function does not return a read guard, so /// you cannot use [`RcuOptionReadGuard::compare_exchange`] to synchronize the /// writers. You may do it via a [`super::SpinLock`]. - pub fn read_with<'a>( - &'a self, - guard: &'a dyn AsAtomicModeGuard, - ) -> Option<&'a

::Target> { + pub fn read_with<'a>(&'a self, guard: &'a dyn AsAtomicModeGuard) -> Option> { self.0.read_with(guard) } } -// RCU guards that have a non-null pointer can be directly dereferenced. -impl Deref for RcuReadGuard<'_, P> { - type Target =

::Target; - - fn deref(&self) -> &Self::Target { - // SAFETY: - // 1. This pointer is not NULL because the type is `RcuReadGuard`. - // 2. Since the preemption is disabled, the pointer is valid because - // other writers won't release the allocation until this task passes - // the quiescent state. - unsafe { &*self.0.obj_ptr } +impl RcuReadGuard<'_, P> { + /// Gets the reference of the protected data. + pub fn get(&self) -> P::Ref<'_> { + self.0.get().unwrap() } -} -impl RcuReadGuard<'_, P> { /// Tries to replace the already read pointer with a new pointer. /// /// If another thread has updated the pointer after the read, this @@ -380,21 +362,12 @@ impl RcuReadGuard<'_, P> { } } -// RCU guards that may have a null pointer can be dereferenced after checking. -impl RcuOptionReadGuard<'_, P> { +impl RcuOptionReadGuard<'_, P> { /// Gets the reference of the protected data. /// /// If the RCU primitive protects nothing, this function returns `None`. - pub fn get(&self) -> Option<&

::Target> { - if self.0.obj_ptr.is_null() { - return None; - } - // SAFETY: - // 1. This pointer is not NULL. - // 2. Since the preemption is disabled, the pointer is valid because - // other writers won't release the allocation until this task passes - // the quiescent state. - Some(unsafe { &*self.0.obj_ptr }) + pub fn get(&self) -> Option> { + self.0.get() } /// Returns if the RCU primitive protects nothing when [`Rcu::read`] happens. @@ -425,12 +398,12 @@ impl RcuOptionReadGuard<'_, P> { /// /// The pointer must be previously returned by `into_raw` and the pointer /// must be only be dropped once. -unsafe fn delay_drop(pointer: NonNull<

::Target>) { - struct ForceSend(NonNull<

::Target>); +unsafe fn delay_drop(pointer: NonNull<

::Target>) { + struct ForceSend(NonNull<

::Target>); // SAFETY: Sending a raw pointer to another task is safe as long as // the pointer access in another task is safe (guaranteed by the trait // bound `P: Send`). - unsafe impl Send for ForceSend

{} + unsafe impl Send for ForceSend

{} let pointer: ForceSend

= ForceSend(pointer); @@ -444,7 +417,7 @@ unsafe fn delay_drop(pointer: NonNull<

::Target>) { // 1. The pointer was previously returned by `into_raw`. // 2. The pointer won't be used anymore since the grace period has // finished and this is the only time the pointer gets dropped. - let p = unsafe {

::from_raw(pointer.0) }; + let p = unsafe {

::from_raw(pointer.0) }; drop(p); }); } diff --git a/ostd/src/sync/rcu/non_null.rs b/ostd/src/sync/rcu/non_null.rs new file mode 100644 index 000000000..f33c5765d --- /dev/null +++ b/ostd/src/sync/rcu/non_null.rs @@ -0,0 +1,190 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! This module provides a trait and some auxiliary types to help abstract and +//! work with non-null pointers. + +use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull}; + +use crate::prelude::*; + +/// A trait that abstracts non-null pointers. +/// +/// All common smart pointer types such as `Box`, `Arc`, and `Weak` +/// implement this trait as they can be converted to and from the raw pointer +/// type of `*const T`. +/// +/// # Safety +/// +/// This trait must be implemented correctly (according to the doc comments for +/// each method). Types like [`Rcu`] rely on this assumption to safely use the +/// raw pointers. +/// +/// [`Rcu`]: super::Rcu +pub unsafe trait NonNullPtr: Send + 'static { + /// The target type that this pointer refers to. + // TODO: Support `Target: ?Sized`. + type Target; + + /// A type that behaves just like a shared reference to the `NonNullPtr`. + type Ref<'a>: Deref + where + Self: 'a; + + /// Converts to a raw pointer. + /// + /// Each call to `into_raw` must be paired with a call to `from_raw` + /// in order to avoid memory leakage. + fn into_raw(self) -> NonNull; + + /// Converts back from a raw pointer. + /// + /// # Safety + /// + /// 1. The raw pointer must have been previously returned by a call to + /// `into_raw`. + /// 2. The raw pointer must not be used after calling `from_raw`. + /// + /// Note that the second point is a hard requirement: Even if the + /// resulting value has not (yet) been dropped, the pointer cannot be + /// used because it may break Rust aliasing rules (e.g., `Box` + /// requires the pointer to be unique and thus _never_ aliased). + unsafe fn from_raw(ptr: NonNull) -> Self; + + /// Obtains a shared reference to the original pointer. + /// + /// # Safety + /// + /// The original pointer must outlive the lifetime parameter `'a`, and during `'a` + /// no mutable references to the pointer will exist. + unsafe fn raw_as_ref<'a>(raw: NonNull) -> Self::Ref<'a>; + + /// Converts a shared reference to a raw pointer. + fn ref_as_raw(ptr_ref: Self::Ref<'_>) -> NonNull; +} + +/// A type that represents `&'a Box`. +#[derive(Debug)] +pub struct BoxRef<'a, T> { + inner: *mut T, + _marker: PhantomData<&'a T>, +} + +impl Deref for BoxRef<'_, T> { + type Target = Box; + + fn deref(&self) -> &Self::Target { + // SAFETY: A `Box` is guaranteed to be represented by a single pointer [1] and a shared + // reference to the `Box` during the lifetime `'a` can be created according to the + // safety requirements of `NonNullPtr::raw_as_ref`. + // + // [1]: https://doc.rust-lang.org/std/boxed/#memory-layout + unsafe { core::mem::transmute(&self.inner) } + } +} + +impl<'a, T> BoxRef<'a, T> { + /// Dereferences `self` to get a reference to `T` with the lifetime `'a`. + pub fn deref_target(&self) -> &'a T { + // SAFETY: The reference is created through `NonNullPtr::raw_as_ref`, hence + // the original owned pointer and target must outlive the lifetime parameter `'a`, + // and during `'a` no mutable references to the pointer will exist. + unsafe { &*(self.inner) } + } +} + +unsafe impl NonNullPtr for Box { + type Target = T; + + type Ref<'a> + = BoxRef<'a, T> + where + Self: 'a; + + fn into_raw(self) -> NonNull { + let ptr = Box::into_raw(self); + + // SAFETY: The pointer representing a `Box` can never be NULL. + unsafe { NonNull::new_unchecked(ptr) } + } + + unsafe fn from_raw(ptr: NonNull) -> Self { + let ptr = ptr.as_ptr(); + + // SAFETY: The safety is upheld by the caller. + unsafe { Box::from_raw(ptr) } + } + + unsafe fn raw_as_ref<'a>(raw: NonNull) -> Self::Ref<'a> { + BoxRef { + inner: raw.as_ptr(), + _marker: PhantomData, + } + } + + fn ref_as_raw(ptr_ref: Self::Ref<'_>) -> NonNull { + // SAFETY: The pointer representing a `Box` can never be NULL. + unsafe { NonNull::new_unchecked(ptr_ref.inner) } + } +} + +/// A type that represents `&'a Arc`. +#[derive(Debug)] +pub struct ArcRef<'a, T> { + inner: ManuallyDrop>, + _marker: PhantomData<&'a Arc>, +} + +impl Deref for ArcRef<'_, T> { + type Target = Arc; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl<'a, T> ArcRef<'a, T> { + /// Dereferences `self` to get a reference to `T` with the lifetime `'a`. + pub fn deref_target(&self) -> &'a T { + // SAFETY: The reference is created through `NonNullPtr::raw_as_ref`, hence + // the original owned pointer and target must outlive the lifetime parameter `'a`, + // and during `'a` no mutable references to the pointer will exist. + unsafe { &*(self.deref().deref() as *const T) } + } +} + +unsafe impl NonNullPtr for Arc { + type Target = T; + + type Ref<'a> + = ArcRef<'a, T> + where + Self: 'a; + + fn into_raw(self) -> NonNull { + let ptr = Arc::into_raw(self).cast_mut(); + + // SAFETY: The pointer representing an `Arc` can never be NULL. + unsafe { NonNull::new_unchecked(ptr) } + } + + unsafe fn from_raw(ptr: NonNull) -> Self { + let ptr = ptr.as_ptr().cast_const(); + + // SAFETY: The safety is upheld by the caller. + unsafe { Arc::from_raw(ptr) } + } + + unsafe fn raw_as_ref<'a>(raw: NonNull) -> Self::Ref<'a> { + // SAFETY: The safety is upheld by the caller. + unsafe { + ArcRef { + inner: ManuallyDrop::new(Arc::from_raw(raw.as_ptr())), + _marker: PhantomData, + } + } + } + + fn ref_as_raw(ptr_ref: Self::Ref<'_>) -> NonNull { + NonNullPtr::into_raw(ManuallyDrop::into_inner(ptr_ref.inner)) + } +} diff --git a/ostd/src/sync/rcu/owner_ptr.rs b/ostd/src/sync/rcu/owner_ptr.rs deleted file mode 100644 index a1177595d..000000000 --- a/ostd/src/sync/rcu/owner_ptr.rs +++ /dev/null @@ -1,94 +0,0 @@ -// SPDX-License-Identifier: MPL-2.0 - -use core::ptr::NonNull; - -use crate::prelude::*; - -/// A trait that abstracts pointers that have the ownership of the objects they -/// refer to. -/// -/// The most typical examples smart pointer types like `Box` and `Arc`, -/// which can be converted to and from the raw pointer type of `*const T`. -/// -/// # Safety -/// -/// This trait must be implemented correctly (according to the doc comments for -/// each method). Types like [`Rcu`] rely on this assumption to safely use the -/// raw pointers. -/// -/// [`Rcu`]: super::Rcu -pub unsafe trait OwnerPtr: Send + 'static { - /// The target type that this pointer refers to. - // TODO: allow ?Sized - type Target; - - /// Creates a new pointer with the given value. - fn new(value: Self::Target) -> Self; - - /// Converts to a raw pointer. - /// - /// Each call to `into_raw` must be paired with a call to `from_raw` - /// in order to avoid memory leakage. - /// - /// The resulting raw pointer must be valid to be immutably accessed - /// or borrowed until `from_raw` is called. - fn into_raw(self) -> NonNull; - - /// Converts back from a raw pointer. - /// - /// # Safety - /// - /// 1. The raw pointer must have been previously returned by a call to - /// `into_raw`. - /// 2. The raw pointer must not be used after calling `from_raw`. - /// - /// Note that the second point is a hard requirement: Even if the - /// resulting value has not (yet) been dropped, the pointer cannot be - /// used because it may break Rust aliasing rules (e.g., `Box` - /// requires the pointer to be unique and thus _never_ aliased). - unsafe fn from_raw(ptr: NonNull) -> Self; -} - -unsafe impl OwnerPtr for Box { - type Target = T; - - fn new(value: Self::Target) -> Self { - Box::new(value) - } - - fn into_raw(self) -> NonNull { - let ptr = Box::into_raw(self); - - // SAFETY: The pointer representing a `Box` can never be NULL. - unsafe { NonNull::new_unchecked(ptr) } - } - - unsafe fn from_raw(ptr: NonNull) -> Self { - let ptr = ptr.as_ptr(); - - // SAFETY: The safety is upheld by the caller. - unsafe { Box::from_raw(ptr) } - } -} - -unsafe impl OwnerPtr for Arc { - type Target = T; - - fn new(value: Self::Target) -> Self { - Arc::new(value) - } - - fn into_raw(self) -> NonNull { - let ptr = Arc::into_raw(self).cast_mut(); - - // SAFETY: The pointer representing an `Arc` can never be NULL. - unsafe { NonNull::new_unchecked(ptr) } - } - - unsafe fn from_raw(ptr: NonNull) -> Self { - let ptr = ptr.as_ptr().cast_const(); - - // SAFETY: The safety is upheld by the caller. - unsafe { Arc::from_raw(ptr) } - } -}