Use u32 instead of i32 for futexes.

This commit is contained in:
Mara Bos 2022-04-14 09:51:25 +02:00
parent f387c930ee
commit 7a35c0f52d
5 changed files with 55 additions and 53 deletions

View file

@ -4,7 +4,7 @@
all(target_os = "emscripten", target_feature = "atomics") all(target_os = "emscripten", target_feature = "atomics")
))] ))]
use crate::sync::atomic::AtomicI32; use crate::sync::atomic::AtomicU32;
use crate::time::Duration; use crate::time::Duration;
/// Wait for a futex_wake operation to wake us. /// Wait for a futex_wake operation to wake us.
@ -13,7 +13,7 @@ use crate::time::Duration;
/// ///
/// Returns false on timeout, and true in all other cases. /// Returns false on timeout, and true in all other cases.
#[cfg(any(target_os = "linux", target_os = "android"))] #[cfg(any(target_os = "linux", target_os = "android"))]
pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) -> bool { pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -> bool {
use super::time::Timespec; use super::time::Timespec;
use crate::ptr::null; use crate::ptr::null;
use crate::sync::atomic::Ordering::Relaxed; use crate::sync::atomic::Ordering::Relaxed;
@ -35,7 +35,7 @@ pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) -
let r = unsafe { let r = unsafe {
libc::syscall( libc::syscall(
libc::SYS_futex, libc::SYS_futex,
futex as *const AtomicI32, futex as *const AtomicU32,
libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG, libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG,
expected, expected,
timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec), timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec),
@ -53,10 +53,10 @@ pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) -
} }
#[cfg(target_os = "emscripten")] #[cfg(target_os = "emscripten")]
pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) { pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) {
extern "C" { extern "C" {
fn emscripten_futex_wait( fn emscripten_futex_wait(
addr: *const AtomicI32, addr: *const AtomicU32,
val: libc::c_uint, val: libc::c_uint,
max_wait_ms: libc::c_double, max_wait_ms: libc::c_double,
) -> libc::c_int; ) -> libc::c_int;
@ -64,10 +64,8 @@ pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) {
unsafe { unsafe {
emscripten_futex_wait( emscripten_futex_wait(
futex as *const AtomicI32, futex,
// `val` is declared unsigned to match the Emscripten headers, but since it's used as expected,
// an opaque value, we can ignore the meaning of signed vs. unsigned and cast here.
expected as libc::c_uint,
timeout.map_or(crate::f64::INFINITY, |d| d.as_secs_f64() * 1000.0), timeout.map_or(crate::f64::INFINITY, |d| d.as_secs_f64() * 1000.0),
); );
} }
@ -78,11 +76,11 @@ pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) {
/// Returns true if this actually woke up such a thread, /// Returns true if this actually woke up such a thread,
/// or false if no thread was waiting on this futex. /// or false if no thread was waiting on this futex.
#[cfg(any(target_os = "linux", target_os = "android"))] #[cfg(any(target_os = "linux", target_os = "android"))]
pub fn futex_wake(futex: &AtomicI32) -> bool { pub fn futex_wake(futex: &AtomicU32) -> bool {
unsafe { unsafe {
libc::syscall( libc::syscall(
libc::SYS_futex, libc::SYS_futex,
futex as *const AtomicI32, futex as *const AtomicU32,
libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG, libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG,
1, 1,
) > 0 ) > 0
@ -91,11 +89,11 @@ pub fn futex_wake(futex: &AtomicI32) -> bool {
/// Wake up all threads that are waiting on futex_wait on this futex. /// Wake up all threads that are waiting on futex_wait on this futex.
#[cfg(any(target_os = "linux", target_os = "android"))] #[cfg(any(target_os = "linux", target_os = "android"))]
pub fn futex_wake_all(futex: &AtomicI32) { pub fn futex_wake_all(futex: &AtomicU32) {
unsafe { unsafe {
libc::syscall( libc::syscall(
libc::SYS_futex, libc::SYS_futex,
futex as *const AtomicI32, futex as *const AtomicU32,
libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG, libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG,
i32::MAX, i32::MAX,
); );
@ -103,10 +101,10 @@ pub fn futex_wake_all(futex: &AtomicI32) {
} }
#[cfg(target_os = "emscripten")] #[cfg(target_os = "emscripten")]
pub fn futex_wake(futex: &AtomicI32) -> bool { pub fn futex_wake(futex: &AtomicU32) -> bool {
extern "C" { extern "C" {
fn emscripten_futex_wake(addr: *const AtomicI32, count: libc::c_int) -> libc::c_int; fn emscripten_futex_wake(addr: *const AtomicU32, count: libc::c_int) -> libc::c_int;
} }
unsafe { emscripten_futex_wake(futex as *const AtomicI32, 1) > 0 } unsafe { emscripten_futex_wake(futex, 1) > 0 }
} }

View file

@ -1,6 +1,6 @@
use crate::cell::UnsafeCell; use crate::cell::UnsafeCell;
use crate::sync::atomic::{ use crate::sync::atomic::{
AtomicI32, AtomicUsize, AtomicU32, AtomicUsize,
Ordering::{Acquire, Relaxed, Release}, Ordering::{Acquire, Relaxed, Release},
}; };
use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all}; use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all};
@ -13,13 +13,13 @@ pub struct Mutex {
/// 0: unlocked /// 0: unlocked
/// 1: locked, no other threads waiting /// 1: locked, no other threads waiting
/// 2: locked, and other threads waiting (contended) /// 2: locked, and other threads waiting (contended)
futex: AtomicI32, futex: AtomicU32,
} }
impl Mutex { impl Mutex {
#[inline] #[inline]
pub const fn new() -> Self { pub const fn new() -> Self {
Self { futex: AtomicI32::new(0) } Self { futex: AtomicU32::new(0) }
} }
#[inline] #[inline]
@ -71,7 +71,7 @@ impl Mutex {
} }
} }
fn spin(&self) -> i32 { fn spin(&self) -> u32 {
let mut spin = 100; let mut spin = 100;
loop { loop {
// We only use `load` (and not `swap` or `compare_exchange`) // We only use `load` (and not `swap` or `compare_exchange`)
@ -110,13 +110,13 @@ pub struct Condvar {
// The value of this atomic is simply incremented on every notification. // The value of this atomic is simply incremented on every notification.
// This is used by `.wait()` to not miss any notifications after // This is used by `.wait()` to not miss any notifications after
// unlocking the mutex and before waiting for notifications. // unlocking the mutex and before waiting for notifications.
futex: AtomicI32, futex: AtomicU32,
} }
impl Condvar { impl Condvar {
#[inline] #[inline]
pub const fn new() -> Self { pub const fn new() -> Self {
Self { futex: AtomicI32::new(0) } Self { futex: AtomicU32::new(0) }
} }
#[inline] #[inline]

View file

@ -1,5 +1,5 @@
use crate::sync::atomic::{ use crate::sync::atomic::{
AtomicI32, AtomicU32,
Ordering::{Acquire, Relaxed, Release}, Ordering::{Acquire, Relaxed, Release},
}; };
use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all}; use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all};
@ -14,36 +14,36 @@ pub struct RwLock {
// 0x3FFF_FFFF: Write locked // 0x3FFF_FFFF: Write locked
// Bit 30: Readers are waiting on this futex. // Bit 30: Readers are waiting on this futex.
// Bit 31: Writers are waiting on the writer_notify futex. // Bit 31: Writers are waiting on the writer_notify futex.
state: AtomicI32, state: AtomicU32,
// The 'condition variable' to notify writers through. // The 'condition variable' to notify writers through.
// Incremented on every signal. // Incremented on every signal.
writer_notify: AtomicI32, writer_notify: AtomicU32,
} }
const READ_LOCKED: i32 = 1; const READ_LOCKED: u32 = 1;
const MASK: i32 = (1 << 30) - 1; const MASK: u32 = (1 << 30) - 1;
const WRITE_LOCKED: i32 = MASK; const WRITE_LOCKED: u32 = MASK;
const MAX_READERS: i32 = MASK - 1; const MAX_READERS: u32 = MASK - 1;
const READERS_WAITING: i32 = 1 << 30; const READERS_WAITING: u32 = 1 << 30;
const WRITERS_WAITING: i32 = 1 << 31; const WRITERS_WAITING: u32 = 1 << 31;
fn is_unlocked(state: i32) -> bool { fn is_unlocked(state: u32) -> bool {
state & MASK == 0 state & MASK == 0
} }
fn is_write_locked(state: i32) -> bool { fn is_write_locked(state: u32) -> bool {
state & MASK == WRITE_LOCKED state & MASK == WRITE_LOCKED
} }
fn has_readers_waiting(state: i32) -> bool { fn has_readers_waiting(state: u32) -> bool {
state & READERS_WAITING != 0 state & READERS_WAITING != 0
} }
fn has_writers_waiting(state: i32) -> bool { fn has_writers_waiting(state: u32) -> bool {
state & WRITERS_WAITING != 0 state & WRITERS_WAITING != 0
} }
fn is_read_lockable(state: i32) -> bool { fn is_read_lockable(state: u32) -> bool {
// This also returns false if the counter could overflow if we tried to read lock it. // This also returns false if the counter could overflow if we tried to read lock it.
// //
// We don't allow read-locking if there's readers waiting, even if the lock is unlocked // We don't allow read-locking if there's readers waiting, even if the lock is unlocked
@ -53,14 +53,14 @@ fn is_read_lockable(state: i32) -> bool {
state & MASK < MAX_READERS && !has_readers_waiting(state) && !has_writers_waiting(state) state & MASK < MAX_READERS && !has_readers_waiting(state) && !has_writers_waiting(state)
} }
fn has_reached_max_readers(state: i32) -> bool { fn has_reached_max_readers(state: u32) -> bool {
state & MASK == MAX_READERS state & MASK == MAX_READERS
} }
impl RwLock { impl RwLock {
#[inline] #[inline]
pub const fn new() -> Self { pub const fn new() -> Self {
Self { state: AtomicI32::new(0), writer_notify: AtomicI32::new(0) } Self { state: AtomicU32::new(0), writer_notify: AtomicU32::new(0) }
} }
#[inline] #[inline]
@ -227,7 +227,7 @@ impl RwLock {
/// If both are waiting, this will wake up only one writer, but will fall /// If both are waiting, this will wake up only one writer, but will fall
/// back to waking up readers if there was no writer to wake up. /// back to waking up readers if there was no writer to wake up.
#[cold] #[cold]
fn wake_writer_or_readers(&self, mut state: i32) { fn wake_writer_or_readers(&self, mut state: u32) {
assert!(is_unlocked(state)); assert!(is_unlocked(state));
// The readers waiting bit might be turned on at any point now, // The readers waiting bit might be turned on at any point now,
@ -287,7 +287,7 @@ impl RwLock {
} }
/// Spin for a while, but stop directly at the given condition. /// Spin for a while, but stop directly at the given condition.
fn spin_until(&self, f: impl Fn(i32) -> bool) -> i32 { fn spin_until(&self, f: impl Fn(u32) -> bool) -> u32 {
let mut spin = 100; // Chosen by fair dice roll. let mut spin = 100; // Chosen by fair dice roll.
loop { loop {
let state = self.state.load(Relaxed); let state = self.state.load(Relaxed);
@ -299,12 +299,12 @@ impl RwLock {
} }
} }
fn spin_write(&self) -> i32 { fn spin_write(&self) -> u32 {
// Stop spinning when it's unlocked or when there's waiting writers, to keep things somewhat fair. // Stop spinning when it's unlocked or when there's waiting writers, to keep things somewhat fair.
self.spin_until(|state| is_unlocked(state) || has_writers_waiting(state)) self.spin_until(|state| is_unlocked(state) || has_writers_waiting(state))
} }
fn spin_read(&self) -> i32 { fn spin_read(&self) -> u32 {
// Stop spinning when it's unlocked or read locked, or when there's waiting threads. // Stop spinning when it's unlocked or read locked, or when there's waiting threads.
self.spin_until(|state| { self.spin_until(|state| {
!is_write_locked(state) || has_readers_waiting(state) || has_writers_waiting(state) !is_write_locked(state) || has_readers_waiting(state) || has_writers_waiting(state)

View file

@ -1,17 +1,21 @@
use crate::arch::wasm32; use crate::arch::wasm32;
use crate::convert::TryInto; use crate::convert::TryInto;
use crate::sync::atomic::AtomicI32; use crate::sync::atomic::AtomicU32;
use crate::time::Duration; use crate::time::Duration;
pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) { pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) {
let timeout = timeout.and_then(|t| t.as_nanos().try_into().ok()).unwrap_or(-1); let timeout = timeout.and_then(|t| t.as_nanos().try_into().ok()).unwrap_or(-1);
unsafe { unsafe {
wasm32::memory_atomic_wait32(futex as *const AtomicI32 as *mut i32, expected, timeout); wasm32::memory_atomic_wait32(
futex as *const AtomicU32 as *mut i32,
expected as i32,
timeout,
);
} }
} }
pub fn futex_wake(futex: &AtomicI32) { pub fn futex_wake(futex: &AtomicU32) {
unsafe { unsafe {
wasm32::memory_atomic_notify(futex as *const AtomicI32 as *mut i32, 1); wasm32::memory_atomic_notify(futex as *const AtomicU32 as *mut i32, 1);
} }
} }

View file

@ -1,14 +1,14 @@
use crate::sync::atomic::AtomicI32; use crate::sync::atomic::AtomicU32;
use crate::sync::atomic::Ordering::{Acquire, Release}; use crate::sync::atomic::Ordering::{Acquire, Release};
use crate::sys::futex::{futex_wait, futex_wake}; use crate::sys::futex::{futex_wait, futex_wake};
use crate::time::Duration; use crate::time::Duration;
const PARKED: i32 = -1; const PARKED: u32 = u32::MAX;
const EMPTY: i32 = 0; const EMPTY: u32 = 0;
const NOTIFIED: i32 = 1; const NOTIFIED: u32 = 1;
pub struct Parker { pub struct Parker {
state: AtomicI32, state: AtomicU32,
} }
// Notes about memory ordering: // Notes about memory ordering:
@ -34,7 +34,7 @@ pub struct Parker {
impl Parker { impl Parker {
#[inline] #[inline]
pub const fn new() -> Self { pub const fn new() -> Self {
Parker { state: AtomicI32::new(EMPTY) } Parker { state: AtomicU32::new(EMPTY) }
} }
// Assumes this is only called by the thread that owns the Parker, // Assumes this is only called by the thread that owns the Parker,