Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use u32 instead of i32 for futexes. #96040

Merged
merged 1 commit into from
Apr 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions library/std/src/sys/unix/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
all(target_os = "emscripten", target_feature = "atomics")
))]

use crate::sync::atomic::AtomicI32;
use crate::sync::atomic::AtomicU32;
use crate::time::Duration;

/// Wait for a futex_wake operation to wake us.
Expand All @@ -13,7 +13,7 @@ use crate::time::Duration;
///
/// Returns false on timeout, and true in all other cases.
#[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 crate::ptr::null;
use crate::sync::atomic::Ordering::Relaxed;
Expand All @@ -35,7 +35,7 @@ pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) -
let r = unsafe {
libc::syscall(
libc::SYS_futex,
futex as *const AtomicI32,
futex as *const AtomicU32,
libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG,
expected,
timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec),
Expand All @@ -53,21 +53,19 @@ pub fn futex_wait(futex: &AtomicI32, expected: i32, timeout: Option<Duration>) -
}

#[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" {
fn emscripten_futex_wait(
addr: *const AtomicI32,
addr: *const AtomicU32,
val: libc::c_uint,
max_wait_ms: libc::c_double,
) -> libc::c_int;
}

unsafe {
emscripten_futex_wait(
futex as *const AtomicI32,
// `val` is declared unsigned to match the Emscripten headers, but since it's used as
// an opaque value, we can ignore the meaning of signed vs. unsigned and cast here.
expected as libc::c_uint,
futex,
expected,
timeout.map_or(crate::f64::INFINITY, |d| d.as_secs_f64() * 1000.0),
);
}
Expand All @@ -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,
/// or false if no thread was waiting on this futex.
#[cfg(any(target_os = "linux", target_os = "android"))]
pub fn futex_wake(futex: &AtomicI32) -> bool {
pub fn futex_wake(futex: &AtomicU32) -> bool {
unsafe {
libc::syscall(
libc::SYS_futex,
futex as *const AtomicI32,
futex as *const AtomicU32,
libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG,
1,
) > 0
Expand All @@ -91,22 +89,22 @@ pub fn futex_wake(futex: &AtomicI32) -> bool {

/// Wake up all threads that are waiting on futex_wait on this futex.
#[cfg(any(target_os = "linux", target_os = "android"))]
pub fn futex_wake_all(futex: &AtomicI32) {
pub fn futex_wake_all(futex: &AtomicU32) {
unsafe {
libc::syscall(
libc::SYS_futex,
futex as *const AtomicI32,
futex as *const AtomicU32,
libc::FUTEX_WAKE | libc::FUTEX_PRIVATE_FLAG,
i32::MAX,
);
}
}

#[cfg(target_os = "emscripten")]
pub fn futex_wake(futex: &AtomicI32) -> bool {
pub fn futex_wake(futex: &AtomicU32) -> bool {
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 }
}
12 changes: 6 additions & 6 deletions library/std/src/sys/unix/locks/futex.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::cell::UnsafeCell;
use crate::sync::atomic::{
AtomicI32, AtomicUsize,
AtomicU32, AtomicUsize,
Ordering::{Acquire, Relaxed, Release},
};
use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all};
Expand All @@ -13,13 +13,13 @@ pub struct Mutex {
/// 0: unlocked
/// 1: locked, no other threads waiting
/// 2: locked, and other threads waiting (contended)
futex: AtomicI32,
futex: AtomicU32,
}

impl Mutex {
#[inline]
pub const fn new() -> Self {
Self { futex: AtomicI32::new(0) }
Self { futex: AtomicU32::new(0) }
}

#[inline]
Expand Down Expand Up @@ -71,7 +71,7 @@ impl Mutex {
}
}

fn spin(&self) -> i32 {
fn spin(&self) -> u32 {
let mut spin = 100;
loop {
// We only use `load` (and not `swap` or `compare_exchange`)
Expand Down Expand Up @@ -110,13 +110,13 @@ pub struct Condvar {
// The value of this atomic is simply incremented on every notification.
// This is used by `.wait()` to not miss any notifications after
// unlocking the mutex and before waiting for notifications.
futex: AtomicI32,
futex: AtomicU32,
}

impl Condvar {
#[inline]
pub const fn new() -> Self {
Self { futex: AtomicI32::new(0) }
Self { futex: AtomicU32::new(0) }
}

#[inline]
Expand Down
40 changes: 20 additions & 20 deletions library/std/src/sys/unix/locks/futex_rwlock.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::sync::atomic::{
AtomicI32,
AtomicU32,
Ordering::{Acquire, Relaxed, Release},
};
use crate::sys::futex::{futex_wait, futex_wake, futex_wake_all};
Expand All @@ -14,36 +14,36 @@ pub struct RwLock {
// 0x3FFF_FFFF: Write locked
// Bit 30: Readers are waiting on this futex.
// Bit 31: Writers are waiting on the writer_notify futex.
state: AtomicI32,
state: AtomicU32,
// The 'condition variable' to notify writers through.
// Incremented on every signal.
writer_notify: AtomicI32,
writer_notify: AtomicU32,
}

const READ_LOCKED: i32 = 1;
const MASK: i32 = (1 << 30) - 1;
const WRITE_LOCKED: i32 = MASK;
const MAX_READERS: i32 = MASK - 1;
const READERS_WAITING: i32 = 1 << 30;
const WRITERS_WAITING: i32 = 1 << 31;
const READ_LOCKED: u32 = 1;
const MASK: u32 = (1 << 30) - 1;
const WRITE_LOCKED: u32 = MASK;
const MAX_READERS: u32 = MASK - 1;
const READERS_WAITING: u32 = 1 << 30;
const WRITERS_WAITING: u32 = 1 << 31;

fn is_unlocked(state: i32) -> bool {
fn is_unlocked(state: u32) -> bool {
state & MASK == 0
}

fn is_write_locked(state: i32) -> bool {
fn is_write_locked(state: u32) -> bool {
state & MASK == WRITE_LOCKED
}

fn has_readers_waiting(state: i32) -> bool {
fn has_readers_waiting(state: u32) -> bool {
state & READERS_WAITING != 0
}

fn has_writers_waiting(state: i32) -> bool {
fn has_writers_waiting(state: u32) -> bool {
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.
//
// We don't allow read-locking if there's readers waiting, even if the lock is unlocked
Expand All @@ -53,14 +53,14 @@ fn is_read_lockable(state: i32) -> bool {
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
}

impl RwLock {
#[inline]
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]
Expand Down Expand Up @@ -227,7 +227,7 @@ impl RwLock {
/// 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.
#[cold]
fn wake_writer_or_readers(&self, mut state: i32) {
fn wake_writer_or_readers(&self, mut state: u32) {
assert!(is_unlocked(state));

// The readers waiting bit might be turned on at any point now,
Expand Down Expand Up @@ -287,7 +287,7 @@ impl RwLock {
}

/// 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.
loop {
let state = self.state.load(Relaxed);
Expand All @@ -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.
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.
self.spin_until(|state| {
!is_write_locked(state) || has_readers_waiting(state) || has_writers_waiting(state)
Expand Down
14 changes: 9 additions & 5 deletions library/std/src/sys/wasm/atomics/futex.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
use crate::arch::wasm32;
use crate::convert::TryInto;
use crate::sync::atomic::AtomicI32;
use crate::sync::atomic::AtomicU32;
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);
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 {
wasm32::memory_atomic_notify(futex as *const AtomicI32 as *mut i32, 1);
wasm32::memory_atomic_notify(futex as *const AtomicU32 as *mut i32, 1);
}
}
12 changes: 6 additions & 6 deletions library/std/src/sys_common/thread_parker/futex.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use crate::sync::atomic::AtomicI32;
use crate::sync::atomic::AtomicU32;
use crate::sync::atomic::Ordering::{Acquire, Release};
use crate::sys::futex::{futex_wait, futex_wake};
use crate::time::Duration;

const PARKED: i32 = -1;
const EMPTY: i32 = 0;
const NOTIFIED: i32 = 1;
const PARKED: u32 = u32::MAX;
const EMPTY: u32 = 0;
const NOTIFIED: u32 = 1;

pub struct Parker {
state: AtomicI32,
state: AtomicU32,
}

// Notes about memory ordering:
Expand All @@ -34,7 +34,7 @@ pub struct Parker {
impl Parker {
#[inline]
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,
Expand Down