From 6813c1c69baf870a479c70e23ad0550d1d9aa9be Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Wed, 3 Jun 2020 20:40:22 +0200 Subject: revise RwLock, which is derived from the wasm implementation - increasing the readability of `Condvar` - simplify the interface to the libos HermitCore --- src/libstd/sys/hermit/condvar.rs | 66 ++++++++++++---------- src/libstd/sys/hermit/rwlock.rs | 115 +++++++++++++++++++++++++++++++++++---- 2 files changed, 143 insertions(+), 38 deletions(-) (limited to 'src/libstd/sys') diff --git a/src/libstd/sys/hermit/condvar.rs b/src/libstd/sys/hermit/condvar.rs index 132e579b3a5..cca8db2714d 100644 --- a/src/libstd/sys/hermit/condvar.rs +++ b/src/libstd/sys/hermit/condvar.rs @@ -1,60 +1,70 @@ use crate::cmp; +use crate::ffi::c_void; +use crate::mem; +use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use crate::sys::hermit::abi; use crate::sys::mutex::Mutex; +use crate::ptr; use crate::time::Duration; +// The implementation is inspired by Andrew D. Birrell's paper +// "Implementing Condition Variables with Semaphores" + pub struct Condvar { - identifier: usize, + counter: AtomicUsize, + sem1: *const c_void, + sem2: *const c_void, } +unsafe impl Send for Condvar {} +unsafe impl Sync for Condvar {} + impl Condvar { pub const fn new() -> Condvar { - Condvar { identifier: 0 } + Condvar { + counter: AtomicUsize::new(0), + sem1: ptr::null(), + sem2: ptr::null(), + } } pub unsafe fn init(&mut self) { - let _ = abi::init_queue(self.id()); + let _ = abi::sem_init(&mut self.sem1 as *mut *const c_void, 0); + let _ = abi::sem_init(&mut self.sem2 as *mut *const c_void, 0); } pub unsafe fn notify_one(&self) { - let _ = abi::notify(self.id(), 1); + if self.counter.load(SeqCst) > 0 { + self.counter.fetch_sub(1, SeqCst); + abi::sem_post(self.sem1); + abi::sem_timedwait(self.sem2, 0); + } } - #[inline] pub unsafe fn notify_all(&self) { - let _ = abi::notify(self.id(), -1 /* =all */); + let counter = self.counter.swap(0, SeqCst); + for _ in 0..counter { + abi::sem_post(self.sem1); + } + for _ in 0..counter { + abi::sem_timedwait(self.sem2, 0); + } } pub unsafe fn wait(&self, mutex: &Mutex) { - // add current task to the wait queue - let _ = abi::add_queue(self.id(), -1 /* no timeout */); + self.counter.fetch_add(1, SeqCst); mutex.unlock(); - let _ = abi::wait(self.id()); + abi::sem_timedwait(self.sem1, 0); + abi::sem_post(self.sem2); mutex.lock(); } pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool { - let nanos = dur.as_nanos(); - let nanos = cmp::min(i64::MAX as u128, nanos); - - // add current task to the wait queue - let _ = abi::add_queue(self.id(), nanos as i64); - - mutex.unlock(); - // If the return value is !0 then a timeout happened, so we return - // `false` as we weren't actually notified. - let ret = abi::wait(self.id()) == 0; - mutex.lock(); - - ret + panic!("wait_timeout not supported on hermit"); } pub unsafe fn destroy(&self) { - let _ = abi::destroy_queue(self.id()); - } - - #[inline] - fn id(&self) -> usize { - &self.identifier as *const usize as usize + let _ = abi::sem_destroy(self.sem1); + let _ = abi::sem_destroy(self.sem2); } } diff --git a/src/libstd/sys/hermit/rwlock.rs b/src/libstd/sys/hermit/rwlock.rs index c19799af3c7..06442e925f4 100644 --- a/src/libstd/sys/hermit/rwlock.rs +++ b/src/libstd/sys/hermit/rwlock.rs @@ -1,49 +1,144 @@ -use super::mutex::Mutex; +use crate::cell::UnsafeCell; +use crate::sys::condvar::Condvar; +use crate::sys::mutex::Mutex; pub struct RWLock { - mutex: Mutex, + lock: Mutex, + cond: Condvar, + state: UnsafeCell, +} + +enum State { + Unlocked, + Reading(usize), + Writing, } unsafe impl Send for RWLock {} unsafe impl Sync for RWLock {} +// This rwlock implementation is a relatively simple implementation which has a +// condition variable for readers/writers as well as a mutex protecting the +// internal state of the lock. A current downside of the implementation is that +// unlocking the lock will notify *all* waiters rather than just readers or just +// writers. This can cause lots of "thundering stampede" problems. While +// hopefully correct this implementation is very likely to want to be changed in +// the future. + impl RWLock { pub const fn new() -> RWLock { - RWLock { mutex: Mutex::new() } + RWLock { lock: Mutex::new(), cond: Condvar::new(), state: UnsafeCell::new(State::Unlocked) } } #[inline] pub unsafe fn read(&self) { - self.mutex.lock(); + self.lock.lock(); + while !(*self.state.get()).inc_readers() { + self.cond.wait(&self.lock); + } + self.lock.unlock(); } #[inline] pub unsafe fn try_read(&self) -> bool { - self.mutex.try_lock() + self.lock.lock(); + let ok = (*self.state.get()).inc_readers(); + self.lock.unlock(); + return ok; } #[inline] pub unsafe fn write(&self) { - self.mutex.lock(); + self.lock.lock(); + while !(*self.state.get()).inc_writers() { + self.cond.wait(&self.lock); + } + self.lock.unlock(); } #[inline] pub unsafe fn try_write(&self) -> bool { - self.mutex.try_lock() + self.lock.lock(); + let ok = (*self.state.get()).inc_writers(); + self.lock.unlock(); + return ok; } #[inline] pub unsafe fn read_unlock(&self) { - self.mutex.unlock(); + self.lock.lock(); + let notify = (*self.state.get()).dec_readers(); + self.lock.unlock(); + if notify { + // FIXME: should only wake up one of these some of the time + self.cond.notify_all(); + } } #[inline] pub unsafe fn write_unlock(&self) { - self.mutex.unlock(); + self.lock.lock(); + (*self.state.get()).dec_writers(); + self.lock.unlock(); + // FIXME: should only wake up one of these some of the time + self.cond.notify_all(); } #[inline] pub unsafe fn destroy(&self) { - self.mutex.destroy(); + self.lock.destroy(); + self.cond.destroy(); + } +} + +impl State { + fn inc_readers(&mut self) -> bool { + match *self { + State::Unlocked => { + *self = State::Reading(1); + true + } + State::Reading(ref mut cnt) => { + *cnt += 1; + true + } + State::Writing => false, + } + } + + fn inc_writers(&mut self) -> bool { + match *self { + State::Unlocked => { + *self = State::Writing; + true + } + State::Reading(_) | State::Writing => false, + } + } + + fn dec_readers(&mut self) -> bool { + let zero = match *self { + State::Reading(ref mut cnt) => { + *cnt -= 1; + *cnt == 0 + } + State::Unlocked | State::Writing => invalid(), + }; + if zero { + *self = State::Unlocked; + } + zero } + + fn dec_writers(&mut self) { + match *self { + State::Writing => {} + State::Unlocked | State::Reading(_) => invalid(), + } + *self = State::Unlocked; + } +} + +fn invalid() -> ! { + panic!("inconsistent rwlock"); } -- cgit 1.4.1-3-g733a5