diff --git a/Makefile b/Makefile index 3eb5788ec..bd29f5dee 100644 --- a/Makefile +++ b/Makefile @@ -469,6 +469,20 @@ $(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-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: \ @@ -573,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) to guard the os_sync wake-at-source -# degradation; 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/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 5e583bda3..6f93835c2 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,15 +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; 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. - * - * 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 @@ -133,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) +#define FUTEX_POLL_QUANTUM_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 @@ -221,6 +189,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 @@ -238,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]; @@ -364,6 +290,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). @@ -378,6 +373,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 @@ -421,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)) @@ -562,41 +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; - - /* 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 = true; } -#endif } void futex_interrupt_request(void) @@ -751,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 @@ -773,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; @@ -793,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; @@ -804,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, @@ -1118,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; @@ -1130,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; @@ -1144,14 +849,28 @@ 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; + futex_park_publish(&b->lock, &waiter.cond); /* 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. Re-park + * under the lock of the bucket that holds it now. + */ + 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; + if (has_timeout) { /* Sleep in bounded quanta rather than to the guest deadline: a * worker parked here for a long guest timeout (JVM parkNanos, @@ -1165,7 +884,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; @@ -1193,11 +912,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; @@ -1225,43 +945,19 @@ 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. + /* Still under the bucket lock the park was published with; the dequeue + * below may trade it for another. */ - 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; + futex_park_withdraw(); - /* Not found: waiter was requeued again between the current hash - * computation and lock acquisition. Re-read uaddr and retry. - */ - } - } + /* 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. + */ + 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 @@ -1295,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 @@ -1383,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 @@ -1402,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++; } @@ -1439,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 @@ -1490,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); @@ -1566,6 +1251,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 { @@ -1584,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 @@ -1772,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. @@ -2204,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); @@ -2230,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: @@ -2490,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; @@ -2505,6 +2172,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/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..dc9260ac2 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) { @@ -648,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 @@ -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/bench-futex.c b/tests/bench-futex.c index a4c4b6aee..dc8a3e3b6 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 @@ -50,6 +50,7 @@ #include #include +#include #include #include #include @@ -364,17 +365,26 @@ 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 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 @@ -383,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; @@ -408,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) @@ -417,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. @@ -435,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) @@ -444,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 */ @@ -565,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); } 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-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-osync-requeue.c b/tests/test-futex-requeue-latency.c similarity index 66% rename from tests/test-osync-requeue.c rename to tests/test-futex-requeue-latency.c index 30fffe0ba..659601472 100644 --- a/tests/test-osync-requeue.c +++ b/tests/test-futex-requeue-latency.c @@ -1,18 +1,16 @@ /* - * 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. 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 @@ -63,15 +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 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_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); @@ -92,20 +89,20 @@ 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); - /* 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; - 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-futex-requeue-race.c b/tests/test-futex-requeue-race.c new file mode 100644 index 000000000..97c05ac91 --- /dev/null +++ b/tests/test-futex-requeue-race.c @@ -0,0 +1,223 @@ +/* + * 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 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); + check("FUTEX_WAIT_BITSET requeued, timeout against wake", bitset, + LEAVE_TIMEOUT); + + SUMMARY("test-futex-requeue-race"); + return 0; +} 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 b06f0d9dc..0aa6894f1 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" \ @@ -824,7 +826,10 @@ 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" test_check "$runner" "test-stress" "0 failed" "$bindir/test-stress" test_rc "$runner" "test-thread-churn" 0 "$bindir/test-thread-churn" 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-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) {} 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,