From a589da4efa4115f32411df688d2ca32430a8f829 Mon Sep 17 00:00:00 2001 From: hky1999 <976929993@qq.com> Date: Wed, 30 Oct 2024 00:55:01 +0800 Subject: [PATCH] [feat] refactor axstd sync, expose sync types provided by axsync directly --- Cargo.lock | 1 + api/axfeat/Cargo.toml | 2 +- modules/axsync/Cargo.toml | 1 + modules/axsync/src/condvar.rs | 4 +- modules/axsync/src/lib.rs | 4 +- ulib/axstd/Cargo.toml | 1 + ulib/axstd/src/sync.rs | 26 +++++ ulib/axstd/src/sync/mod.rs | 19 ---- ulib/axstd/src/sync/mutex.rs | 198 ---------------------------------- 9 files changed, 35 insertions(+), 221 deletions(-) create mode 100644 ulib/axstd/src/sync.rs delete mode 100644 ulib/axstd/src/sync/mod.rs delete mode 100644 ulib/axstd/src/sync/mutex.rs diff --git a/Cargo.lock b/Cargo.lock index 227429fa76..cd9aed4045 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -491,6 +491,7 @@ dependencies = [ "axerrno", "axfeat", "axio", + "axsync", "kspin", ] diff --git a/api/axfeat/Cargo.toml b/api/axfeat/Cargo.toml index e535f62111..2da4935fdd 100644 --- a/api/axfeat/Cargo.toml +++ b/api/axfeat/Cargo.toml @@ -19,7 +19,7 @@ smp = ["axhal/smp", "axruntime/smp", "axtask?/smp", "kspin/smp"] fp_simd = ["axhal/fp_simd"] # Interrupts -irq = ["axhal/irq", "axruntime/irq", "axtask?/irq"] +irq = ["axhal/irq", "axruntime/irq", "axtask?/irq", "axsync/irq"] # Memory alloc = ["axalloc", "axruntime/alloc"] diff --git a/modules/axsync/Cargo.toml b/modules/axsync/Cargo.toml index bccfcb9f7a..a6d95c4bbd 100644 --- a/modules/axsync/Cargo.toml +++ b/modules/axsync/Cargo.toml @@ -11,6 +11,7 @@ documentation = "https://arceos-org.github.io/arceos/axsync/index.html" [features] multitask = ["axtask/multitask", "dep:axhal"] +irq = ["axtask/irq"] default = [] [dependencies] diff --git a/modules/axsync/src/condvar.rs b/modules/axsync/src/condvar.rs index 9a73a23e4d..f498aac7c9 100644 --- a/modules/axsync/src/condvar.rs +++ b/modules/axsync/src/condvar.rs @@ -179,7 +179,9 @@ impl Condvar { }; // Lock the mutex again. - (mutex.lock(), WaitTimeoutResult(!success)) + mutex.inner_lock(); + + (guard, WaitTimeoutResult(!success)) } /// Waits on this condition variable for a notification, timing out after a diff --git a/modules/axsync/src/lib.rs b/modules/axsync/src/lib.rs index ce4cc861ef..18b8b8ae67 100644 --- a/modules/axsync/src/lib.rs +++ b/modules/axsync/src/lib.rs @@ -28,10 +28,10 @@ cfg_if::cfg_if! { mod semaphore; pub use self::barrier::{Barrier, BarrierWaitResult}; - pub use self::condvar::Condvar; + pub use self::condvar::{Condvar, WaitTimeoutResult}; #[doc(cfg(feature = "multitask"))] pub use self::mutex::{Mutex, MutexGuard}; - pub use semaphore::Semaphore; + pub use semaphore::{Semaphore, SemaphoreGuard}; } else { #[doc(cfg(not(feature = "multitask")))] pub use kspin::{SpinNoIrq as Mutex, SpinNoIrqGuard as MutexGuard}; diff --git a/ulib/axstd/Cargo.toml b/ulib/axstd/Cargo.toml index 50d91444a5..e46e407c83 100644 --- a/ulib/axstd/Cargo.toml +++ b/ulib/axstd/Cargo.toml @@ -75,6 +75,7 @@ log-level-trace = ["axfeat/log-level-trace"] [dependencies] axfeat = { workspace = true } arceos_api = { workspace = true } +axsync = { workspace = true } axio = "0.1" axerrno = "0.1" kspin = "0.1" diff --git a/ulib/axstd/src/sync.rs b/ulib/axstd/src/sync.rs new file mode 100644 index 0000000000..dd8af2081a --- /dev/null +++ b/ulib/axstd/src/sync.rs @@ -0,0 +1,26 @@ +//! Useful synchronization primitives. + +#[doc(no_inline)] +pub use core::sync::atomic; + +#[cfg(feature = "alloc")] +#[doc(no_inline)] +pub use alloc::sync::{Arc, Weak}; + +// Re-export the `Mutex` and `MutexGuard` types. +pub use axsync::{Mutex, MutexGuard}; + +// Re-export the `RwLock` and related types. +pub use axsync::{ + MappedRwLockReadGuard, MappedRwLockWriteGuard, RwLock, RwLockReadGuard, RwLockWriteGuard, +}; + +// Re-export the `Barrier` and `BarrierWaitResult` types. +#[cfg(feature = "multitask")] +pub use axsync::{Barrier, BarrierWaitResult}; +// Re-export the `Condvar` and `WaitTimeoutResult` types. +#[cfg(feature = "multitask")] +pub use axsync::{Condvar, WaitTimeoutResult}; +// Re-export the `Semaphore` and `SemaphoreGuard` types. +#[cfg(feature = "multitask")] +pub use axsync::{Semaphore, SemaphoreGuard}; diff --git a/ulib/axstd/src/sync/mod.rs b/ulib/axstd/src/sync/mod.rs deleted file mode 100644 index 45546e77c7..0000000000 --- a/ulib/axstd/src/sync/mod.rs +++ /dev/null @@ -1,19 +0,0 @@ -//! Useful synchronization primitives. - -#[doc(no_inline)] -pub use core::sync::atomic; - -#[cfg(feature = "alloc")] -#[doc(no_inline)] -pub use alloc::sync::{Arc, Weak}; - -#[cfg(feature = "multitask")] -mod mutex; - -#[cfg(feature = "multitask")] -#[doc(cfg(feature = "multitask"))] -pub use self::mutex::{Mutex, MutexGuard}; - -#[cfg(not(feature = "multitask"))] -#[doc(cfg(not(feature = "multitask")))] -pub use kspin::{SpinRaw as Mutex, SpinRawGuard as MutexGuard}; // never used in IRQ context diff --git a/ulib/axstd/src/sync/mutex.rs b/ulib/axstd/src/sync/mutex.rs deleted file mode 100644 index d3ed49c907..0000000000 --- a/ulib/axstd/src/sync/mutex.rs +++ /dev/null @@ -1,198 +0,0 @@ -//! A naïve sleeping mutex. - -use core::cell::UnsafeCell; -use core::fmt; -use core::ops::{Deref, DerefMut}; -use core::sync::atomic::{AtomicU64, Ordering}; - -use arceos_api::task::{self as api, AxWaitQueueHandle}; - -/// A mutual exclusion primitive useful for protecting shared data, similar to -/// [`std::sync::Mutex`](https://doc.rust-lang.org/std/sync/struct.Mutex.html). -/// -/// When the mutex is locked, the current task will block and be put into the -/// wait queue. When the mutex is unlocked, all tasks waiting on the queue -/// will be woken up. -pub struct Mutex { - wq: AxWaitQueueHandle, - owner_id: AtomicU64, - data: UnsafeCell, -} - -/// A guard that provides mutable data access. -/// -/// When the guard falls out of scope it will release the lock. -pub struct MutexGuard<'a, T: ?Sized + 'a> { - lock: &'a Mutex, - data: *mut T, -} - -// Same unsafe impls as `std::sync::Mutex` -unsafe impl Sync for Mutex {} -unsafe impl Send for Mutex {} - -impl Mutex { - /// Creates a new [`Mutex`] wrapping the supplied data. - #[inline(always)] - pub const fn new(data: T) -> Self { - Self { - wq: AxWaitQueueHandle::new(), - owner_id: AtomicU64::new(0), - data: UnsafeCell::new(data), - } - } - - /// Consumes this [`Mutex`] and unwraps the underlying data. - #[inline(always)] - pub fn into_inner(self) -> T { - // We know statically that there are no outstanding references to - // `self` so there's no need to lock. - let Mutex { data, .. } = self; - data.into_inner() - } -} - -impl Mutex { - /// Returns `true` if the lock is currently held. - /// - /// # Safety - /// - /// This function provides no synchronization guarantees and so its result should be considered 'out of date' - /// the instant it is called. Do not use it for synchronization purposes. However, it may be useful as a heuristic. - #[inline(always)] - pub fn is_locked(&self) -> bool { - self.owner_id.load(Ordering::Relaxed) != 0 - } - - /// Locks the [`Mutex`] and returns a guard that permits access to the inner data. - /// - /// The returned value may be dereferenced for data access - /// and the lock will be dropped when the guard falls out of scope. - pub fn lock(&self) -> MutexGuard { - let current_id = api::ax_current_task_id(); - loop { - // Can fail to lock even if the spinlock is not locked. May be more efficient than `try_lock` - // when called in a loop. - match self.owner_id.compare_exchange_weak( - 0, - current_id, - Ordering::Acquire, - Ordering::Relaxed, - ) { - Ok(_) => break, - Err(owner_id) => { - assert_ne!( - owner_id, current_id, - "Thread({}) tried to acquire mutex it already owns.", - current_id, - ); - // Wait until the lock looks unlocked before retrying - api::ax_wait_queue_wait_until(&self.wq, || !self.is_locked(), None); - } - } - } - MutexGuard { - lock: self, - data: unsafe { &mut *self.data.get() }, - } - } - - /// Try to lock this [`Mutex`], returning a lock guard if successful. - #[inline(always)] - pub fn try_lock(&self) -> Option> { - let current_id = api::ax_current_task_id(); - // The reason for using a strong compare_exchange is explained here: - // https://github.com/Amanieu/parking_lot/pull/207#issuecomment-575869107 - if self - .owner_id - .compare_exchange(0, current_id, Ordering::Acquire, Ordering::Relaxed) - .is_ok() - { - Some(MutexGuard { - lock: self, - data: unsafe { &mut *self.data.get() }, - }) - } else { - None - } - } - - /// Force unlock the [`Mutex`]. - /// - /// # Safety - /// - /// This is *extremely* unsafe if the lock is not held by the current - /// thread. However, this can be useful in some instances for exposing - /// the lock to FFI that doesn’t know how to deal with RAII. - pub unsafe fn force_unlock(&self) { - let owner_id = self.owner_id.swap(0, Ordering::Release); - let current_id = api::ax_current_task_id(); - assert_eq!( - owner_id, current_id, - "Thread({}) tried to release mutex it doesn't own", - current_id, - ); - // wake up one waiting thread. - api::ax_wait_queue_wake(&self.wq, 1); - } - - /// Returns a mutable reference to the underlying data. - /// - /// Since this call borrows the [`Mutex`] mutably, and a mutable reference is guaranteed to be exclusive in - /// Rust, no actual locking needs to take place -- the mutable borrow statically guarantees no locks exist. As - /// such, this is a 'zero-cost' operation. - #[inline(always)] - pub fn get_mut(&mut self) -> &mut T { - // We know statically that there are no other references to `self`, so - // there's no need to lock the inner mutex. - unsafe { &mut *self.data.get() } - } -} - -impl Default for Mutex { - #[inline(always)] - fn default() -> Self { - Self::new(Default::default()) - } -} - -impl fmt::Debug for Mutex { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.try_lock() { - Some(guard) => write!(f, "Mutex {{ data: ") - .and_then(|()| (*guard).fmt(f)) - .and_then(|()| write!(f, "}}")), - None => write!(f, "Mutex {{ }}"), - } - } -} - -impl Deref for MutexGuard<'_, T> { - type Target = T; - #[inline(always)] - fn deref(&self) -> &T { - // We know statically that only we are referencing data - unsafe { &*self.data } - } -} - -impl DerefMut for MutexGuard<'_, T> { - #[inline(always)] - fn deref_mut(&mut self) -> &mut T { - // We know statically that only we are referencing data - unsafe { &mut *self.data } - } -} - -impl fmt::Debug for MutexGuard<'_, T> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Debug::fmt(&**self, f) - } -} - -impl Drop for MutexGuard<'_, T> { - /// The dropping of the [`MutexGuard`] will release the lock it was created from. - fn drop(&mut self) { - unsafe { self.lock.force_unlock() } - } -}