From 5bcc2e8ebca42bcb8372a8943d40359b405d3a1a Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 9 Oct 2026 15:45:57 +0800 Subject: [PATCH 1/7] Read a tid once before waiting for it to clear Three tests wait for a thread to exit with while (tid != 0) raw_futex_wait_cleartid(&tid, tid); which reads the volatile tid twice. A thread that exits between the two reads clears it and sends its wake, the second read then passes 0 as the value to wait on, and the wait parks on a word that already holds 0 with no wake left to come. raw_wait_cleartid reads it once per pass. The five sites in test-thread, test-stress and test-credentials use it. This is in the tests, not in elfuse: test-thread's five cases run in a loop stall on main and on Linux 6.12 under qemu as well, within 1000 to 140000 iterations. With the tid read once, six concurrent loops ran 200000 iterations each without a stall. It timed out one make check run of this branch, which is why it is fixed here. --- tests/raw-syscall.h | 10 ++++++++++ tests/test-credentials.c | 3 +-- tests/test-stress.c | 4 +--- tests/test-thread.c | 11 +++-------- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/raw-syscall.h b/tests/raw-syscall.h index 0ba0a3b9f..6d765e2e7 100644 --- a/tests/raw-syscall.h +++ b/tests/raw-syscall.h @@ -255,6 +255,16 @@ static inline long raw_futex_wait_cleartid(int *addr, int val) 0); } +/* Wait until CLONE_CHILD_CLEARTID has cleared *@tid. The tid is read once per + * pass. Read a second time for the futex argument, it can already be 0, and the + * wait then parks on a word whose exit wake has passed. + */ +static inline void raw_wait_cleartid(volatile int *tid) +{ + for (int seen; (seen = *tid) != 0;) + raw_futex_wait_cleartid((int *) tid, seen); +} + static inline long raw_futex_wake(int *addr, int count) { return raw_syscall6(__NR_futex, (long) addr, diff --git a/tests/test-credentials.c b/tests/test-credentials.c index 762b09eb7..4878241c7 100644 --- a/tests/test-credentials.c +++ b/tests/test-credentials.c @@ -105,8 +105,7 @@ static void release_priority_child(void) { prio_child_release = 1; raw_futex_wake((int *) &prio_child_release, 1); - while (prio_child_tid != 0) - raw_futex_wait_cleartid((int *) &prio_child_tid, prio_child_tid); + raw_wait_cleartid(&prio_child_tid); } static long wait_for_dead_priority_tid(int tid) diff --git a/tests/test-stress.c b/tests/test-stress.c index 39366cc80..03a49d944 100644 --- a/tests/test-stress.c +++ b/tests/test-stress.c @@ -73,9 +73,7 @@ static void test_max_threads(void) /* Wait for all children to exit via CLONE_CHILD_CLEARTID */ for (int i = 0; i < STRESS_THREADS; i++) { - while (thread_tids[i] != 0) { - raw_futex_wait_cleartid((int *) &thread_tids[i], thread_tids[i]); - } + raw_wait_cleartid(&thread_tids[i]); } /* Verify all threads ran */ diff --git a/tests/test-thread.c b/tests/test-thread.c index 14c242d4c..e0454db19 100644 --- a/tests/test-thread.c +++ b/tests/test-thread.c @@ -134,9 +134,7 @@ static void test_parent_settid(void) /* child_tid was set by CLONE_PARENT_SETTID in test_clone_thread Wait for * child to fully exit (CLONE_CHILD_CLEARTID clears it) */ - while (child_tid != 0) { - raw_futex_wait_cleartid((int *) &child_tid, child_tid); - } + raw_wait_cleartid(&child_tid); /* If execution reaches this point, CHILD_CLEARTID cleared it and FUTEX_WAKE * woke the parent. The original value was > 0 (written by PARENT_SETTID). @@ -212,9 +210,7 @@ static void test_multi_thread(void) /* Wait for all children to complete (CHILD_CLEARTID clears tids) */ for (int i = 0; i < N_THREADS; i++) { - while (mt_tids[i] != 0) { - raw_futex_wait_cleartid((int *) &mt_tids[i], mt_tids[i]); - } + raw_wait_cleartid(&mt_tids[i]); } /* Verify all threads ran and got unique TIDs */ @@ -282,8 +278,7 @@ static void test_clone_stack_unmap_reuse(void) parked_state = 2; raw_futex_wake((int *) &parked_state, 1); - while (child_tid != 0) - raw_futex_wait_cleartid((int *) &child_tid, child_tid); + raw_wait_cleartid(&child_tid); void *reuse = mmap(stack, stack_size, PROT_READ | PROT_WRITE, From 884cb0875d97b5d329f5699b20efedf4c3d6851f Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 9 Oct 2026 14:19:55 +0800 Subject: [PATCH 2/7] Park a requeued futex waiter under its new lock FUTEX_REQUEUE moves a parked waiter into another bucket's chain, but the waiter went on sleeping, and testing waiter.woken, under the lock of the bucket it entered with. A wake at the new address takes only the new bucket's lock, so nothing ordered the two: - A waiter leaving on a timeout or a signal looked for itself in the chain after the wake had unlinked it, and looped forever holding the new bucket's lock. Every later futex call on that bucket parked behind it. - The waiter could return and destroy its condvar while the wake was still inside pthread_cond_signal on it. main took a host SIGBUS there, in _pthread_cond_updateval, in a 40000-round run. - A wake landing between the waiter's test of woken and its sleep was answered a polling quantum late. Requeue now records the bucket that holds the waiter in waiter.home and signals it, and the waiter re-parks under that bucket's lock before it tests woken or sleeps again. It takes the same lock before it dequeues, which is what makes the test of woken final and the condvar safe to destroy. The search loop that could not end is gone. A futex_waitv waiter sets home at enqueue as well. It leaves through waitv_unlink and never reads the field, but a requeue stores into home on every waiter it moves, and those waiters sit in an uninitialized stack array. tests/test-futex-requeue-race.c requeues a parked thread and then wakes it at the new word, alone, against a signal, and against the waiter's own timeout. FUTEX_WAIT_BITSET is the case this tree can reach: a requeue wakes plain FUTEX_WAIT in place until the next commit moves it to the bucket. On the parent the timeout case hangs in 2 of 5 runs. A build without the re-park fails the first case 3 of 3, and one without the lock before the dequeue fails the signal case. Refs #378 --- Makefile | 7 + src/runtime/futex.c | 90 +++++++------ tests/test-futex-requeue-race.c | 219 ++++++++++++++++++++++++++++++++ tests/test-matrix.sh | 2 + 4 files changed, 281 insertions(+), 37 deletions(-) create mode 100644 tests/test-futex-requeue-race.c diff --git a/Makefile b/Makefile index 3eb5788ec..5f000a306 100644 --- a/Makefile +++ b/Makefile @@ -469,6 +469,13 @@ $(BUILD_DIR)/test-wait-signal-latency: \ @echo " CROSS $< (with -lpthread)" $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread +# test-futex-requeue-race requeues a parked sibling thread and then races a +# signal against a wake at the new word. +$(BUILD_DIR)/test-futex-requeue-race: \ + tests/test-futex-requeue-race.c | $(BUILD_DIR) + @echo " CROSS $< (with -lpthread)" + $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread + # test-nanosleep-process-signal parks several threads in a sleep and sends the # group one signal. $(BUILD_DIR)/test-nanosleep-process-signal: \ diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 5e583bda3..69e987016 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -221,6 +221,17 @@ typedef struct futex_waiter { * to zero, which is a lost wakeup. */ bool pub_follows; + + /* Index of the bucket whose chain holds this waiter. A wake takes only the + * lock of the bucket it finds the waiter in, so the owner has to sleep, and + * test woken, under that same lock or the two are not ordered. Requeue + * stores this under both bucket locks when it moves the waiter and signals + * cond, and the owner loads it to learn which lock to hold. Only a + * futex_wait_inner waiter reads it: a PI waiter is never moved, and a + * futex_waitv one sleeps on its group, and sets it only so that a requeue + * does not store into a field nothing initialized. + */ + _Atomic unsigned home; } futex_waiter_t; /* If the waiter belongs to a futex_waitv group, signal the group's cond so the @@ -378,6 +389,25 @@ static void bucket_unlink_locked(futex_bucket_t *b, const futex_waiter_t *w) } } +/* Trade @b->lock, which the caller holds, for the lock of the bucket @w is + * queued in or was woken from, and return that bucket. home cannot change under + * the lock it names, since moving the waiter out takes that lock, so once this + * returns a wake has either finished with @w or not found it yet. + */ +static futex_bucket_t *futex_waiter_follow(futex_bucket_t *b, + const futex_waiter_t *w) +{ + for (;;) { + futex_bucket_t *home = + &buckets[atomic_load_explicit(&w->home, memory_order_acquire)]; + if (home == b) + return b; + pthread_mutex_unlock(&b->lock); + pthread_mutex_lock(&home->lock); + b = home; + } +} + /* Unlink the waiter at *pp from its bucket list and wake it. The bucket lock * must be held. Unlinking before the store/signal keeps a woken waiter from * observing itself still queued; the release store pairs with the waiter's @@ -1144,6 +1174,7 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, .next = b->head, .pub_bucket = idx, .pub_follows = true, + .home = idx, }; pthread_cond_init(&waiter.cond, NULL); b->head = &waiter; @@ -1151,7 +1182,14 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, /* Wait until woken or timeout */ int ret = 0; - while (!atomic_load_explicit(&waiter.woken, memory_order_acquire)) { + for (;;) { + /* FUTEX_REQUEUE may have moved the waiter since the last pass. Sleep + * under the lock of the bucket that holds it now. + */ + b = futex_waiter_follow(b, &waiter); + if (atomic_load_explicit(&waiter.woken, memory_order_acquire)) + break; + if (has_timeout) { /* Sleep in bounded quanta rather than to the guest deadline: a * worker parked here for a long guest timeout (JVM parkNanos, @@ -1225,43 +1263,14 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, } } - /* Dequeue waiter. If woken=1, the wake/requeue operation already unlinked - * the waiter from the bucket list, so skip dequeue. If woken=0 (timeout / - * interrupt), the waiter is still in the list and must self-dequeue. - * - * For the self-dequeue path: requeue may have moved the waiter to a - * different bucket (changed waiter.uaddr), so re-hash. Race: between - * releasing the old bucket lock and acquiring the new one, another requeue - * can move the waiter again. Loop until the waiter is found and dequeued. + /* Dequeue under the lock a wake would hold. A wake unlinks the waiter, + * stores woken and signals cond all under that lock, so here it has either + * finished, and cond is safe to destroy, or it has not found the waiter, + * which is then still in the chain to unlink. */ - if (!atomic_load_explicit(&waiter.woken, memory_order_acquire)) { - for (;;) { - unsigned dequeue_idx = futex_hash(waiter.uaddr); - futex_bucket_t *b_dequeue = &buckets[dequeue_idx]; - if (b_dequeue != b) { - pthread_mutex_unlock(&b->lock); - pthread_mutex_lock(&b_dequeue->lock); - b = b_dequeue; - } - /* Search for the current waiter in the bucket */ - bool found = false; - futex_waiter_t **pp = &b->head; - while (*pp) { - if (*pp == &waiter) { - *pp = waiter.next; - found = true; - break; - } - pp = &(*pp)->next; - } - if (found) - break; - - /* Not found: waiter was requeued again between the current hash - * computation and lock acquisition. Re-read uaddr and retry. - */ - } - } + b = futex_waiter_follow(b, &waiter); + if (!atomic_load_explicit(&waiter.woken, memory_order_acquire)) + bucket_unlink_locked(b, &waiter); /* Report where the charge ended up, read while the bucket lock is still * held. A requeue rewrites pub_bucket under both bucket locks, so the entry @@ -1566,6 +1575,12 @@ static int64_t futex_requeue(guest_t *g, w->uaddr = uaddr2; w->next = b_dst->head; b_dst->head = w; + + /* The owner sleeps under the source lock, and a wake at uaddr2 + * takes the destination's. Send it to re-park there. + */ + atomic_store_explicit(&w->home, idx_dst, memory_order_release); + pthread_cond_signal(&w->cond); } requeued++; } else { @@ -2505,6 +2520,7 @@ int64_t sys_futex_waitv(guest_t *g, w->group_cond = &shared.cond; w->pub_bucket = idx; w->pub_follows = true; + atomic_store_explicit(&w->home, idx, memory_order_relaxed); pthread_cond_init(&w->cond, NULL); b->head = w; enqueued++; diff --git a/tests/test-futex-requeue-race.c b/tests/test-futex-requeue-race.c new file mode 100644 index 000000000..2cdd24988 --- /dev/null +++ b/tests/test-futex-requeue-race.c @@ -0,0 +1,219 @@ +/* + * A requeued futex waiter leaving its wait while a wake reaches it + * + * Copyright 2026 elfuse contributors + * SPDX-License-Identifier: Apache-2.0 + * + * FUTEX_REQUEUE moves a parked waiter to another futex word, which is how + * musl's pthread_cond_broadcast hands its waiters to the mutex. A wake at the + * new word then races the waiter re-parking there, or leaving on its own with + * EINTR for a signal or ETIMEDOUT for its deadline. Whichever wins, the waiter + * has to return, a wake that counted it must find it returning 0 and promptly, + * and a wake that found nobody must find it reporting why it left. + * + * The two words sit on different pages so they hash to different buckets, and + * the wake is aimed across the waiter's exit so it lands on every side of it. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "test-harness.h" + +int passes = 0, fails = 0; + +/* What, besides the wake, ends the wait. */ +enum { LEAVE_NEVER, LEAVE_SIGNAL, LEAVE_TIMEOUT }; + +#define ROUNDS 20000 +#define TIMEOUT_ROUNDS 4000 +#define MAX_DELAY_SPINS 30000 +#define TIMEOUT_NS 500000LL + +/* How far past the deadline the wake is aimed. A host timeout fires late by + * about half its length, so the window brackets that. + */ +#define AIM_SPAN_NS 600000LL + +/* A wake that reaches a waiter sleeping under the wrong lock is answered a + * polling quantum late, 100 ms. A fifth of that, far above a real handoff. + */ +#define MAX_GAP_NS 20000000LL +#define GIVE_UP_NS 3000000000LL + +static uint32_t word_a __attribute__((aligned(4096))); +static uint32_t word_b __attribute__((aligned(4096))); + +static int wait_op, wait_leave; +static _Atomic int round_go, round_done, waiter_errno; +static _Atomic int waiter_tid; +static _Atomic long long round_deadline_ns, waiter_back_ns; + +static long long clock_ns(clockid_t clock) +{ + struct timespec t; + clock_gettime(clock, &t); + return (long long) t.tv_sec * 1000000000LL + t.tv_nsec; +} + +static void on_signal(int sig) +{ + (void) sig; +} + +static void *waiter(void *arg) +{ + (void) arg; + atomic_store(&waiter_tid, (int) syscall(SYS_gettid)); + for (int seen = 0;;) { + while (atomic_load(&round_go) == seen) + ; + seen = atomic_load(&round_go); + if (seen < 0) + return NULL; + + /* FUTEX_WAIT counts its timeout from the call and FUTEX_WAIT_BITSET + * takes an instant, here on CLOCK_REALTIME. + */ + long long deadline_ns = clock_ns(CLOCK_REALTIME) + TIMEOUT_NS; + struct timespec ts = {0, TIMEOUT_NS}; + if (wait_op != FUTEX_WAIT) { + ts.tv_sec = deadline_ns / 1000000000LL; + ts.tv_nsec = deadline_ns % 1000000000LL; + } + atomic_store(&round_deadline_ns, deadline_ns); + long rc = syscall(SYS_futex, &word_a, wait_op, 0, + wait_leave == LEAVE_TIMEOUT ? &ts : NULL, NULL, + FUTEX_BITSET_MATCH_ANY); + atomic_store(&waiter_errno, rc == 0 ? 0 : errno); + atomic_store(&waiter_back_ns, clock_ns(CLOCK_MONOTONIC)); + atomic_store(&round_done, seen); + } +} + +/* NULL when every round came back consistent, else what went wrong. */ +static const char *race(int op, int leave) +{ + wait_op = op; + wait_leave = leave; + atomic_store(&round_go, 0); + atomic_store(&round_done, 0); + atomic_store(&waiter_tid, 0); + + pthread_t th; + int rc = pthread_create(&th, NULL, waiter, NULL); + if (rc != 0) { + errno = rc; + return "could not start the waiter"; + } + while (!atomic_load(&waiter_tid)) + ; + + unsigned seed = 12345; + int rounds = leave == LEAVE_TIMEOUT ? TIMEOUT_ROUNDS : ROUNDS; + int left_alone = leave == LEAVE_TIMEOUT ? ETIMEDOUT : EINTR; + for (int r = 1; r <= rounds; r++) { + atomic_store(&round_deadline_ns, 0); + atomic_store(&round_go, r); + while (!atomic_load(&round_deadline_ns)) + ; + + /* Requeue returns 0 until the waiter is parked. A timed waiter may + * leave first, and then the round has nothing to race. + */ + long long give_up_ns = clock_ns(CLOCK_MONOTONIC) + GIVE_UP_NS; + while (syscall(SYS_futex, &word_a, FUTEX_REQUEUE, 0, 1, &word_b, 0) != + 1) { + if (atomic_load(&round_done) == r) + break; + if (clock_ns(CLOCK_MONOTONIC) > give_up_ns) + return "the waiter never parked"; + } + + if (leave == LEAVE_TIMEOUT) { + long long aim_ns = + atomic_load(&round_deadline_ns) + rand_r(&seed) % AIM_SPAN_NS; + while (clock_ns(CLOCK_REALTIME) < aim_ns) + ; + } else { + if (leave == LEAVE_SIGNAL && + syscall(SYS_tgkill, getpid(), atomic_load(&waiter_tid), + SIGUSR1) != 0) + return "could not signal the waiter"; + for (volatile long i = rand_r(&seed) % (MAX_DELAY_SPINS + 1); i > 0; + i--) + ; + } + long long woke_ns = clock_ns(CLOCK_MONOTONIC); + long woke = syscall(SYS_futex, &word_b, FUTEX_WAKE, 1, NULL, NULL, 0); + if (woke != 0 && woke != 1) + return "the wake failed"; + + give_up_ns = woke_ns + GIVE_UP_NS; + while (atomic_load(&round_done) != r) { + if (clock_ns(CLOCK_MONOTONIC) > give_up_ns) + return "the waiter never returned"; + } + + int err = atomic_load(&waiter_errno); + if (woke == 1 && err != 0) + return "a wake counted a waiter that reported an error"; + if (woke == 1 && atomic_load(&waiter_back_ns) - woke_ns > MAX_GAP_NS) + return "a wake reached the waiter a polling quantum late"; + if (woke == 0 && (leave == LEAVE_NEVER || err != left_alone)) + return "the waiter returned without a wake or a reason"; + } + + atomic_store(&round_go, -1); + pthread_join(th, NULL); + return NULL; +} + +static void check(const char *name, int op, int leave) +{ + const char *why = race(op, leave); + TEST(name); + if (!why) { + PASS(); + return; + } + + /* Stop here. A waiter that never returned may be holding a bucket lock, and + * the next futex call on that bucket would park this thread behind it. + */ + FAIL(why); + SUMMARY("test-futex-requeue-race"); + fflush(stdout); + _exit(1); +} + +int main(void) +{ + struct sigaction sa; + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = on_signal; + if (sigaction(SIGUSR1, &sa, NULL) != 0) { + printf(" sigaction failed, errno=%d\n", errno); + return 1; + } + + int bitset = FUTEX_WAIT_BITSET | FUTEX_CLOCK_REALTIME; + check("FUTEX_WAIT_BITSET requeued, then woken", bitset, LEAVE_NEVER); + check("FUTEX_WAIT_BITSET requeued, signal against wake", bitset, + LEAVE_SIGNAL); + check("FUTEX_WAIT_BITSET requeued, timeout against wake", bitset, + LEAVE_TIMEOUT); + + SUMMARY("test-futex-requeue-race"); + return 0; +} diff --git a/tests/test-matrix.sh b/tests/test-matrix.sh index b06f0d9dc..780746d9c 100755 --- a/tests/test-matrix.sh +++ b/tests/test-matrix.sh @@ -825,6 +825,8 @@ run_unit_tests() test_check "$runner" "test-cntvct-thread" "0 failed" \ "$bindir/test-cntvct-thread" test_check "$runner" "test-osync-requeue" "0 failed" "$bindir/test-osync-requeue" + test_check "$runner" "test-futex-requeue-race" "0 failed" \ + "$bindir/test-futex-requeue-race" test_check "$runner" "test-simd-clone" "0 failed" "$bindir/test-simd-clone" test_check "$runner" "test-stress" "0 failed" "$bindir/test-stress" test_rc "$runner" "test-thread-churn" 0 "$bindir/test-thread-churn" From aae06fb6672db5a5c13182eaf2afe7ce81dda5c2 Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 9 Oct 2026 13:44:05 +0800 Subject: [PATCH 3/7] Park plain FUTEX_WAIT on the bucket condvar Plain FUTEX_WAIT parked in os_sync_wait_on_address, and the only way to reach a thread parked there is to wake its address. That is also what FUTEX_WAKE does, so the waiter cannot tell a wake from anything else that needs it out of the wait. Ending the wait for a queued signal that way made a waiter a FUTEX_WAKE had already counted report EINTR, and made a signal the waiter could not claim, an ignored SIGCHLD among them, return 0 to it and to every sibling on the word. A bucket waiter has waiter.woken to tell the two apart, so plain FUTEX_WAIT now takes the path FUTEX_WAIT_BITSET always has, and a later commit adds the signal wake on top of it. FUTEX_REQUEUE moves a plain waiter instead of waking it in place, which is the path musl's pthread_cond_broadcast takes and why the previous commit comes first. tests/test-futex-requeue-race.c gains the plain case. The address-wait code stays, unreached, behind os_sync_wait_enabled. Cost, main against the tip of this branch, alternating, medians of 9: the bench-futex handoff reads 128.5 against 129.1 ns per round trip. bench-futex-contend with 4 threads reads 1.085 against 1.123 s, with ranges of 0.941 to 1.190 and 1.023 to 1.209, and with 8 threads 0.946 against 0.948 s. A relative timeout now overshoots about twice as far, 515 us rather than 264 us on a 1 ms wait, where Linux 6.12 under qemu overshoots 590 us. Refs #378 --- Makefile | 4 ++-- src/runtime/futex.c | 30 ++++++++---------------------- tests/bench-futex.c | 21 ++++++++++----------- tests/test-exec-handoff.c | 9 ++++----- tests/test-exit-group-worker.c | 3 +-- tests/test-futex-requeue-race.c | 4 ++++ tests/test-osync-requeue.c | 27 ++++++++++++--------------- 7 files changed, 41 insertions(+), 57 deletions(-) diff --git a/Makefile b/Makefile index 5f000a306..0d634153d 100644 --- a/Makefile +++ b/Makefile @@ -581,8 +581,8 @@ $(BUILD_DIR)/test-sigtimedwait: tests/test-sigtimedwait.c | $(BUILD_DIR) $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread # test-osync-requeue drives a raw FUTEX_REQUEUE against a plain-FUTEX_WAIT -# waiter (musl unlock_requeue pattern) to guard the os_sync wake-at-source -# degradation; needs -lpthread. +# waiter (musl unlock_requeue pattern), which a requeue that only walked part of +# the waiters would strand; needs -lpthread. $(BUILD_DIR)/test-osync-requeue: tests/test-osync-requeue.c | $(BUILD_DIR) @echo " CROSS $< (with -lpthread)" $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 69e987016..3ce32f6a3 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -118,9 +118,8 @@ _Static_assert(FUTEX_WAKE_BITSET == 10, * * os_sync_available is set in futex_init() when the runtime supports the * os_sync_wait_on_address family (macOS 14.4+). os_sync_wait_enabled gates - * whether plain FUTEX_WAIT / FUTEX_WAKE use the address-wait path; it is set - * alongside os_sync_available now that futex_os_sync_wait's compare-after-block - * re-check preserves Linux's -EAGAIN race semantics. + * whether plain FUTEX_WAIT / FUTEX_WAKE use the address-wait path; futex_init() + * leaves it off and says why. * * The wait quantum is capped at 100 ms so proc_exit_group_requested() and * futex_interrupt_consume() get noticed promptly without a process-wide @@ -605,26 +604,13 @@ void futex_init(void) if (__builtin_available(macOS 14.4, *)) { os_sync_available = true; - /* Plain FUTEX_WAIT / FUTEX_WAKE take the Darwin address-wait path. - * Enable the gate only where the API is actually available so the two - * flags cannot disagree. The late-EAGAIN gap is bridged by the - * compare-after-block re-check in futex_os_sync_wait (a rc>=0 return - * with the word moved off expected maps to -EAGAIN, matching Linux for - * the pre-block race; the post-wake case is equally safe since a - * correct caller re-reads the word either way). FUTEX_REQUEUE of such a - * waiter cannot migrate a kernel os_sync waiter between addresses, so - * futex_requeue degrades the requeue portion into a wake at the source - * address; woken callers re-acquire what they need in userspace. glibc - * condvars are safe because broadcast/signal wake with FUTEX_WAKE, not - * requeue (2.25+ dropped the requeue optimization); musl's - * private-condvar barrier->mutex handoff tolerates wake-in-place, - * matching its own emscripten fallback. Only plain FUTEX_WAIT enqueues - * on the os_sync queue; FUTEX_WAIT_BITSET, PI waits, and futex_waitv - * stay on the bucket path. Every wake site drains both queues via - * futex_wake_topup_osync, so os_sync waiters are reached regardless of - * which wake op (FUTEX_WAKE, requeue, wake_op) fires. + /* os_sync_wait_enabled stays false, so plain FUTEX_WAIT parks on the + * bucket path with the rest. A queued signal has to end the wait with + * EINTR, and the only way to reach an address-wait park is to wake its + * address, which the waiter cannot tell from a FUTEX_WAKE: it reports + * EINTR for a wake that counted it, or 0 for a signal it could not + * claim. A bucket waiter has waiter.woken to tell the two apart. */ - os_sync_wait_enabled = true; } #endif } diff --git a/tests/bench-futex.c b/tests/bench-futex.c index a4c4b6aee..48377b69a 100644 --- a/tests/bench-futex.c +++ b/tests/bench-futex.c @@ -34,9 +34,9 @@ * as a regression. * * Set BENCH_FUTEX_HANDOFF_BITSET to run the handoff with FUTEX_WAIT_BITSET and - * MATCH_ANY instead of plain FUTEX_WAIT. Semantically the same wait, but the - * two take different backends inside elfuse, and that row is how the difference - * is measured. + * MATCH_ANY instead of plain FUTEX_WAIT. Semantically the same wait, on the + * same bucket path inside elfuse; the row is kept for comparing the two + * spellings. * * Run under ELFUSE_SHIM_STATS=1 to attribute the fast-path rows: the host * prints FUTEX_EAGAIN_HIT, FUTEX_EFAULT_HIT, FUTEX_SHAPE_BAIL and @@ -364,14 +364,13 @@ static double run_mainworker_floor(unsigned long iters) /* concurrent wake scaling */ -/* futex_wake takes the hash-bucket lock even when the waiter it is looking for - * sits on the Darwin address-wait queue rather than in the chain, so threads - * waking unrelated futexes contend whenever their addresses collide. This row - * is that collision and nothing else: every thread wakes its own private word - * and no waiter exists anywhere, so all it measures is a lock acquire and an - * empty walk. Divide it by the single-threaded wake-nowaiter row to read the - * contention factor. A bucket table too narrow to keep those two close is what - * this catches, and nothing else in the suite does. +/* futex_wake takes the hash-bucket lock even when nothing is queued in the + * chain, so threads waking unrelated futexes contend whenever their addresses + * collide. This row is that collision and nothing else: every thread wakes its + * own private word and no waiter exists anywhere, so all it measures is a lock + * acquire and an empty walk. Divide it by the single-threaded wake-nowaiter row + * to read the contention factor. A bucket table too narrow to keep those two + * close is what this catches, and nothing else in the suite does. */ #define WAKE_SCALE_THREADS 8 #define WAKE_SCALE_STACK 4096 diff --git a/tests/test-exec-handoff.c b/tests/test-exec-handoff.c index 5d94e2f5a..4065111b2 100644 --- a/tests/test-exec-handoff.c +++ b/tests/test-exec-handoff.c @@ -476,11 +476,10 @@ static void expect_timeout_still_expires(void) double sleep_ms = elapsed_ms(&t0, &t1); /* The other two relative-timeout waits reach the same hazard through - * different code: a plain FUTEX_WAIT, which elfuse serves on an - * address-wait fast path rather than the bucket path FUTEX_WAIT_BITSET - * uses, and sigtimedwait, which spends its timeout in a chunk loop of its - * own. Neither is retried here, because one bounded return is the whole - * question. + * different code: a plain FUTEX_WAIT, whose timeout is relative where + * FUTEX_WAIT_BITSET's is absolute, and sigtimedwait, which spends its + * timeout in a chunk loop of its own. Neither is retried here, because one + * bounded return is the whole question. */ int word = 0; struct timespec rel = {1, 0}; diff --git a/tests/test-exit-group-worker.c b/tests/test-exit-group-worker.c index cbee53be3..c950723a2 100644 --- a/tests/test-exit-group-worker.c +++ b/tests/test-exit-group-worker.c @@ -60,8 +60,7 @@ static void *spinner_fn(void *arg) } /* Park in a timed futex wait far in the future. FUTEX_WAIT_BITSET takes an - * absolute deadline and is what glibc timed waits issue; on elfuse it maps to - * the condvar wait path rather than the os_sync one, so this exercises the + * absolute deadline and is what glibc timed waits issue, so this exercises the * bounded-quantum teardown re-check. Re-arm on any spurious wake; exit_group is * the only way out. */ diff --git a/tests/test-futex-requeue-race.c b/tests/test-futex-requeue-race.c index 2cdd24988..97c05ac91 100644 --- a/tests/test-futex-requeue-race.c +++ b/tests/test-futex-requeue-race.c @@ -208,6 +208,10 @@ int main(void) } int bitset = FUTEX_WAIT_BITSET | FUTEX_CLOCK_REALTIME; + check("FUTEX_WAIT requeued, then woken", FUTEX_WAIT, LEAVE_NEVER); + check("FUTEX_WAIT requeued, signal against wake", FUTEX_WAIT, LEAVE_SIGNAL); + check("FUTEX_WAIT requeued, timeout against wake", FUTEX_WAIT, + LEAVE_TIMEOUT); check("FUTEX_WAIT_BITSET requeued, then woken", bitset, LEAVE_NEVER); check("FUTEX_WAIT_BITSET requeued, signal against wake", bitset, LEAVE_SIGNAL); diff --git a/tests/test-osync-requeue.c b/tests/test-osync-requeue.c index 30fffe0ba..0853c0674 100644 --- a/tests/test-osync-requeue.c +++ b/tests/test-osync-requeue.c @@ -1,14 +1,12 @@ /* - * Spike probe: does routing plain FUTEX_WAIT to the Darwin os_sync address-wait - * queue break FUTEX_REQUEUE of that waiter? + * Does FUTEX_REQUEUE reach a waiter parked by plain FUTEX_WAIT? * * musl's private condvar (pthread_cond_timedwait.c: lock()/unlock_requeue()) * parks waiters with a plain FUTEX_WAIT on a per-node "barrier" word, then its * broadcast/handoff path uses FUTEX_REQUEUE to move those parked waiters from - * the barrier word onto the mutex word. If plain FUTEX_WAIT enqueues on the - * kernel os_sync queue while FUTEX_REQUEUE only walks the emulator's hash - * bucket, the requeue reaches nobody: the waiter is stranded until the 100 ms - * os_sync poll cap re-checks the (now changed) word and returns. + * the barrier word onto the mutex word. Plain FUTEX_WAIT once parked on a + * Darwin address-wait queue that FUTEX_REQUEUE could not walk, and a requeue + * that reaches nobody strands the waiter until its 100 ms polling quantum ends. * * This reproduces the pattern with raw syscalls (libc-agnostic) and measures * the wake latency. A correct requeue path wakes the waiter in well under the @@ -69,9 +67,8 @@ int main(void) } /* Let the waiter reach FUTEX_WAIT. Keep this well below the difference - * between the 100 ms os_sync poll cap (FUTEX_OS_SYNC_POLL_CAP_NS) and the - * pass threshold below, so a stranded waiter (regression: requeue no longer - * degrades to a wake at the source address) surfaces near the cap, far + * between the 100 ms polling quantum (FUTEX_OS_SYNC_POLL_CAP_NS) and the + * pass threshold below, so a stranded waiter surfaces near the quantum, far * above the threshold, instead of landing on it. */ usleep(10 * 1000); @@ -96,12 +93,12 @@ int main(void) printf(" requeue rc=%d wake rc=%d\n", rq, wk); printf(" wake latency: %.2f ms\n", latency); - /* A working requeue path wakes well under the 100 ms os_sync poll cap. - * Allow generous headroom for scheduler jitter; a stranded waiter only - * comes back on the ~100 ms timeout. Also require the requeue or wake to - * have reached the waiter (rq/wk > 0): if the waiter had not blocked yet, - * both reach nobody and the waiter exits fast on its own, which would - * otherwise show a false sub-threshold PASS. + /* A working requeue path wakes well under the 100 ms polling quantum. Allow + * generous headroom for scheduler jitter; a stranded waiter only comes back + * on the ~100 ms timeout. Also require the requeue or wake to have reached + * the waiter (rq/wk > 0): if the waiter had not blocked yet, both reach + * nobody and the waiter exits fast on its own, which would otherwise show a + * false sub-threshold PASS. */ int reached = (rq > 0) || (wk > 0); int pass = reached && latency < 50.0; From 4d0af31b26c6449c0cdf71011718110e7ffccaf2 Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 9 Oct 2026 13:44:18 +0800 Subject: [PATCH 4/7] Wake a futex wait on a queued signal A thread parked in FUTEX_WAIT or FUTEX_WAIT_BITSET is outside hv_vcpu_run and watches neither the wakeup pipe nor its condition variable, so it noticed a queued signal only when its 100 ms polling quantum ended: 43 ms from pthread_kill to the handler at the median, against 0.044 ms under Linux 6.12. Queueing a signal now kicks every other thread that has a pending signal it does not block. A parked thread publishes the mutex and condvar it sleeps on. The kick sets a flag and then signals under that mutex, and the thread tests the flag under the same mutex before it sleeps, so a kick that arrives ahead of the park is not lost. A waiter that FUTEX_REQUEUE moved publishes again under its new bucket's lock. A kicked thread that finds nothing to claim parks again, which is what an ignored signal or one a sibling took comes to. FUTEX_LOCK_PI and futex_waitv keep their quantum. tests/test-futex-signal-latency.c fails all four cases on the parent commit and passes here, and requires EINTR from each wait. Not in the tree: 40000 signals aimed at the entry of each wait were all answered within 0.19 ms, a FUTEX_WAKE racing a signal never left a counted waiter reporting EINTR in 40000 rounds, and an ignored SIGCHLD left both waiters on a word parked, as under Linux 6.12 in qemu. Refs #378 --- Makefile | 7 + src/runtime/futex.c | 91 +++++++- src/runtime/futex.h | 7 + src/runtime/thread.c | 31 ++- src/runtime/thread.h | 36 ++- src/syscall/internal.h | 9 +- src/syscall/signal.c | 5 + tests/test-futex-signal-latency.c | 331 +++++++++++++++++++++++++++ tests/test-matrix.sh | 2 + tests/test-teardown-live-vcpu-host.c | 5 + 10 files changed, 503 insertions(+), 21 deletions(-) create mode 100644 tests/test-futex-signal-latency.c diff --git a/Makefile b/Makefile index 0d634153d..277915e0c 100644 --- a/Makefile +++ b/Makefile @@ -476,6 +476,13 @@ $(BUILD_DIR)/test-futex-requeue-race: \ @echo " CROSS $< (with -lpthread)" $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread +# test-futex-signal-latency aims a signal at a sibling thread parked in a futex +# wait. +$(BUILD_DIR)/test-futex-signal-latency: \ + tests/test-futex-signal-latency.c | $(BUILD_DIR) + @echo " CROSS $< (with -lpthread)" + $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread + # test-nanosleep-process-signal parks several threads in a sleep and sends the # group one signal. $(BUILD_DIR)/test-nanosleep-process-signal: \ diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 3ce32f6a3..bc2c882a7 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -374,6 +374,75 @@ static bool futex_poll_signal_relock(futex_bucket_t *b) return sig_ready; } +/* Signal kick. A thread parked in FUTEX_WAIT or FUTEX_WAIT_BITSET watches + * neither the wakeup pipe nor its condition variable, so futex_kick is what + * tells it a signal was queued. The polling quantum stays, for teardown. + * + * The kicker sets futex_kick and then looks for a park; the thread publishes + * its park and then tests futex_kick before it sleeps. All of it is seq_cst, so + * at least one of the two sees the other. The park is signalled under the mutex + * it published, which closes the window between its test and its sleep. A + * waiter FUTEX_REQUEUE moved publishes again under its new bucket's lock. + */ +static bool futex_kick_consume(void) +{ + thread_entry_t *t = current_thread; + return t && atomic_exchange_explicit(&t->futex_kick, 0, + memory_order_seq_cst) != 0; +} + +/* Caller holds @lock, and holds it again whenever it tests the kick. */ +static void futex_park_publish(pthread_mutex_t *lock, pthread_cond_t *cond) +{ + thread_entry_t *t = current_thread; + if (!t) + return; + atomic_store_explicit(&t->futex_park_cond, cond, memory_order_seq_cst); + atomic_store_explicit(&t->futex_park_lock, lock, memory_order_seq_cst); +} + +/* Under the mutex the park published and before its condvar is destroyed: the + * kicker signals only while futex_park_lock, re-read under that mutex, still + * names it. + */ +static void futex_park_withdraw(void) +{ + thread_entry_t *t = current_thread; + if (t) + atomic_store_explicit(&t->futex_park_lock, NULL, memory_order_seq_cst); +} + +/* Sleep on a published park until @until, unless a kick is already waiting. The + * caller runs its signal check after either. + */ +static void futex_park_wait(pthread_cond_t *cond, + pthread_mutex_t *lock, + const struct timespec *until) +{ + if (!futex_kick_consume()) + pthread_cond_timedwait(cond, lock, until); +} + +void futex_kick(thread_entry_t *t) +{ + atomic_store_explicit(&t->futex_kick, 1, memory_order_seq_cst); + + pthread_mutex_t *lock = + atomic_load_explicit(&t->futex_park_lock, memory_order_seq_cst); + if (!lock) + return; + + pthread_mutex_lock(lock); + pthread_mutex_t *still = + atomic_load_explicit(&t->futex_park_lock, memory_order_seq_cst); + if (still == lock) { + pthread_cond_t *cond = + atomic_load_explicit(&t->futex_park_cond, memory_order_seq_cst); + pthread_cond_signal(cond); + } + pthread_mutex_unlock(lock); +} + /* Unlink a waiter from its bucket's singly-linked list. Caller must hold * b->lock. Silently returns if the waiter is not in the list (already unlinked * by a wake/requeue). @@ -1164,15 +1233,21 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, }; pthread_cond_init(&waiter.cond, NULL); b->head = &waiter; + futex_park_publish(&b->lock, &waiter.cond); /* Wait until woken or timeout */ int ret = 0; for (;;) { - /* FUTEX_REQUEUE may have moved the waiter since the last pass. Sleep + /* FUTEX_REQUEUE may have moved the waiter since the last pass. Re-park * under the lock of the bucket that holds it now. */ - b = futex_waiter_follow(b, &waiter); + if (&buckets[atomic_load_explicit(&waiter.home, + memory_order_acquire)] != b) { + futex_park_withdraw(); + b = futex_waiter_follow(b, &waiter); + futex_park_publish(&b->lock, &waiter.cond); + } if (atomic_load_explicit(&waiter.woken, memory_order_acquire)) break; @@ -1189,7 +1264,7 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, ret = -LINUX_ETIMEDOUT; break; } - pthread_cond_timedwait(&waiter.cond, &b->lock, &quantum); + futex_park_wait(&waiter.cond, &b->lock, &quantum); if (thread_stop_requested() || futex_interrupt_consume()) { ret = -LINUX_EINTR; break; @@ -1217,11 +1292,12 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, } /* No timeout specified: poll every 100 ms to check for exit_group, - * futex_interrupt, expired guest itimers, and queued signals. + * futex_interrupt and expired guest itimers. A queued signal does not + * wait for the poll: its kick ends the sleep. */ struct timespec poll_ts; timespec_deadline_in_ms(&poll_ts, 100); - pthread_cond_timedwait(&waiter.cond, &b->lock, &poll_ts); + futex_park_wait(&waiter.cond, &b->lock, &poll_ts); if (thread_stop_requested() || futex_interrupt_consume()) { ret = -LINUX_EINTR; @@ -1249,6 +1325,11 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, } } + /* Still under the bucket lock the park was published with; the dequeue + * below may trade it for another. + */ + futex_park_withdraw(); + /* Dequeue under the lock a wake would hold. A wake unlinks the waiter, * stores woken and signals cond all under that lock, so here it has either * finished, and cond is safe to destroy, or it has not found the waiter, diff --git a/src/runtime/futex.h b/src/runtime/futex.h index c631736ce..642242b07 100644 --- a/src/runtime/futex.h +++ b/src/runtime/futex.h @@ -29,6 +29,13 @@ void futex_interrupt_request(void); void futex_interrupt_clear(void); int futex_interrupt_consume(void); +/* Tell @t that a signal it can take was queued, and wake it if it is parked in + * FUTEX_WAIT or FUTEX_WAIT_BITSET. The kick is remembered, so one that lands + * before @t parks is answered when it does. FUTEX_LOCK_PI and futex_waitv do + * not take part and keep their polling quantum. + */ +void futex_kick(thread_entry_t *t); + /* Main futex syscall entry point. op: futex operation (FUTEX_WAIT, FUTEX_WAKE, * etc.) uaddr: guest virtual address of the futex word val: expected value * (WAIT) or max wakeups (WAKE) timeout_gva: guest pointer to timespec (or 0 for diff --git a/src/runtime/thread.c b/src/runtime/thread.c index 6b3ea1859..52f7d5a15 100644 --- a/src/runtime/thread.c +++ b/src/runtime/thread.c @@ -200,14 +200,21 @@ _Static_assert(THREAD_SLOT_KEPT(tpending.pending), "tpending.pending is cleared by the memset"); _Static_assert(THREAD_SLOT_KEPT(in_syscall), "in_syscall is cleared by the memset"); +_Static_assert(THREAD_SLOT_KEPT(futex_park_lock), + "futex_park_lock is cleared by the memset"); +_Static_assert(THREAD_SLOT_KEPT(futex_park_cond), + "futex_park_cond is cleared by the memset"); +_Static_assert(THREAD_SLOT_KEPT(futex_kick), + "futex_kick is cleared by the memset"); /* Clear a slot that thread_alloc is about to hand to a new thread. * - * A whole-struct memset cannot be used. Three scans walk the table lock-free - * and each loads active and then a payload field with no lock in between, so - * one of them can still be reading this slot after it went inactive and while - * the recycle runs. Plain-writing any field they read is a data race, so the - * five they touch are stored atomically and the memset skips them. + * A whole-struct memset cannot be used. The scans that walk the table lock-free + * each load active and then a payload field with no lock in between, so one of + * them can still be reading this slot after it went inactive and while the + * recycle runs. Plain-writing any field they read is a data race, so the memset + * skips them all: five are stored atomically here, and the futex park fields + * are left as the last owner cleared them (runtime/thread.h). */ static void thread_slot_clear(thread_entry_t *t) { @@ -853,6 +860,20 @@ bool thread_signal_deliverable(uint64_t sigbit) return false; } +void thread_kick_futex_waiters(uint64_t shared_pending) +{ + /* Lock-free scan, on the same terms as thread_signal_deliverable above. A + * stale blocked or pending read costs a kick the thread answers by parking + * again, or misses one its own polling quantum still covers. The caller is + * skipped: it tests its own pending signals on the way out. + */ + THREAD_FOR_EACH_ACTIVE_RELAXED (t) { + uint64_t pending = pending_load(&t->tpending.pending) | shared_pending; + if (t != current_thread && (pending & ~thread_blocked_load(t))) + futex_kick(t); + } +} + /* Fork quiesce. */ bool thread_quiesce_siblings(void) diff --git a/src/runtime/thread.h b/src/runtime/thread.h index 92f153a8d..c7aa89db4 100644 --- a/src/runtime/thread.h +++ b/src/runtime/thread.h @@ -43,16 +43,16 @@ * signal API without an include cycle. */ typedef struct thread_entry { - /* Read without thread_lock. Three scans walk the table lock-free - * (thread_pending_union, thread_tid_alive, thread_signal_deliverable): each - * loads active, then one of blocked, guest_tid, or tpending.pending, with - * nothing in between. A scanner that saw active as 1 just before this slot - * went inactive can still be loading one of those when thread_alloc - * recycles the slot, so none of them may be plain-written there. They are - * grouped here, cleared with atomic stores, and deliberately excluded from - * the memset that clears the rest. Keep them ahead of the rest of the - * struct; thread.c asserts that each one sits below where its memset - * starts. + /* Read without thread_lock. Four scans walk the table lock-free + * (thread_pending_union, thread_tid_alive, thread_signal_deliverable, + * thread_kick_futex_waiters): each loads active, then one or two of + * blocked, guest_tid, and tpending.pending, with nothing in between. A + * scanner that saw active as 1 just before this slot went inactive can + * still be loading one of those when thread_alloc recycles the slot, so + * none of them may be plain-written there. They are grouped here, cleared + * with atomic stores, and deliberately excluded from the memset that clears + * the rest. Keep them ahead of the rest of the struct; thread.c asserts + * that each one sits below where its memset starts. */ _Atomic int active; /* Non-zero while thread is running. Stays int (not * bool) because lock-free paths in thread.c use @@ -74,6 +74,15 @@ typedef struct thread_entry { */ _Atomic int32_t in_syscall; + /* Where this thread is parked in a futex wait, for futex_kick, which loads + * these with no lock (runtime/futex.c states the protocol). Not cleared on + * recycle: a thread leaves its wait before it exits, and a kick left set + * costs the next owner one early pass of its wait loop. + */ + pthread_mutex_t *_Atomic futex_park_lock; /* Mutex of a condvar park */ + pthread_cond_t *_Atomic futex_park_cond; /* Valid under that mutex */ + _Atomic uint32_t futex_kick; /* A kick not answered yet */ + /* Thread-directed pending signals (Linux task->pending). tgkill/tkill and * pthread_kill queue here so only this thread consumes them. Written and * read under the signal module's sig_lock; do not touch without it. Only @@ -468,6 +477,13 @@ void thread_wake_exit_waiters(void); */ bool thread_signal_deliverable(uint64_t sigbit); +/* Kick every other thread that has a pending signal it does not block, so one + * parked in a futex wait re-checks now rather than when its polling quantum + * ends. @shared_pending is the process-directed pending set. Lock-free scan; + * safe from any thread. + */ +void thread_kick_futex_waiters(uint64_t shared_pending); + /* Fork quiesce helpers. */ /* Quiesce all sibling vCPUs for fork snapshot consistency. Calls hv_vcpus_exit diff --git a/src/syscall/internal.h b/src/syscall/internal.h index a6487d9f3..0de01ebd9 100644 --- a/src/syscall/internal.h +++ b/src/syscall/internal.h @@ -100,7 +100,14 @@ * cwd_lock dropped * pid_lock (syscall/proc.c): process table / wait state * pidfd_lock (syscall/proc-pidfd.c): pidfd registry - * futex bucket (runtime/futex.c): per-bucket, index-ordered if >1 + * futex bucket (runtime/futex.c): per-bucket, index-ordered if >1. Also + * taken alone by futex_kick, beneath + * whatever lock a signal is queued under, + * the FUSE session lock among them. That + * closes no cycle: a bucket lock holds only + * another bucket's and a futex_waitv group + * lock beneath it, and a signal is queued + * under neither * cwd_lock (syscall/proc-state.c): cached guest cwd. A leaf on every * path but one: proc_acquire_cwd_view * returns still holding it, and both diff --git a/src/syscall/signal.c b/src/syscall/signal.c index 1cc86f271..ccf62ad32 100644 --- a/src/syscall/signal.c +++ b/src/syscall/signal.c @@ -543,6 +543,10 @@ void signal_set_shim_globals_guest(guest_t *g) * wait -- which hv_vcpus_exit cannot reach because it is not inside hv_vcpu_run * -- wakes and rechecks signal_pending(). Matches how exit_group and * futex_interrupt already signal the pipe. + * + * A futex wait is on neither, so it gets a kick of its own. The shared set is + * read without sig_lock, like the pending hint: a signal queued before this + * call is in it, and a bit that has since been taken costs one spare kick. */ static inline void attention_raise(void) { @@ -552,6 +556,7 @@ static inline void attention_raise(void) else thread_interrupt_all(); wakeup_pipe_signal(); + thread_kick_futex_waiters(pending_load(&sig_state.shared.pending)); } /* Predicate matches the deliverability gate used by signal_queue and diff --git a/tests/test-futex-signal-latency.c b/tests/test-futex-signal-latency.c new file mode 100644 index 000000000..6e8f6aa4e --- /dev/null +++ b/tests/test-futex-signal-latency.c @@ -0,0 +1,331 @@ +/* + * Signal delivery latency to a thread parked in a futex wait + * + * Copyright 2026 elfuse contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The companion of test-nanosleep-signal-latency and test-wait-signal-latency + * for futex. A waiter is parked host-side, outside hv_vcpu_run, and on neither + * the wakeup pipe nor its condition variable, so a queued signal reaches it + * only through the kick the signal sends. A wait that is not kicked still + * returns EINTR, but only once its polling quantum ends, so the defect shows as + * delay rather than as a wrong result. Nothing wakes the futex word, so nothing + * but the signal can end the wait. + * + * FUTEX_WAIT and FUTEX_WAIT_BITSET are both exercised because they take their + * timeout on different terms, relative and absolute, and each with and without + * one because the two size their quantum differently. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "test-harness.h" + +int passes = 0, fails = 0; + +/* A fifth of the 100 ms quantum that is the failure being ruled out, and far + * above the sub-millisecond gap a kicked wait shows. + */ +#define MAX_GAP_MS 20.0 + +/* A wait that spins instead of parking still answers the signal at once, so + * only its CPU share gives it away. A parked waiter reads under 1% through + * CLOCK_THREAD_CPUTIME_ID and a spinning one 10 to 13%. + */ +#define MAX_BUSY 0.05 +#define ITERS 8 +#define SLEEP_SEC 3 + +/* Out-of-band results, below any gap a real measurement can produce. */ +#define GAP_NO_THREAD (-1.0) +#define GAP_NO_HANDLER (-2.0) +#define GAP_NOT_EINTR (-3.0) +#define GAP_NO_RETURN (-4.0) + +/* Written by the handler on the waiter thread and read by the main thread's + * wait loop. volatile carries no inter-thread ordering, and a lock-free atomic + * store is what a handler may use. + */ +static _Atomic int handler_ran; +static _Atomic int64_t handler_ns; + +/* Set by the waiter immediately before it enters the wait. */ +static _Atomic int waiter_armed; + +/* The waiter's CPU share over its wait, stored before it exits. */ +static _Atomic double waiter_busy; + +static long long now_ns(void) +{ + struct timespec t; + clock_gettime(CLOCK_MONOTONIC, &t); + return (long long) t.tv_sec * 1000000000LL + t.tv_nsec; +} + +static long long thread_cpu_ns(void) +{ + struct timespec t; + clock_gettime(CLOCK_THREAD_CPUTIME_ID, &t); + return (long long) t.tv_sec * 1000000000LL + t.tv_nsec; +} + +typedef struct { + long long cpu_ns, wall_ns; +} wait_start_t; + +static wait_start_t wait_arm(void) +{ + wait_start_t start = {thread_cpu_ns(), now_ns()}; + atomic_store_explicit(&waiter_armed, 1, memory_order_release); + return start; +} + +static void wait_done(wait_start_t start) +{ + double busy = (double) (thread_cpu_ns() - start.cpu_ns) / + (double) (now_ns() - start.wall_ns); + atomic_store_explicit(&waiter_busy, busy, memory_order_release); +} + +static void on_signal(int sig) +{ + (void) sig; + atomic_store_explicit(&handler_ns, now_ns(), memory_order_relaxed); + atomic_store_explicit(&handler_ran, 1, memory_order_release); +} + +/* Never written and never woken. */ +static uint32_t idle_word; + +/* What the wait returned, stored before the waiter exits. Only EINTR is right: + * a 0 here is a wake nobody sent. WAIT_PENDING until the wait returns. + */ +#define WAIT_PENDING (-1) +static _Atomic int waiter_errno; + +static void futex_wait_op(int op, const struct timespec *timeout) +{ + wait_start_t start = wait_arm(); + long rc = syscall(SYS_futex, &idle_word, op, 0, timeout, NULL, + FUTEX_BITSET_MATCH_ANY); + atomic_store_explicit(&waiter_errno, rc == 0 ? 0 : errno, + memory_order_release); + wait_done(start); +} + +static void *wait_finite(void *arg) +{ + (void) arg; + struct timespec rel = {SLEEP_SEC, 0}; + futex_wait_op(FUTEX_WAIT, &rel); + return NULL; +} + +static void *wait_indefinite(void *arg) +{ + (void) arg; + futex_wait_op(FUTEX_WAIT, NULL); + return NULL; +} + +/* FUTEX_WAIT_BITSET takes its timeout as an absolute instant, on the clock + * glibc's pthread_cond_timedwait names. + */ +static void *wait_bitset_finite(void *arg) +{ + (void) arg; + struct timespec until; + clock_gettime(CLOCK_REALTIME, &until); + until.tv_sec += SLEEP_SEC; + futex_wait_op(FUTEX_WAIT_BITSET | FUTEX_CLOCK_REALTIME, &until); + return NULL; +} + +static void *wait_bitset_indefinite(void *arg) +{ + (void) arg; + futex_wait_op(FUTEX_WAIT_BITSET, NULL); + return NULL; +} + +static void sort_doubles(double *v, int n) +{ + for (int i = 1; i < n; i++) { + double key = v[i]; + int j = i - 1; + while (j >= 0 && v[j] > key) { + v[j + 1] = v[j]; + j--; + } + v[j + 1] = key; + } +} + +/* Median gap from sending the signal to the handler running, GAP_NO_THREAD if a + * thread could not be started, GAP_NO_HANDLER if one never ran its handler, + * GAP_NO_RETURN if a wait outlived its handler, or GAP_NOT_EINTR if a wait + * returned anything else. @busy_out receives the median CPU share of the waiter + * over its wait. + * + * The median rather than the minimum: an unjoined wake leaves the gap spread + * across the quantum rather than pinned to its top, so a minimum would report + * the one lucky iteration that arrived just before a recheck. + */ +static double median_gap(void *(*body)(void *), double *busy_out) +{ + double gaps[ITERS], busy[ITERS]; + + for (int i = 0; i < ITERS; i++) { + pthread_t th; + atomic_store_explicit(&handler_ran, 0, memory_order_relaxed); + atomic_store_explicit(&handler_ns, 0, memory_order_relaxed); + atomic_store_explicit(&waiter_armed, 0, memory_order_relaxed); + atomic_store_explicit(&waiter_errno, WAIT_PENDING, + memory_order_relaxed); + + int rc = pthread_create(&th, NULL, body, NULL); + if (rc != 0) { + errno = rc; + return GAP_NO_THREAD; + } + + /* Wait for the waiter to arm, then let it reach the host wait. A signal + * that arrives first is answered by the check at the top of the wait + * loop, which is fast whether or not the wait itself is joined, and a + * slow pthread_create would otherwise put iterations there. + */ + long long armed_by_ns = now_ns() + (long long) SLEEP_SEC * 1000000000LL; + while (!atomic_load_explicit(&waiter_armed, memory_order_acquire)) { + if (now_ns() > armed_by_ns) { + /* Detached rather than joined on every failure path: a waiter + * with no timeout that no signal reaches never returns, and the + * exit after SUMMARY tears it down instead. + */ + pthread_detach(th); + return GAP_NO_THREAD; + } + } + struct timespec settle = {0, (150 + i * 11) * 1000000L}; + nanosleep(&settle, NULL); + + long long sent_ns = now_ns(); + rc = pthread_kill(th, SIGUSR1); + if (rc != 0) { + pthread_detach(th); + errno = rc; + return GAP_NO_THREAD; + } + + /* Bounded, so a regression that strands the waiter fails the test + * rather than hanging the lane it runs in. + */ + long long give_up_ns = sent_ns + (long long) SLEEP_SEC * 2000000000LL; + while (!atomic_load_explicit(&handler_ran, memory_order_acquire)) { + if (now_ns() > give_up_ns) { + pthread_detach(th); + return GAP_NO_HANDLER; + } + } + + gaps[i] = + (double) (atomic_load_explicit(&handler_ns, memory_order_relaxed) - + sent_ns) / + 1e6; + + /* Bounded as well. The settle above does not prove the waiter parked: a + * waiter held between arming and its syscall takes the signal first, + * and then nothing ends a wait that has no timeout. + */ + give_up_ns = now_ns() + (long long) SLEEP_SEC * 2000000000LL; + while (atomic_load_explicit(&waiter_errno, memory_order_acquire) == + WAIT_PENDING) { + if (now_ns() > give_up_ns) { + pthread_detach(th); + return GAP_NO_RETURN; + } + } + pthread_join(th, NULL); + if (atomic_load_explicit(&waiter_errno, memory_order_acquire) != EINTR) + return GAP_NOT_EINTR; + busy[i] = atomic_load_explicit(&waiter_busy, memory_order_acquire); + } + + sort_doubles(busy, ITERS); + *busy_out = (busy[ITERS / 2 - 1] + busy[ITERS / 2]) / 2.0; + sort_doubles(gaps, ITERS); + return (gaps[ITERS / 2 - 1] + gaps[ITERS / 2]) / 2.0; +} + +static void check(const char *name, void *(*body)(void *) ) +{ + double busy = 0; + double gap = median_gap(body, &busy); + TEST(name); + const char *gave_up = NULL; + if (gap == GAP_NO_THREAD) + gave_up = "could not start or signal a waiter"; + else if (gap == GAP_NO_HANDLER) + gave_up = "the handler never ran"; + else if (gap == GAP_NO_RETURN) + gave_up = "the wait did not return after the handler ran"; + if (gave_up) { + /* Stop here. The waiter was detached and may still be running, and it + * reports through the globals the next check would read as its own. + */ + FAIL(gave_up); + SUMMARY("test-futex-signal-latency"); + fflush(stdout); + _exit(1); + } + if (gap == GAP_NOT_EINTR) { + FAIL("the wait did not return EINTR"); + return; + } + char msg[96]; + if (gap >= MAX_GAP_MS) { + snprintf(msg, sizeof(msg), + "handler ran %.1fms after the signal (max %.0f)", gap, + MAX_GAP_MS); + FAIL(msg); + return; + } + if (busy > MAX_BUSY) { + snprintf(msg, sizeof(msg), "waiter spent %.0f%% of its wait on CPU", + busy * 100.0); + FAIL(msg); + return; + } + PASS(); +} + +int main(void) +{ + struct sigaction sa; + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = on_signal; + if (sigaction(SIGUSR1, &sa, NULL) != 0) { + printf(" sigaction failed, errno=%d\n", errno); + return 1; + } + + check("FUTEX_WAIT with a timeout wakes on a queued signal", wait_finite); + check("FUTEX_WAIT without a timeout wakes on a queued signal", + wait_indefinite); + check("FUTEX_WAIT_BITSET with a timeout wakes on a queued signal", + wait_bitset_finite); + check("FUTEX_WAIT_BITSET without a timeout wakes on a queued signal", + wait_bitset_indefinite); + + SUMMARY("test-futex-signal-latency"); + return fails ? 1 : 0; +} diff --git a/tests/test-matrix.sh b/tests/test-matrix.sh index 780746d9c..883aa4677 100755 --- a/tests/test-matrix.sh +++ b/tests/test-matrix.sh @@ -733,6 +733,8 @@ run_unit_tests() "$bindir/test-nanosleep-process-signal" test_check "$runner" "test-wait-signal-latency" "PASS" \ "$bindir/test-wait-signal-latency" + test_check "$runner" "test-futex-signal-latency" "PASS" \ + "$bindir/test-futex-signal-latency" test_check "$runner" "test-wait-process-signal" " - PASS" \ "$bindir/test-wait-process-signal" test_check "$runner" "test-wait-sigmask-signal" " - PASS" \ diff --git a/tests/test-teardown-live-vcpu-host.c b/tests/test-teardown-live-vcpu-host.c index a2d428df9..df68c6086 100644 --- a/tests/test-teardown-live-vcpu-host.c +++ b/tests/test-teardown-live-vcpu-host.c @@ -45,6 +45,11 @@ int proc_exit_group_requested(void) void futex_interrupt_request(void) {} +void futex_kick(thread_entry_t *t) +{ + (void) t; +} + void wakeup_pipe_signal(void) {} void exec_handoff_wake_waiters(void) {} From 14496cb1e11a4f6475186241cbaadbf94d36f36c Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 9 Oct 2026 13:44:19 +0800 Subject: [PATCH 5/7] Remove the unreached address-wait futex path Plain FUTEX_WAIT no longer parks in os_sync_wait_on_address, so nothing reaches futex_os_sync_wait, the per-bucket os_sync_waiters census that told a wake whether to drain the Darwin queue, or futex_wake_topup_osync, which every bucket-walking wake called to do it. They go, with the SDK and runtime probes that gated them. futex_wake, futex_requeue and futex_wake_op return what the bucket walk counted, and futex_should_block loses the out-parameter only the address wait used. No behavior changes: the gate these sat behind has been off since the commit that moved FUTEX_WAIT to the bucket. The note on why the address wait cannot serve FUTEX_WAIT moves to the FUTEX_WAIT case in sys_futex. FUTEX_OS_SYNC_POLL_CAP_NS stays, as the quantum of every futex wait, and the next commit renames it. Refs #378 --- scripts/check-eintr-contract.py | 4 - src/core/shim.S | 3 +- src/runtime/futex.c | 487 +++----------------------------- 3 files changed, 37 insertions(+), 457 deletions(-) diff --git a/scripts/check-eintr-contract.py b/scripts/check-eintr-contract.py index a68f33c80..6e5de8eca 100644 --- a/scripts/check-eintr-contract.py +++ b/scripts/check-eintr-contract.py @@ -76,10 +76,6 @@ "A finite epoll wait has spent part of the guest's timeout, and ready " "events outrank leader work because kqueue consumes the edge.", ), - "runtime/futex.c::futex_os_sync_wait": ( - "forbids", - "The address-wait path plain FUTEX_WAIT takes; its deadline is " "relative.", - ), "runtime/futex.c::futex_wait_inner": ( "forbids", "Relative FUTEX_WAIT has spent part of its timeout; FUTEX_WAIT_BITSET " diff --git a/src/core/shim.S b/src/core/shim.S index 9fdbbccb9..964a31d47 100644 --- a/src/core/shim.S +++ b/src/core/shim.S @@ -1009,8 +1009,7 @@ attn_bail: * because every wait there genuinely blocks. * * No address check beyond alignment. The host resolves nothing for a wake - * either: futex_wake matches enqueued waiters by raw uaddr, and the os_sync - * top-up returns 0 rather than EFAULT when guest_ptr fails. So an unmapped or + * either: futex_wake matches enqueued waiters by raw uaddr. So an unmapped or * tagged address answers 0 on both sides, and hashing the raw value here is * what keeps the two agreeing. */ diff --git a/src/runtime/futex.c b/src/runtime/futex.c index bc2c882a7..4b298f9fd 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -48,25 +48,6 @@ #include "proved/futexwakeop.h" #include "proved/timespec.h" -/* macOS 14.4+ ships os_sync_{wait_on_address_with_timeout,wake_by_address_any} - * with futex-style compare-and-wait semantics on process-private addresses. - * elfuse routes plain FUTEX_WAIT / FUTEX_WAKE through this path. Darwin folds - * the Linux -EAGAIN pre-block race into a successful wait; futex_os_sync_wait - * closes that common gap with a compare-after-block re-check (word moved off - * the expected value on a rc>=0 return maps to -EAGAIN). FUTEX_WAIT_BITSET, PI - * variants, and futex_waitv stay on the bucket path: they need state the kernel - * API does not expose. - * - * SDK gate: probe the header via __has_include so older SDKs build clean. - * Runtime gate: __builtin_available cached in futex_init(). - */ -#if __has_include() -#include -#define ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS 1 -#else -#define ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS 0 -#endif - /* Interrupt flag: when set, futex_wait returns -EINTR. Raised only by teardown * through thread_wake_all_blocked, so every blocked wait can observe that the * process is tearing down without a full exit_group. @@ -114,14 +95,7 @@ _Static_assert(FUTEX_WAKE_BITSET == 10, * which carries the layout and Linux's own constants. */ -/* Address-wait helper state. - * - * os_sync_available is set in futex_init() when the runtime supports the - * os_sync_wait_on_address family (macOS 14.4+). os_sync_wait_enabled gates - * whether plain FUTEX_WAIT / FUTEX_WAKE use the address-wait path; futex_init() - * leaves it off and says why. - * - * The wait quantum is capped at 100 ms so proc_exit_group_requested() and +/* The wait quantum is capped at 100 ms so proc_exit_group_requested() and * futex_interrupt_consume() get noticed promptly without a process-wide * broadcast channel. EINTR is only returned when an actual deliverable signal * is queued for this thread (confirmed under sig_lock via signal_pending(), not @@ -132,23 +106,18 @@ _Static_assert(FUTEX_WAKE_BITSET == 10, * multi-threaded runtimes, but that broke POSIX sem_wait callers that do not * retry on EINTR (e.g. foot's render worker). */ -#if ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS -static bool os_sync_available; -static bool os_sync_wait_enabled; -#endif - #define FUTEX_OS_SYNC_POLL_CAP_NS (100ULL * 1000 * 1000) /* Hash table. * * Sized for wakers, not waiters. At most MAX_THREADS (64) threads can be queued * at once, so 64 buckets would already keep the chains short; what makes the - * table too narrow is that futex_wake takes the bucket lock even when the - * waiter it is looking for lives on the Darwin address-wait queue rather than - * in the chain. Two threads waking unrelated futexes therefore serialize - * whenever their addresses collide, which at 64 buckets is often enough to cost - * more than the wake itself: eight threads each waking their own private futex - * measured 801 ns per wake, worse than the same test at four threads. + * table too narrow is that futex_wake takes the bucket lock even when nothing + * is queued in the chain. Two threads waking unrelated futexes therefore + * serialize whenever their addresses collide, which at 64 buckets is often + * enough to cost more than the wake itself: eight threads each waking their own + * private futex measured 801 ns per wake, worse than the same test at four + * threads. * * At 1024, paired with the multiply-shift hash below, the collapse is gone (316 * ns per wake against 801, and eight threads now beat four). Width alone was @@ -248,70 +217,17 @@ static void futex_waiter_notify_group(futex_waiter_t *w) /* One bucket in the hash table. Protected by its own mutex. Lock order: 7 (leaf * locks, index-ordered when two acquired). Padded to a cache line. The struct - * is 80 bytes and this host's line is 128, so unpadded neighbours share a line + * is 72 bytes and this host's line is 128, so unpadded neighbours share a line * and two futexes the hash correctly separated still contend on it. Measured * with eight threads each taking its own adjacent bucket: 16-18 ns per lock * unpadded against 5-8 ns padded, a 2-4x share of exactly the contention the * 64-to-1024 widening was made to remove. The padding itself costs BSS only, - * 81920 bytes to 131072 at this bucket count; the table's total is above + * 73728 bytes to 131072 at this bucket count; the table's total is above * FUTEX_BUCKETS. */ typedef struct { pthread_mutex_t lock; futex_waiter_t *head; /* Linked list of waiters hashing to this bucket */ - - /* Threads inside futex_os_sync_wait for an address hashing here, so a wake - * can tell an empty Darwin address-wait queue from a populated one without - * asking the kernel. - * - * Every bucket-walking wake tops up from that queue when it woke fewer than - * asked, and the top-up costs a guest_ptr walk plus an - * os_sync_wake_by_address_any syscall whether or not anyone is queued - * (measured: 242 ns per wake). Only plain FUTEX_WAIT enqueues there, so a - * guest whose waiters are all FUTEX_WAIT_BITSET, PI, or futex_waitv paid - * for a queue that could not hold a waiter. - * - * Per bucket rather than process-wide: one idle worker parked in - * pthread_cond_wait puts musl on that queue, and a single global count then - * re-enables the walk and the syscall for every wake on every unrelated - * address, which is the steady state of any threaded guest. - * - * Ordering, and why a wake cannot be lost. The waiter increments before - * loading the futex word, and both that increment and the waker's read are - * seq_cst, so a waker reading zero is ordered before the increment and - * hence before the waiter's load of the word, which therefore sees the - * waker's store and returns EAGAIN rather than blocking. Waiter and waker - * reach the same counter because both index it with futex_hash(uaddr). Two - * addresses colliding in one bucket read as occupied, which costs one - * pointless syscall, the behavior this replaces. - * - * What orders the guest's store to the word against the waker's read is - * that every call site reaches futex_wake_topup_osync just after a - * pthread_mutex_unlock, whose release store supplies the edge, and that the - * guest's own unlock is an STLR on the same hardware thread as the vCPU. - * Moving the read above that unlock would break it. - * - * Not a licence to drop the bucket mutex on an empty wake. This counts the - * Darwin queue only; FUTEX_WAIT_BITSET, PI and futex_waitv still live in - * head, and an unlocked pre-check of head is unsound anyway: the waiter is - * load-then-store (read the word under the lock, then publish) against the - * waker's store-then-load, which is not the store-buffer shape and so is - * permitted to miss. - * - * Keyed on the guest VA, like the chain beside it, which costs nothing - * observable here. The reflex worry is a futex reached through two - * mappings: keyed per VA the wake consults one bucket and misses a waiter - * parked under the other. It does not arise. GUEST_IPA_BASE is 0, so a GVA - * is its own GPA and two mappings of one file get two GPAs, hence two host - * addresses; the Darwin queue keys on the host address, so such a wake - * could not have reached across the aliases before this change either. The - * one genuine GVA alias onto a single GPA is the Rosetta kbuf TTBR0/TTBR1 - * pair, which holds no futexes. Every other futex path here is already - * GVA-keyed (the chain walk compares w->uaddr exactly), so this keeps the - * file uniform. Linux does match across aliases, keying a shared futex on - * (inode, offset); that gap is older and wider than this counter. - */ - _Atomic uint32_t os_sync_waiters; } __attribute__((aligned(128))) futex_bucket_t; static futex_bucket_t buckets[FUTEX_BUCKETS]; @@ -519,30 +435,25 @@ static bool futex_word_load(const uint32_t *word, uint32_t *out) * into a wait nothing will ever satisfy. * * Returns 0 when the caller should block, -LINUX_EAGAIN when the word moved, - * -LINUX_EFAULT when uaddr does not resolve or the load faults. word_out, when - * non-NULL, receives the resolved host pointer; only futex_os_sync_wait needs - * it, to hand to the kernel address-wait. + * -LINUX_EFAULT when uaddr does not resolve or the load faults. * - * Four callers share this: futex_os_sync_wait, futex_wait, futex_requeue's - * CMP_REQUEUE, and futex_waitv. What differs between them is which bucket locks - * are held on an error return, so each keeps its own unlock ladder rather than - * this taking one. + * Three callers share this: futex_wait, futex_requeue's CMP_REQUEUE, and + * futex_waitv. What differs between them is which bucket locks are held on an + * error return, so each keeps its own unlock ladder rather than this taking + * one. * - * futex_wait_fast in core/shim.S is a fifth implementation, in EL1 assembly, + * futex_wait_fast in core/shim.S is a fourth implementation, in EL1 assembly, * answering the -LINUX_EAGAIN case without the HVC round trip. It bails to the * host for every input this function would answer differently, so a change to * the outcomes or their order here needs the same change there. */ static int64_t futex_should_block(const guest_t *g, uint64_t uaddr, - uint32_t expected, - uint32_t **word_out) + uint32_t expected) { uint32_t *word = (uint32_t *) guest_ptr(g, uaddr); if (!word) return -LINUX_EFAULT; - if (word_out) - *word_out = word; uint32_t current; if (!futex_word_load(word, ¤t)) @@ -660,28 +571,7 @@ void futex_init(void) for (unsigned i = 0; i < FUTEX_BUCKETS; i++) { pthread_mutex_init(&buckets[i].lock, NULL); buckets[i].head = NULL; - - /* Reset with head, not left to BSS. The shim reads a bucket's published - * count without a lock and answers a wake with zero when it is zero, so - * a count that survived a reinit would leave that bucket permanently - * occupied and its wake fast path silently dead. - */ - atomic_store_explicit(&buckets[i].os_sync_waiters, 0, - memory_order_relaxed); - } -#if ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS - if (__builtin_available(macOS 14.4, *)) { - os_sync_available = true; - - /* os_sync_wait_enabled stays false, so plain FUTEX_WAIT parks on the - * bucket path with the rest. A queued signal has to end the wait with - * EINTR, and the only way to reach an address-wait park is to wake its - * address, which the waiter cannot tell from a FUTEX_WAKE: it reports - * EINTR for a wake that counted it, or 0 for a signal it could not - * claim. A bucket waiter has waiter.woken to tell the two apart. - */ } -#endif } void futex_interrupt_request(void) @@ -889,276 +779,6 @@ static bool futex_quantum_deadline(const struct timespec *deadline, return true; } -#if ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS - -/* Wake up to budget kernel-side waiters at uaddr via - * os_sync_wake_by_address_any. Loops until the cap is hit or the API stops - * finding waiters; ENOENT (no more waiters) and any other error break out. _all - * overshoots the syscall return -- the count must be exact -- so this function - * intentionally calls _any in a loop instead. - * - * budget is uint64_t to accommodate FUTEX_REQUEUE's wake + requeue sum after - * the bucket walk; the loop counter is clamped to UINT32_MAX, which still - * preserves the INT_MAX "wake all" sentinel without overflow. - */ -static uint32_t futex_os_sync_wake_n(const guest_t *g, - uint64_t uaddr, - uint64_t budget) -{ - if (!os_sync_available || !os_sync_wait_enabled || budget == 0) - return 0; - - /* Nobody is on the Darwin queue for this address, so skip the walk and the - * syscall. - */ - if (atomic_load_explicit(&buckets[futex_hash(uaddr)].os_sync_waiters, - memory_order_seq_cst) == 0) - return 0; - void *host_addr = guest_ptr(g, uaddr); - if (!host_addr) - return 0; - if (budget > UINT32_MAX) - budget = UINT32_MAX; - - uint32_t woken = 0; - while (woken < (uint32_t) budget) { - int rc = os_sync_wake_by_address_any(host_addr, 4, - OS_SYNC_WAKE_BY_ADDRESS_NONE); - if (rc < 0) - break; - woken++; - } - return woken; -} - -/* Plain FUTEX_WAIT routed through os_sync_wait_on_address_with_timeout. - * - * The pre-check is required: the kernel API silently returns rc>=0 when the - * value already differs at entry, indistinguishable from a real wakeup. Linux - * returns -EAGAIN in that case, so an explicit atomic load bridges the contract - * gap. - * - * Quantum is bounded by FUTEX_OS_SYNC_POLL_CAP_NS so proc_exit_group_requested - * and futex_interrupt_consume get observed without a global wake-everyone - * broadcast channel. ETIMEDOUT, EINTR, EFAULT, and ENOMEM are all transient per - * Apple's docs; each must run the flag check before re-arming. EINVAL would - * indicate a programmer error here (size != 4/8 or bad flags), so it surfaces - * directly rather than spinning. - */ -static int64_t futex_os_sync_wait(guest_t *g, - uint64_t uaddr, - uint32_t expected, - uint64_t timeout_gva) -{ - if (!futex_uaddr_is_aligned(uaddr)) - return -LINUX_EINVAL; - - bool has_timeout = (timeout_gva != 0); - struct timespec deadline; - if (has_timeout) { - int rc = - futex_make_deadline(g, timeout_gva, /*is_absolute=*/0, &deadline); - if (rc == -1) - return -LINUX_EFAULT; - if (rc == -2) - return -LINUX_EINVAL; - } - - uint32_t *host_addr; - int64_t block = futex_should_block(g, uaddr, expected, &host_addr); - if (block != 0) - return block; - - /* Bound consecutive EFAULT retries. Apple documents EFAULT as transient - * (kernel copyin failure under memory pressure), so a few retries are fine; - * but a genuinely bad page would otherwise cause the loop to spin with no - * real sleep (timeout_ns is supplied to a syscall that returns immediately) - * until the user deadline finally bails out. Surface EFAULT to the guest - * after this many back-to-back failures so the host CPU does not burn for - * ~1 s. - */ - int efault_retries = 0; - - /* Once, before the wait, not inside it. The spin exists to catch a waker's - * store racing the entry to this wait, which is the handoff case. On a - * re-arm after a 100 ms quantum there is no imminent handoff, and the "word - * moved between the checks and the syscall" case is already covered - * atomically by os_sync_wait_on_address_with_timeout, which returns rc >= 0 - * when the value already differs at entry. Inside the loop this burned 4.3 - * us per parked thread per quantum for nothing: 0.28 percent of a core with - * MAX_THREADS parked. - */ - if (!has_timeout || - futex_remaining_ns(&deadline, FUTEX_OS_SYNC_POLL_CAP_NS, NULL) > 0) { - if (futex_spin_word_moved(host_addr, expected)) - return -LINUX_EAGAIN; - } - - for (;;) { - uint64_t timeout_ns; - if (has_timeout) { - timeout_ns = - futex_remaining_ns(&deadline, FUTEX_OS_SYNC_POLL_CAP_NS, NULL); - if (timeout_ns == 0) - return -LINUX_ETIMEDOUT; - } else { - timeout_ns = FUTEX_OS_SYNC_POLL_CAP_NS; - } - - int rc = os_sync_wait_on_address_with_timeout( - host_addr, (uint64_t) expected, 4, OS_SYNC_WAIT_ON_ADDRESS_NONE, - OS_CLOCK_MACH_ABSOLUTE_TIME, timeout_ns); - if (rc >= 0) { - /* Compare-after-block re-check. Darwin folds two distinct Linux - * outcomes into a single rc>=0: a genuine wake, and the racy "value - * moved off expected between the pre-check and the in-kernel - * compare" case that Linux reports as -EAGAIN. Reload the word: - * value still == expected means a real (or spurious) wake -> return - * 0; value != expected means it moved, which is -EAGAIN under Linux - * for the pre-block race and is equally safe for the post-wake - * case, since a correct futex caller must re-read the word and - * re-test its condition on either return. This is not a perfect - * oracle: a value that moves off expected and back before the - * reload returns 0 where Linux returns -EAGAIN, a benign spurious - * wake the futex contract permits. No wake is lost: the kernel's - * atomic compare-and-block already guarantees a waiter enqueued at - * expected cannot miss an os_sync_wake_by_address, and any value - * change carries the state the caller re-reads. - */ - uint32_t observed; - if (!futex_word_load(host_addr, &observed)) - return -LINUX_EFAULT; - return observed == expected ? 0 : -LINUX_EAGAIN; - } - - int err = errno; - if (err != ETIMEDOUT && err != EINTR && err != EFAULT && err != ENOMEM) - return -LINUX_EINVAL; - - if (err == EFAULT) { - if (++efault_retries >= 8) - return -LINUX_EFAULT; - } else { - efault_retries = 0; - } - - if (thread_stop_requested() || futex_interrupt_consume()) { - /* This path's deadline is relative (futex_make_deadline above is - * called with is_absolute = 0), so part of it is already spent and - * a restart would re-derive it from the guest's original value. - */ - if (has_timeout) - syscall_restart_forbid(); - return -LINUX_EINTR; - } - - /* Drain any expired guest itimer so its SIGALRM / SIGVTALRM / SIGPROF - * queues into sig_state.pending; without this poke, a guest with all - * threads parked in futex_wait would never advance the timers. - */ - signal_check_timer_real(); - - /* Return EINTR only when a real deliverable signal is queued for this - * thread. POSIX callers (e.g. glibc sem_wait, foot's render worker) - * often do not retry on EINTR, so synthetic spurious wakeups cannot be - * issued here. The claim confirms under sig_lock, so the atomic hint - * cannot produce a stale-true edge after rt_sigprocmask masked the - * queued signal, and takes a process-directed signal for this thread so - * a sibling woken by the same one does not report EINTR for it too. - */ - if (signal_claim_interruption()) { - if (has_timeout) - syscall_restart_forbid(); - return -LINUX_EINTR; - } - - /* For has_timeout: futex_remaining_ns returns 0 next iteration once the - * user deadline elapses, so the loop exits with -ETIMEDOUT. - */ - } -} - -/* Hold this address's bucket census up for the whole wait, from before - * futex_os_sync_wait's first load of the futex word to after its last. That - * function returns from a dozen places; counting out here rather than in it is - * what makes "every exit decrements" a property of the code rather than a rule - * each new return has to remember. futex_os_sync_wait keeps its own name so - * scripts/check-eintr-contract.py still records its EINTR returns against it. - */ -static int64_t futex_os_sync_wait_counted(guest_t *g, - uint64_t uaddr, - uint32_t expected, - uint64_t timeout_gva) -{ - unsigned idx = futex_hash(uaddr); - futex_bucket_t *b = &buckets[idx]; - - /* Both counts go up before the wait reads the futex word, and the shim's - * wake path depends on that order: see shim_globals_futex_waiters_add. - * os_sync_waiters stays separate because it answers a narrower question, - * whether the Darwin queue specifically needs draining. - * - * The published count strictly encloses os_sync_waiters, rather than the - * two nesting the other way round. futex_wake reads this bucket's census as - * the chain plus os_sync_waiters and holds the published count to be no - * lower, which is the invariant its contract assert checks and the one the - * shim reads a zero against. Charging os_sync_waiters first, or dropping - * the published count first, opens an instant on either side where the - * census counts a waiter the published count does not: a contended plain - * FUTEX_WAIT workload trips that assert within a second. - */ - shim_globals_futex_waiters_add(g, idx, +1); - atomic_fetch_add_explicit(&b->os_sync_waiters, 1, memory_order_seq_cst); - int64_t rc = futex_os_sync_wait(g, uaddr, expected, timeout_gva); - atomic_fetch_sub_explicit(&b->os_sync_waiters, 1, memory_order_seq_cst); - shim_globals_futex_waiters_add(g, idx, -1); - return rc; -} - -#else /* !ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS */ - -/* Stub fallback: dead branch on builds whose SDK lacks the header. The dispatch - * sites guard on os_sync_available, so this stub is unreachable at runtime, but - * it keeps the link clean. - */ -static uint32_t futex_os_sync_wake_n(const guest_t *g, - uint64_t uaddr, - uint64_t budget) -{ - (void) g; - (void) uaddr; - (void) budget; - return 0; -} - -#endif /* ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS */ - -/* Top up a wake after the bucket walk. A plain FUTEX_WAIT enqueues on the - * kernel os_sync queue, not the hash bucket, so any site that wakes by walking - * the bucket must also drain the os_sync queue at the same address or it - * strands those waiters. woken is how many of target the bucket walk already - * satisfied at uaddr; this drains the shortfall and returns the new total. - * futex_os_sync_wake_n is a no-op when the address-wait path is disabled or - * when no kernel waiter sits at uaddr, so this is safe to call unconditionally. - * Call it AFTER dropping the bucket lock: os_sync_wake is a syscall and must - * not run under the leaf lock. - * - * Every bucket-walking wake site (futex_wake, futex_requeue, futex_wake_op) - * routes through here so the "drain both queues" invariant is one named step, - * not a rule each new caller has to remember. The int64_t return absorbs - * futex_os_sync_wake_n's uint32_t count without sign-extension. - */ -static int64_t futex_wake_topup_osync(const guest_t *g, - uint64_t uaddr, - int64_t woken, - uint64_t target) -{ - if ((uint64_t) woken >= target) - return woken; - uint64_t budget = target - (uint64_t) woken; - return woken + (int64_t) futex_os_sync_wake_n(g, uaddr, budget); -} - /* FUTEX_WAIT / FUTEX_WAIT_BITSET: atomically check word == val, then sleep. */ static int64_t futex_wait_inner(unsigned *pub_bucket_out, guest_t *g, @@ -1215,7 +835,7 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, * is ordered against a concurrent wake. A mismatch returns EAGAIN and never * enqueues. */ - int64_t block = futex_should_block(g, uaddr, expected, NULL); + int64_t block = futex_should_block(g, uaddr, expected); if (block != 0) { pthread_mutex_unlock(&b->lock); return block; @@ -1371,13 +991,10 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, * waiters are unlinked from the bucket list so subsequent operations do not * count them as still-sleeping entries. * - * After walking the bucket, futex_wake_topup_osync drains any kernel-side - * waiters at the same address up to the remaining budget so a wake that walked - * only the bucket does not strand them; it is a no-op when the Darwin - * address-wait path is inactive. Plain FUTEX_WAIT enqueues with implicit - * FUTEX_BITSET_MATCH_ANY, which matches every legal FUTEX_WAKE_BITSET mask - * (mask must be non-zero by Linux contract), so those waiters remain valid wake - * targets. Publish this bucket's waiter count around the whole wait. + * Plain FUTEX_WAIT enqueues with implicit FUTEX_BITSET_MATCH_ANY, which matches + * every legal FUTEX_WAKE_BITSET mask (mask must be non-zero by Linux contract), + * so those waiters remain valid wake targets. Publish this bucket's waiter + * count around the whole wait. * * The increment has to land before the wait reads the futex word, which it does * here because every read futex_wait_inner makes is inside the call. A wrapper @@ -1459,13 +1076,9 @@ static int64_t futex_wake(const guest_t *g, /* Re-read before believing a violation, because a single reading of * this cannot tell one from a transient. * - * The chain is stable here: this holds the bucket lock. os_sync_waiters - * is not, by design, since the Darwin address-wait path takes no lock. - * So the three loads are not one instant, and either order is wrong in - * one direction. Read the census last and a waiter that finishes in - * between drops it under a population already counted; read it first - * and a waiter that arrives in between raises the population above a - * census already read. Neither is an understatement. + * The chain is stable here, under the bucket lock, but the published + * count is not: a waiter charges it before it takes this lock and drops + * the charge after releasing it. * * A real one is a charge that is missing rather than in flight, so it * does not go away when looked at again. Requiring the violation to @@ -1478,16 +1091,12 @@ static int64_t futex_wake(const guest_t *g, unsigned parked = 0; for (const futex_waiter_t *q = b->head; q; q = q->next) parked++; - parked += - atomic_load_explicit(&b->os_sync_waiters, memory_order_seq_cst); if (shim_globals_futex_waiters_get(g, idx) < parked) { int persisted = 0; for (int retry = 0; retry < FUTEX_CENSUS_RECHECKS; retry++) { unsigned again = 0; for (const futex_waiter_t *q = b->head; q; q = q->next) again++; - again += atomic_load_explicit(&b->os_sync_waiters, - memory_order_seq_cst); if (shim_globals_futex_waiters_get(g, idx) < again) persisted++; } @@ -1515,7 +1124,7 @@ static int64_t futex_wake(const guest_t *g, pthread_mutex_unlock(&b->lock); - return futex_wake_topup_osync(g, uaddr, woken, val); + return woken; } /* FUTEX_REQUEUE / FUTEX_CMP_REQUEUE: wake val waiters at uaddr, then move up to @@ -1566,7 +1175,7 @@ static int64_t futex_requeue(guest_t *g, /* CMP_REQUEUE: atomically verify *uaddr == expected */ if (do_cmp) { - int64_t block = futex_should_block(g, uaddr, expected, NULL); + int64_t block = futex_should_block(g, uaddr, expected); if (block != 0) { if (idx_src != idx_dst) pthread_mutex_unlock(&b_dst->lock); @@ -1666,16 +1275,7 @@ static int64_t futex_requeue(guest_t *g, pthread_mutex_unlock(&b_dst->lock); } - /* The kernel os_sync API cannot migrate waiters between addresses, so the - * requeue portion degrades into a wake at the source uaddr: os_sync waiters - * return to userland and re-acquire what they actually need (typically the - * mutex pthread_cond_broadcast wanted to requeue onto). Both the wake and - * requeue shortfalls therefore drain as wakes at uaddr; target is the full - * wake_count + requeue_count so the returned count stays within the Linux - * contract (woken + requeued must not exceed that sum). - */ - return futex_wake_topup_osync(g, uaddr, (int64_t) woken + requeued, - (uint64_t) wake_count + requeue_count); + return (int64_t) woken + requeued; } /* FUTEX_WAKE_OP: atomically modify *uaddr2, wake val waiters at uaddr, then @@ -1854,14 +1454,7 @@ static int64_t futex_wake_op(guest_t *g, pthread_mutex_unlock(&b2->lock); } - /* Drain the os_sync queue at each address for the bucket shortfall. The - * uaddr2 wake only fires when the predicate matched the old *uaddr2. - */ - int64_t total1 = futex_wake_topup_osync(g, uaddr, woken1, val); - int64_t total2 = - cond_met ? futex_wake_topup_osync(g, uaddr2, woken2, val2) : woken2; - - return total1 + total2; + return (int64_t) woken1 + woken2; } /* PI (Priority-Inheritance) futex. @@ -2286,10 +1879,13 @@ int64_t sys_futex(guest_t *g, switch (cmd) { case FUTEX_WAIT: -#if ELFUSE_HAVE_OS_SYNC_WAIT_ON_ADDRESS - if (os_sync_available && os_sync_wait_enabled) - return futex_os_sync_wait_counted(g, uaddr, val, timeout_gva); -#endif + /* On the bucket, like FUTEX_WAIT_BITSET. Darwin's + * os_sync_wait_on_address would park it with no bucket lock, but the + * only way to reach a thread parked there is to wake its address, which + * it cannot tell from a FUTEX_WAKE. Ending the wait for a queued signal + * that way reports EINTR for a wake that counted the waiter, or 0 for a + * signal it could not claim. waiter.woken is what tells them apart. + */ return futex_wait(g, uaddr, val, timeout_gva, FUTEX_BITSET_MATCH_ANY, /*is_absolute=*/0); @@ -2312,17 +1908,6 @@ int64_t sys_futex(guest_t *g, val3); case FUTEX_WAIT_BITSET: - /* Stays on the bucket even when val3 is MATCH_ANY, which is - * semantically a plain wait and could take the address-wait backend - * instead. Measured on the bench-futex handoff with that spelling, n=45 - * paired runs: 1.03x, p=0.16. There is no win to bank, and routing it - * would cost one: a kernel address-wait waiter cannot be migrated - * between addresses, so futex_requeue degrades to a wake in place, and - * that divergence is today confined to plain FUTEX_WAIT. Widening it - * for an effect that does not survive its own error bars is the wrong - * trade. tests/bench-futex.c keeps the row (BENCH_FUTEX_HANDOFF_BITSET) - * so the next attempt starts from a measurement rather than the idea. - */ return futex_wait(g, uaddr, val, timeout_gva, val3, /*is_absolute=*/1); case FUTEX_WAKE_BITSET: @@ -2572,7 +2157,7 @@ int64_t sys_futex_waitv(guest_t *g, unsigned idx = entry_bucket[i]; futex_bucket_t *b = &buckets[idx]; - int64_t block = futex_should_block(g, uaddr, expected, NULL); + int64_t block = futex_should_block(g, uaddr, expected); if (block != 0) { result_err = block; goto unlock_early; From 9ec5806b2153f6fe7b05d70284d0de23aa9f9e09 Mon Sep 17 00:00:00 2001 From: xalestar Date: Sun, 11 Oct 2026 17:54:33 +0800 Subject: [PATCH 6/7] Rename the constant and test named for os_sync FUTEX_OS_SYNC_POLL_CAP_NS was named for the cap on one os_sync_wait_on_address sleep, and test-osync-requeue for the os_sync wait that plain FUTEX_WAIT parked in. No futex wait reaches that call any longer. The constant bounds every condvar sleep on a bucket and the spin guard in futex_wait_inner, so it becomes FUTEX_POLL_QUANTUM_NS. The test times how long FUTEX_REQUEUE takes to reach a plain FUTEX_WAIT waiter, so it becomes test-futex-requeue-latency, beside the other futex requeue tests. Its header described the Darwin address-wait queue as current behavior and states the condition the test catches instead. No behavior changes. Refs #378 --- Makefile | 8 +++---- src/runtime/futex.c | 12 +++++----- src/runtime/thread.c | 6 ++--- ...requeue.c => test-futex-requeue-latency.c} | 22 +++++++++---------- tests/test-matrix.sh | 3 ++- 5 files changed, 26 insertions(+), 25 deletions(-) rename tests/{test-osync-requeue.c => test-futex-requeue-latency.c} (82%) diff --git a/Makefile b/Makefile index 277915e0c..bd29f5dee 100644 --- a/Makefile +++ b/Makefile @@ -587,10 +587,10 @@ $(BUILD_DIR)/test-sigtimedwait: tests/test-sigtimedwait.c | $(BUILD_DIR) @echo " CROSS $< (with -lpthread)" $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread -# test-osync-requeue drives a raw FUTEX_REQUEUE against a plain-FUTEX_WAIT -# waiter (musl unlock_requeue pattern), which a requeue that only walked part of -# the waiters would strand; needs -lpthread. -$(BUILD_DIR)/test-osync-requeue: tests/test-osync-requeue.c | $(BUILD_DIR) +# test-futex-requeue-latency drives a raw FUTEX_REQUEUE against a +# plain-FUTEX_WAIT waiter (musl unlock_requeue pattern), which a requeue that +# only walked part of the waiters would strand; needs -lpthread. +$(BUILD_DIR)/test-futex-requeue-latency: tests/test-futex-requeue-latency.c | $(BUILD_DIR) @echo " CROSS $< (with -lpthread)" $(Q)$(CROSS_COMPILE)gcc $(CROSS_TEST_CFLAGS) -o $@ $< -lpthread diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 4b298f9fd..6f93835c2 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -106,7 +106,7 @@ _Static_assert(FUTEX_WAKE_BITSET == 10, * multi-threaded runtimes, but that broke POSIX sem_wait callers that do not * retry on EINTR (e.g. foot's render worker). */ -#define FUTEX_OS_SYNC_POLL_CAP_NS (100ULL * 1000 * 1000) +#define FUTEX_POLL_QUANTUM_NS (100ULL * 1000 * 1000) /* Hash table. * @@ -726,7 +726,7 @@ static uint64_t futex_remaining_ns(const struct timespec *deadline, } /* Absolute CLOCK_REALTIME wait target one bounded quantum from now: the guest - * deadline capped at FUTEX_OS_SYNC_POLL_CAP_NS. Every condvar sleep in the + * deadline capped at FUTEX_POLL_QUANTUM_NS. Every condvar sleep in the * timed-wait paths goes through this so a waiter parked on a distant guest * deadline still re-checks the process-wide teardown flags * (proc_exit_group_requested / futex_interrupt) each quantum; an uncapped sleep @@ -748,7 +748,7 @@ static bool futex_quantum_deadline(const struct timespec *deadline, { int64_t now_ns = 0; uint64_t rem_ns = - futex_remaining_ns(deadline, FUTEX_OS_SYNC_POLL_CAP_NS, &now_ns); + futex_remaining_ns(deadline, FUTEX_POLL_QUANTUM_NS, &now_ns); if (rem_ns == 0) return false; @@ -768,8 +768,8 @@ static bool futex_quantum_deadline(const struct timespec *deadline, * INT64_MAX casts to a negative add, and INT64_MAX - add then overflows. * Clamping in the unsigned domain keeps the comparison well defined. */ - uint64_t bounded = rem_ns > (uint64_t) FUTEX_OS_SYNC_POLL_CAP_NS - ? (uint64_t) FUTEX_OS_SYNC_POLL_CAP_NS + uint64_t bounded = rem_ns > (uint64_t) FUTEX_POLL_QUANTUM_NS + ? (uint64_t) FUTEX_POLL_QUANTUM_NS : rem_ns; int64_t add = (int64_t) bounded; int64_t at_ns = now_ns > INT64_MAX - add ? INT64_MAX : now_ns + add; @@ -823,7 +823,7 @@ static int64_t futex_wait_inner(unsigned *pub_bucket_out, * before it. */ if (!has_timeout || - futex_remaining_ns(&deadline, FUTEX_OS_SYNC_POLL_CAP_NS, NULL) > 0) { + futex_remaining_ns(&deadline, FUTEX_POLL_QUANTUM_NS, NULL) > 0) { const uint32_t *spin_word = (const uint32_t *) guest_ptr(g, uaddr); if (spin_word && futex_spin_word_moved(spin_word, expected)) return -LINUX_EAGAIN; diff --git a/src/runtime/thread.c b/src/runtime/thread.c index 52f7d5a15..dc9260ac2 100644 --- a/src/runtime/thread.c +++ b/src/runtime/thread.c @@ -655,9 +655,9 @@ static void thread_join_workers_mode(bool exec_mode) * re-check latency of every such state or a worker that WOULD wind down * cleanly gets abandoned on timing alone: the interruptible io wait * re-checks every 200ms (io.c wait helper) and the futex paths every 100ms - * (FUTEX_OS_SYNC_POLL_CAP_NS quanta), so 500ms covers the slowest bound - * with margin. One shared deadline keeps worst-case shutdown at the cap - * even with many stuck workers, rather than multiplying a per-worker cap by + * (FUTEX_POLL_QUANTUM_NS quanta), so 500ms covers the slowest bound with + * margin. One shared deadline keeps worst-case shutdown at the cap even + * with many stuck workers, rather than multiplying a per-worker cap by * nworkers. A worker still alive past the cap is detached and, if this pass * claimed its handle, marked join_abandoned so a later pass does not touch * it again. The vCPU is NOT destroyed here because HVF vCPUs are diff --git a/tests/test-osync-requeue.c b/tests/test-futex-requeue-latency.c similarity index 82% rename from tests/test-osync-requeue.c rename to tests/test-futex-requeue-latency.c index 0853c0674..659601472 100644 --- a/tests/test-osync-requeue.c +++ b/tests/test-futex-requeue-latency.c @@ -4,13 +4,13 @@ * musl's private condvar (pthread_cond_timedwait.c: lock()/unlock_requeue()) * parks waiters with a plain FUTEX_WAIT on a per-node "barrier" word, then its * broadcast/handoff path uses FUTEX_REQUEUE to move those parked waiters from - * the barrier word onto the mutex word. Plain FUTEX_WAIT once parked on a - * Darwin address-wait queue that FUTEX_REQUEUE could not walk, and a requeue - * that reaches nobody strands the waiter until its 100 ms polling quantum ends. + * the barrier word onto the mutex word. If a plain FUTEX_WAIT waiter is parked + * on a queue that FUTEX_REQUEUE does not walk, the requeue cannot reach it, and + * it stays stranded until its 100 ms polling quantum ends. * * This reproduces the pattern with raw syscalls (libc-agnostic) and measures - * the wake latency. A correct requeue path wakes the waiter in well under the - * poll cap; a stranded waiter only surfaces after ~100 ms. + * the wake latency. A correct requeue path wakes the waiter in well under that + * quantum; a stranded waiter only surfaces after ~100 ms. */ #include @@ -61,14 +61,14 @@ int main(void) pthread_t th; if (pthread_create(&th, NULL, waiter_fn, NULL) != 0) { printf( - "test-osync-requeue: 0 passed, 1 failed - FAIL " + "test-futex-requeue-latency: 0 passed, 1 failed - FAIL " "(pthread_create)\n"); return 1; } /* Let the waiter reach FUTEX_WAIT. Keep this well below the difference - * between the 100 ms polling quantum (FUTEX_OS_SYNC_POLL_CAP_NS) and the - * pass threshold below, so a stranded waiter surfaces near the quantum, far + * between the 100 ms polling quantum (FUTEX_POLL_QUANTUM_NS) and the pass + * threshold below, so a stranded waiter surfaces near the quantum, far * above the threshold, instead of landing on it. */ usleep(10 * 1000); @@ -89,7 +89,7 @@ int main(void) pthread_join(th, NULL); double latency = ms_since(&t0, &t_wake); - printf("test-osync-requeue: musl-style requeue wake latency\n"); + printf("test-futex-requeue-latency: musl-style requeue wake latency\n"); printf(" requeue rc=%d wake rc=%d\n", rq, wk); printf(" wake latency: %.2f ms\n", latency); @@ -102,7 +102,7 @@ int main(void) */ int reached = (rq > 0) || (wk > 0); int pass = reached && latency < 50.0; - printf("test-osync-requeue: %d passed, %d failed - %s\n", pass, !pass, - pass ? "PASS" : "FAIL"); + printf("test-futex-requeue-latency: %d passed, %d failed - %s\n", pass, + !pass, pass ? "PASS" : "FAIL"); return pass ? 0 : 1; } diff --git a/tests/test-matrix.sh b/tests/test-matrix.sh index 883aa4677..0aa6894f1 100755 --- a/tests/test-matrix.sh +++ b/tests/test-matrix.sh @@ -826,7 +826,8 @@ run_unit_tests() test_check "$runner" "test-pthread" "0 failed" "$bindir/test-pthread" test_check "$runner" "test-cntvct-thread" "0 failed" \ "$bindir/test-cntvct-thread" - test_check "$runner" "test-osync-requeue" "0 failed" "$bindir/test-osync-requeue" + test_check "$runner" "test-futex-requeue-latency" "0 failed" \ + "$bindir/test-futex-requeue-latency" test_check "$runner" "test-futex-requeue-race" "0 failed" \ "$bindir/test-futex-requeue-race" test_check "$runner" "test-simd-clone" "0 failed" "$bindir/test-simd-clone" From cba5304d81919cc541f605aa526612baae2ecc22 Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 9 Oct 2026 23:50:26 +0800 Subject: [PATCH 7/7] Send the concurrent wake bench to the host The shim answers FUTEX_WAKE at EL1 while the bucket's published waiter count is zero, so the concurrent wake row, which had no waiter, never reached futex_wake. It reported 36.6 ns per wake against 35.6 ns for the one-thread wake-nowaiter row and could not see the bucket lock it was written to measure. Each waker's word now carries one waiter parked with FUTEX_WAIT_BITSET on a bit the wakes do not ask for. The count is nonzero, so the call goes to the host, which takes the bucket lock, walks one entry and wakes nobody. The waiter sits on the waker's own word, so the bench needs neither the bucket hash nor the table size. A one-thread row on the same path replaces wake-nowaiter as the baseline, and the concurrent row is renamed wake-nomatch-concurrent. Measured over six runs: wake-nomatch 763 to 776 ns, at the HVC floor, and wake-nomatch-concurrent 300 to 319 ns per wake over 8 threads. --- tests/bench-futex.c | 121 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 105 insertions(+), 16 deletions(-) diff --git a/tests/bench-futex.c b/tests/bench-futex.c index 48377b69a..dc8a3e3b6 100644 --- a/tests/bench-futex.c +++ b/tests/bench-futex.c @@ -50,6 +50,7 @@ #include #include +#include #include #include #include @@ -364,16 +365,26 @@ static double run_mainworker_floor(unsigned long iters) /* concurrent wake scaling */ -/* futex_wake takes the hash-bucket lock even when nothing is queued in the - * chain, so threads waking unrelated futexes contend whenever their addresses - * collide. This row is that collision and nothing else: every thread wakes its - * own private word and no waiter exists anywhere, so all it measures is a lock - * acquire and an empty walk. Divide it by the single-threaded wake-nowaiter row - * to read the contention factor. A bucket table too narrow to keep those two - * close is what this catches, and nothing else in the suite does. +/* futex_wake takes the hash-bucket lock before it looks for a waiter, so + * threads waking unrelated futexes contend whenever their addresses collide. + * These rows are that collision and nothing else: every thread wakes its own + * private word and wakes nobody. + * + * The shim answers a wake at EL1 while the bucket's published waiter count is + * zero, so a wake with no waiter never reaches futex_wake. Each word therefore + * carries one parked waiter. It waits on SCALE_PARK_BIT and the wakes ask for + * SCALE_WAKE_BIT, so the host takes the lock, walks one entry and matches + * nothing. + * + * The one-thread row is the same call with nobody to contend with. Divide the + * concurrent row by it to read the contention factor. A bucket table too narrow + * to keep those two close is what this catches, and nothing else in the suite + * does. */ #define WAKE_SCALE_THREADS 8 #define WAKE_SCALE_STACK 4096 +#define SCALE_PARK_BIT 1 +#define SCALE_WAKE_BIT 2 /* One page apart. A power-of-two stride is what real allocators emit, and it is * the layout a shift-xor bucket hash aliases on, so this stride is what makes @@ -382,23 +393,92 @@ static double run_mainworker_floor(unsigned long iters) static int scale_words[WAKE_SCALE_THREADS][1024]; static int scale_stacks[WAKE_SCALE_THREADS - 1][WAKE_SCALE_STACK] __attribute__((aligned(16))); +static int scale_park_stacks[WAKE_SCALE_THREADS][WAKE_SCALE_STACK] + __attribute__((aligned(16))); static unsigned long scale_iters; static int scale_go; static int scale_done; +static int scale_parking; +static int scale_unparked; + +static long scale_wake(int id) +{ + return raw_syscall6(__NR_futex, (long) &scale_words[id][0], + FUTEX_WAKE_BITSET | FUTEX_PRIVATE_FLAG, 1, 0, 0, + SCALE_WAKE_BIT); +} + +static void scale_parker(int id) +{ + int *w = &scale_words[id][0]; + + __atomic_add_fetch(&scale_parking, 1, __ATOMIC_RELEASE); + while (__atomic_load_n(w, __ATOMIC_ACQUIRE) == 0) + raw_syscall6(__NR_futex, (long) w, + FUTEX_WAIT_BITSET | FUTEX_PRIVATE_FLAG, 0, 0, 0, + SCALE_PARK_BIT); + __atomic_add_fetch(&scale_unparked, 1, __ATOMIC_RELEASE); + raw_futex_wake(&scale_unparked, 1); + raw_exit(0); +} + +/* Release @parked waiters and wait for each to leave its stack. */ +static void scale_unpark(int parked) +{ + for (int i = 0; i < parked; i++) { + __atomic_store_n(&scale_words[i][0], 1, __ATOMIC_RELEASE); + raw_futex_wake(&scale_words[i][0], 1); + } + for (;;) { + int seen = __atomic_load_n(&scale_unparked, __ATOMIC_ACQUIRE); + if (seen >= parked) + break; + raw_futex_wait(&scale_unparked, seen); + } +} + +/* Park one waiter on every word. False on a failed spawn. */ +static bool scale_park(void) +{ + int parked = 0; + + for (int i = 0; i < WAKE_SCALE_THREADS; i++) { + void *top = + (char *) scale_park_stacks[i] + sizeof(scale_park_stacks[i]); + long rc = raw_clone(HANDOFF_CLONE_FLAGS, top, NULL, 0, NULL); + if (rc == 0) + scale_parker(i); + if (rc < 0) + break; + parked++; + } + if (parked != WAKE_SCALE_THREADS) { + scale_unpark(parked); + return false; + } + + /* A waiter counts itself just before its wait, and nothing reports the park + * itself, so give the last one time to reach the bucket. + */ + while (__atomic_load_n(&scale_parking, __ATOMIC_ACQUIRE) < parked) + ; + usleep(10 * 1000); + return true; +} static void scale_worker(int id) { while (__atomic_load_n(&scale_go, __ATOMIC_ACQUIRE) == 0) ; for (unsigned long i = 0; i < scale_iters; i++) - raw_futex_wake(&scale_words[id][0], 1); + scale_wake(id); __atomic_add_fetch(&scale_done, 1, __ATOMIC_RELEASE); raw_futex_wake(&scale_done, 1); raw_exit(0); } -/* Returns ns per wake aggregated over every thread, or -1 on a failed spawn. */ -static double run_wake_scale(unsigned long iters) +/* Returns ns per wake aggregated over @threads, or -1 on a failed spawn. */ +static double run_wake_scale(unsigned long iters, int threads) { uint64_t start, elapsed; int spawned = 0; @@ -407,7 +487,7 @@ static double run_wake_scale(unsigned long iters) scale_go = 0; scale_done = 0; - for (int i = 1; i < WAKE_SCALE_THREADS; i++) { + for (int i = 1; i < threads; i++) { void *top = (char *) scale_stacks[i - 1] + sizeof(scale_stacks[i - 1]); long rc = raw_clone(HANDOFF_CLONE_FLAGS, top, NULL, 0, NULL); if (rc == 0) @@ -416,7 +496,7 @@ static double run_wake_scale(unsigned long iters) break; spawned++; } - if (spawned != WAKE_SCALE_THREADS - 1) { + if (spawned != threads - 1) { /* Release the workers that did start, or they spin on scale_go for ever * and the process never exits. They finish their loop against an * unmeasured word; the caller discards the run either way. @@ -434,7 +514,7 @@ static double run_wake_scale(unsigned long iters) start = monotonic_ns(); __atomic_store_n(&scale_go, 1, __ATOMIC_RELEASE); for (unsigned long i = 0; i < iters; i++) - raw_futex_wake(&scale_words[0][0], 1); + scale_wake(0); for (;;) { int seen = __atomic_load_n(&scale_done, __ATOMIC_ACQUIRE); if (seen >= spawned) @@ -443,7 +523,7 @@ static double run_wake_scale(unsigned long iters) } elapsed = monotonic_ns() - start; - return (double) elapsed / (double) (iters * WAKE_SCALE_THREADS); + return (double) elapsed / (double) (iters * (unsigned long) threads); } /* harness */ @@ -564,14 +644,23 @@ int main(int argc, char **argv) printf("\n[concurrent wake scaling]\n"); { unsigned long n = iters < 60000 ? iters : 60000; - double per = run_wake_scale(n); + double one, per; + if (!scale_park()) { + fprintf(stderr, " wake-scale: clone failed\n"); + return 1; + } + one = run_wake_scale(n, 1); + per = run_wake_scale(n, WAKE_SCALE_THREADS); + scale_unpark(WAKE_SCALE_THREADS); if (per < 0.0) { fprintf(stderr, " wake-scale: clone failed\n"); return 1; } + printf(" %-28s %9.1f ns/wake %6.2fx floor (1 thread)\n", + "wake-nomatch", one, one / floor_ns); printf(" %-28s %9.1f ns/wake %6.2fx floor (%d threads)\n", - "wake-nowaiter-concurrent", per, per / floor_ns, + "wake-nomatch-concurrent", per, per / floor_ns, WAKE_SCALE_THREADS); }