Skip to content

feat: bucketized futexes #2469

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.

Sign up for GitHub

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

Open
zyuiop wants to merge 5 commits into
base: main
Choose a base branch
from
+124 −67
Open

feat: bucketized futexes #2469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d9e24f9
feat(futex): multiple locks to reduce lock contention
zyuiop Jun 6, 2026
902bbad
feat(futex): if SMP is disabled, we normally only need one bucket
zyuiop Jun 6, 2026
5b80555
Fix formatting
zyuiop Jun 6, 2026
9eef574
fix: new futex lock was not interrupt safe
zyuiop Jun 6, 2026
49d45ab
futexes: change data structure
zyuiop Sep 8, 2026
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
7 changes: 7 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ num_enum = { version = "0.7", default-features = false }
pci_types = { version = "0.10" }
pci-ids = { version = "0.2", optional = true }
rand_chacha = { version = "0.10", default-features = false }
seahash = "4.1.0"
shlex = { version = "2", default-features = false }
simple-shell = { version = "0.0.1", optional = true }
smallvec = { version = "1", features = ["const_new"] }
Expand Down
183 changes: 116 additions & 67 deletions src/synch/futex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,81 @@
use alloc::collections::BTreeMap;
use core::sync::atomic::AtomicU32;
use core::sync::atomic::Ordering::SeqCst;

use ahash::RandomState;
use hashbrown::HashMap;
use hashbrown::hash_map::Entry;
use hermit_sync::InterruptTicketMutex;
use hermit_sync::{InterruptSpinMutex, InterruptSpinMutexGuard};

use crate::arch::kernel::core_local::core_scheduler;
use crate::arch::kernel::processor::get_timer_ticks;
use crate::errno::Errno;
use crate::scheduler::PerCoreSchedulerExt;
use crate::scheduler::task::TaskHandlePriorityQueue;
use crate::scheduler::task::{TaskHandle, TaskHandlePriorityQueue};

// TODO: Replace with a concurrent hashmap.
static PARKING_LOT: InterruptTicketMutex<HashMap<usize, TaskHandlePriorityQueue, RandomState>> =
InterruptTicketMutex::new(HashMap::with_hasher(RandomState::with_seeds(0, 0, 0, 0)));
type Bucket = InterruptSpinMutex<TaskListBucket>;

#[repr(transparent)]
struct TaskListBucket(BTreeMap<usize, TaskHandlePriorityQueue>);

impl TaskListBucket {
pub fn insert_task(&mut self, address: usize, handle: TaskHandle) {
self.0.entry(address).or_default().push(handle);
}

pub fn contains_task(&self, address: usize, handle: TaskHandle) -> bool {
self.0
.get(&address)
.is_some_and(|queue| queue.contains(handle))
}

/// Removes a task from this bucket, and returns a boolean indicating if it was present.
pub fn remove_task(&mut self, address: usize, task: TaskHandle) -> bool {
let Some(queue) = self.0.get_mut(&address) else {
return false;
};

let was_present = queue.remove(task);
if queue.is_empty() {
self.0.remove(&address);
}

was_present
}

fn get_queue(&mut self, address: usize) -> Option<&mut TaskHandlePriorityQueue> {
self.0.get_mut(&address)
}

fn remove_queue(&mut self, address: usize) {
self.0.remove(&address);
}
}

struct BucketList<const N: usize>([Bucket; N]);

impl<const N: usize> BucketList<N> {
pub const fn new() -> Self {
Self([const { InterruptSpinMutex::new(TaskListBucket(BTreeMap::new())) }; N])
}

fn hash_key(v: usize) -> usize {
let v = (v >> 3).to_be_bytes();

@joboet joboet Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're trying to remove the zero bits resulting from AtomicU32's alignment, then you have to shift by 2, not 3. Since you're hashing anyway, I don't think this is necessary anyway.

@zyuiop zyuiop Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to remove the last 3 bits from the address, which for some reason I believed to always be 0

But you are correct that this is in any case not needed since we hash. It may be the remain of an attempt to not use a hash function at all for better performance.

If it produced a more or less uniformly distributed distribution, (addr >> 3) % N would likely be better as it is less intensive than computing a hash.

Let me know what you think :)

let hashed = seahash::hash(&v) as usize;
hashed % N
}

pub fn lock_bucket(&self, address: usize) -> InterruptSpinMutexGuard<'_, TaskListBucket> {
if N == 1 {
return self.0[0].lock();
}
let bucket = Self::hash_key(address);
self.0[bucket].lock()
}
}

#[cfg(feature = "smp")]
static PARKING_LOT: BucketList<64> = BucketList::new();

#[cfg(not(feature = "smp"))]
static PARKING_LOT: BucketList<1> = BucketList::new();

bitflags! {
pub struct Flags: u32 {
Expand All @@ -23,6 +84,7 @@ bitflags! {
}
}

#[inline(always)]
fn addr(addr: &AtomicU32) -> usize {
let ptr: *const _ = addr;
ptr.addr()
Expand All @@ -40,7 +102,8 @@ pub(crate) fn futex_wait(
timeout: Option<u64>,
flags: Flags,
) -> i32 {
let mut parking_lot = PARKING_LOT.lock();
let address_usize = addr(address);
let mut parking_lot = PARKING_LOT.lock_bucket(address_usize);
// Check the futex value after locking the parking lot so that all changes are observed.
if address.load(SeqCst) != expected {
return -i32::from(Errno::Again);
Expand All @@ -55,40 +118,34 @@ pub(crate) fn futex_wait(
let scheduler = core_scheduler();
scheduler.block_current_task(wakeup_time);
let handle = scheduler.get_current_task_handle();
parking_lot.entry(addr(address)).or_default().push(handle);
parking_lot.insert_task(address_usize, handle);
drop(parking_lot);

loop {
scheduler.reschedule();
// Assume this will return immediately (no other task on core!)

let mut parking_lot = PARKING_LOT.lock();
let mut parking_lot = PARKING_LOT.lock_bucket(address_usize);
if matches!(wakeup_time, Some(t) if t <= get_timer_ticks()) {
let mut wakeup = true;
// Timeout occurred, try to remove ourselves from the waiting queue.
if let Entry::Occupied(mut queue) = parking_lot.entry(addr(address)) {
// If we are not in the waking queue, this must have been a wakeup.
wakeup = !queue.get_mut().remove(handle);
if queue.get().is_empty() {
queue.remove();
}
}
let was_present = parking_lot.remove_task(address_usize, handle);

if wakeup {
return 0;
return if was_present {
-i32::from(Errno::Timedout)
} else {
return -i32::from(Errno::Timedout);
}
// If we are not in the waking queue, this must have been a wakeup.
0
};
} else {
// If we are not in the waking queue, this must have been a wakeup.
let wakeup = !matches!(parking_lot
.get(&addr(address)), Some(queue) if queue.contains(handle));
let is_in_queue = parking_lot.contains_task(address_usize, handle);

if wakeup {
return 0;
} else {
if is_in_queue {
// A spurious wakeup occurred, sleep again.
// Tasks do not change core, so the handle in the parking lot is still current.
scheduler.block_current_task(wakeup_time);
} else {
// If we are not in the waking queue, this must have been a wakeup.
return 0;
}
}
drop(parking_lot);
Expand All @@ -109,7 +166,8 @@ pub(crate) fn futex_wait_and_set(
flags: Flags,
new_value: u32,
) -> i32 {
let mut parking_lot = PARKING_LOT.lock();
let address_usize = addr(address);
let mut parking_lot = PARKING_LOT.lock_bucket(address_usize);
// Check the futex value after locking the parking lot so that all changes are observed.
if address.swap(new_value, SeqCst) != expected {
return -i32::from(Errno::Again);
Expand All @@ -124,40 +182,33 @@ pub(crate) fn futex_wait_and_set(
let scheduler = core_scheduler();
scheduler.block_current_task(wakeup_time);
let handle = scheduler.get_current_task_handle();
parking_lot.entry(addr(address)).or_default().push(handle);
parking_lot.insert_task(address_usize, handle);
drop(parking_lot);

loop {
scheduler.reschedule();

let mut parking_lot = PARKING_LOT.lock();
let mut parking_lot = PARKING_LOT.lock_bucket(address_usize);
if matches!(wakeup_time, Some(t) if t <= get_timer_ticks()) {
let mut wakeup = true;
// Timeout occurred, try to remove ourselves from the waiting queue.
if let Entry::Occupied(mut queue) = parking_lot.entry(addr(address)) {
// If we are not in the waking queue, this must have been a wakeup.
wakeup = !queue.get_mut().remove(handle);
if queue.get().is_empty() {
queue.remove();
}
}
let was_present = parking_lot.remove_task(address_usize, handle);

if wakeup {
return 0;
return if was_present {
-i32::from(Errno::Timedout)
} else {
return -i32::from(Errno::Timedout);
}
// If we are not in the waking queue, this must have been a wakeup.
0
};
} else {
// If we are not in the waking queue, this must have been a wakeup.
let wakeup = !matches!(parking_lot
.get(&addr(address)), Some(queue) if queue.contains(handle));
let is_in_queue = parking_lot.contains_task(address_usize, handle);

if wakeup {
return 0;
} else {
if is_in_queue {
// A spurious wakeup occurred, sleep again.
// Tasks do not change core, so the handle in the parking lot is still current.
scheduler.block_current_task(wakeup_time);
} else {
// If we are not in the waking queue, this must have been a wakeup.
return 0;
}
}
drop(parking_lot);
Expand All @@ -174,24 +225,24 @@ pub(crate) fn futex_wake(address: *const AtomicU32, count: i32) -> i32 {
return -i32::from(Errno::Inval);
}

let mut parking_lot = PARKING_LOT.lock();
let mut queue = match parking_lot.entry(address.addr()) {
Entry::Occupied(entry) => entry,
Entry::Vacant(_) => return 0,
let address_usize = address.addr();
let mut parking_lot = PARKING_LOT.lock_bucket(address_usize);
let Some(queue) = parking_lot.get_queue(address_usize) else {
return 0;
};

let scheduler = core_scheduler();
let mut woken = 0;
while woken != count || count == i32::MAX {
match queue.get_mut().pop() {
match queue.pop() {
Some(handle) => scheduler.custom_wakeup(handle),
None => break,
}
woken = woken.saturating_add(1);
}

if queue.get().is_empty() {
queue.remove();
if queue.is_empty() {
parking_lot.remove_queue(address_usize);
}

woken
Expand All @@ -206,27 +257,25 @@ pub(crate) fn futex_wake_or_set(address: &AtomicU32, count: i32, new_value: u32)
return -i32::from(Errno::Inval);
}

let mut parking_lot = PARKING_LOT.lock();
let mut queue = match parking_lot.entry(addr(address)) {
Entry::Occupied(entry) => entry,
Entry::Vacant(_) => {
address.store(new_value, SeqCst);
return 0;
}
let address_usize = addr(address);
let mut parking_lot = PARKING_LOT.lock_bucket(address_usize);
let Some(queue) = parking_lot.get_queue(address_usize) else {
address.store(new_value, SeqCst);
return 0;
};

let scheduler = core_scheduler();
let mut woken = 0;
while woken != count || count == i32::MAX {
match queue.get_mut().pop() {
match queue.pop() {
Some(handle) => scheduler.custom_wakeup(handle),
None => break,
}
woken = woken.saturating_add(1);
}

if queue.get().is_empty() {
queue.remove();
if queue.is_empty() {
parking_lot.remove_queue(address_usize);
}

if woken == 0 {
Expand Down
Loading