Skip to content

Commit ae8794c

Browse files
authored
Rollup merge of #98391 - joboet:sgx_parker, r=m-ou-se
Reimplement std's thread parker on top of events on SGX Mutex and Condvar are being replaced by more efficient implementations, which need thread parking themselves (see #93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used. SGX enclaves have a per-thread event state, which allows waiting for and setting specific bits. This is already used by the current mutex implementation. The thread parker can however be much more efficient, as it only needs to store the `TCS` address of one thread. This address is stored in a state variable, which can also be set to indicate the thread was already notified. `park_timeout` does not guard against spurious wakeups like the current condition variable does. This is allowed by the API of `Parker`, and I think it is better to let users handle these wakeups themselves as the guarding is quite expensive and might not be necessary. `@jethrogb` as you wrote the initial SGX support for `std`, I assume you are the target maintainer? Could you help me test this, please? Lacking a x86_64 chip, I can't run SGX.
2 parents a000811 + a40d300 commit ae8794c

File tree

4 files changed

+119
-12
lines changed

4 files changed

+119
-12
lines changed

library/std/src/sys/sgx/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub mod process;
3434
pub mod stdio;
3535
pub mod thread;
3636
pub mod thread_local_key;
37+
pub mod thread_parker;
3738
pub mod time;
3839

3940
mod condvar;

library/std/src/sys/sgx/thread.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,39 +65,36 @@ mod task_queue {
6565
/// execution. The signal is sent once all TLS destructors have finished at
6666
/// which point no new thread locals should be created.
6767
pub mod wait_notify {
68-
use super::super::waitqueue::{SpinMutex, WaitQueue, WaitVariable};
68+
use super::super::thread_parker::Parker;
69+
use crate::pin::Pin;
6970
use crate::sync::Arc;
7071

71-
pub struct Notifier(Arc<SpinMutex<WaitVariable<bool>>>);
72+
pub struct Notifier(Arc<Parker>);
7273

7374
impl Notifier {
7475
/// Notify the waiter. The waiter is either notified right away (if
7576
/// currently blocked in `Waiter::wait()`) or later when it calls the
7677
/// `Waiter::wait()` method.
7778
pub fn notify(self) {
78-
let mut guard = self.0.lock();
79-
*guard.lock_var_mut() = true;
80-
let _ = WaitQueue::notify_one(guard);
79+
Pin::new(&*self.0).unpark()
8180
}
8281
}
8382

84-
pub struct Waiter(Arc<SpinMutex<WaitVariable<bool>>>);
83+
pub struct Waiter(Arc<Parker>);
8584

8685
impl Waiter {
8786
/// Wait for a notification. If `Notifier::notify()` has already been
8887
/// called, this will return immediately, otherwise the current thread
8988
/// is blocked until notified.
9089
pub fn wait(self) {
91-
let guard = self.0.lock();
92-
if *guard.lock_var() {
93-
return;
94-
}
95-
WaitQueue::wait(guard, || {});
90+
// This is not actually `unsafe`, but it uses the `Parker` API,
91+
// which needs `unsafe` on some platforms.
92+
unsafe { Pin::new(&*self.0).park() }
9693
}
9794
}
9895

9996
pub fn new() -> (Notifier, Waiter) {
100-
let inner = Arc::new(SpinMutex::new(WaitVariable::new(false)));
97+
let inner = Arc::new(Parker::new_internal());
10198
(Notifier(inner.clone()), Waiter(inner))
10299
}
103100
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
//! Thread parking based on SGX events.
2+
3+
use super::abi::{thread, usercalls};
4+
use crate::io::ErrorKind;
5+
use crate::pin::Pin;
6+
use crate::ptr::{self, NonNull};
7+
use crate::sync::atomic::AtomicPtr;
8+
use crate::sync::atomic::Ordering::{Acquire, Relaxed, Release};
9+
use crate::time::Duration;
10+
use fortanix_sgx_abi::{EV_UNPARK, WAIT_INDEFINITE};
11+
12+
// The TCS structure must be page-aligned (this is checked by EENTER), so these cannot
13+
// be valid pointers
14+
const EMPTY: *mut u8 = ptr::invalid_mut(1);
15+
const NOTIFIED: *mut u8 = ptr::invalid_mut(2);
16+
17+
pub struct Parker {
18+
/// The park state. One of EMPTY, NOTIFIED or a TCS address.
19+
/// A state change to NOTIFIED must be done with release ordering
20+
/// and be observed with acquire ordering so that operations after
21+
/// `thread::park` returns will not occur before the unpark message
22+
/// was sent.
23+
state: AtomicPtr<u8>,
24+
}
25+
26+
impl Parker {
27+
/// Construct the thread parker. The UNIX parker implementation
28+
/// requires this to happen in-place.
29+
pub unsafe fn new(parker: *mut Parker) {
30+
unsafe { parker.write(Parker::new_internal()) }
31+
}
32+
33+
pub(super) fn new_internal() -> Parker {
34+
Parker { state: AtomicPtr::new(EMPTY) }
35+
}
36+
37+
// This implementation doesn't require `unsafe` and `Pin`, but other implementations do.
38+
pub unsafe fn park(self: Pin<&Self>) {
39+
if self.state.load(Acquire) != NOTIFIED {
40+
let mut prev = EMPTY;
41+
loop {
42+
// Guard against changing TCS addresses by always setting the state to
43+
// the current value.
44+
let tcs = thread::current().as_ptr();
45+
if self.state.compare_exchange(prev, tcs, Relaxed, Acquire).is_ok() {
46+
let event = usercalls::wait(EV_UNPARK, WAIT_INDEFINITE).unwrap();
47+
assert!(event & EV_UNPARK == EV_UNPARK);
48+
prev = tcs;
49+
} else {
50+
// The state was definitely changed by another thread at this point.
51+
// The only time this occurs is when the state is changed to NOTIFIED.
52+
// We observed this change with acquire ordering, so we can simply
53+
// change the state to EMPTY with a relaxed store.
54+
break;
55+
}
56+
}
57+
}
58+
59+
// At this point, the token was definately read with acquire ordering,
60+
// so this can be a relaxed store.
61+
self.state.store(EMPTY, Relaxed);
62+
}
63+
64+
// This implementation doesn't require `unsafe` and `Pin`, but other implementations do.
65+
pub unsafe fn park_timeout(self: Pin<&Self>, dur: Duration) {
66+
let timeout = u128::min(dur.as_nanos(), WAIT_INDEFINITE as u128 - 1) as u64;
67+
let tcs = thread::current().as_ptr();
68+
69+
if self.state.load(Acquire) != NOTIFIED {
70+
if self.state.compare_exchange(EMPTY, tcs, Relaxed, Acquire).is_ok() {
71+
match usercalls::wait(EV_UNPARK, timeout) {
72+
Ok(event) => assert!(event & EV_UNPARK == EV_UNPARK),
73+
Err(e) => {
74+
assert!(matches!(e.kind(), ErrorKind::TimedOut | ErrorKind::WouldBlock))
75+
}
76+
}
77+
78+
// Swap to provide acquire ordering even if the timeout occurred
79+
// before the token was set. This situation can result in spurious
80+
// wakeups on the next call to `park_timeout`, but it is better to let
81+
// those be handled by the user than do some perhaps unnecessary, but
82+
// always expensive guarding.
83+
self.state.swap(EMPTY, Acquire);
84+
return;
85+
}
86+
}
87+
88+
// The token was already read with `acquire` ordering, this can be a store.
89+
self.state.store(EMPTY, Relaxed);
90+
}
91+
92+
// This implementation doesn't require `Pin`, but other implementations do.
93+
pub fn unpark(self: Pin<&Self>) {
94+
let state = self.state.swap(NOTIFIED, Release);
95+
96+
if !matches!(state, EMPTY | NOTIFIED) {
97+
// There is a thread waiting, wake it up.
98+
let tcs = NonNull::new(state).unwrap();
99+
// This will fail if the thread has already terminated or its TCS is destroyed
100+
// by the time the signal is sent, but that is fine. If another thread receives
101+
// the same TCS, it will receive this notification as a spurious wakeup, but
102+
// all users of `wait` should and (internally) do guard against those where
103+
// necessary.
104+
let _ = usercalls::send(EV_UNPARK, Some(tcs));
105+
}
106+
}
107+
}

library/std/src/sys_common/thread_parker/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ cfg_if::cfg_if! {
1616
pub use wait_flag::Parker;
1717
} else if #[cfg(any(windows, target_family = "unix"))] {
1818
pub use crate::sys::thread_parker::Parker;
19+
} else if #[cfg(all(target_vendor = "fortanix", target_env = "sgx"))] {
20+
pub use crate::sys::thread_parker::Parker;
1921
} else {
2022
mod generic;
2123
pub use generic::Parker;

0 commit comments

Comments
 (0)