From 80c34c2de3d970eb82049798357743cfa71e8b54 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 22 Jun 2022 16:28:53 -0400 Subject: [PATCH] Fix: workqueue: futex wait: handle spurious futex wakeups MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== The workqueue thread futex_wait() returns with a workqueue->futex state of -1, which is unexpected. In this situation, the workqueue thread is observed to use 99% of CPU as workqueue->futex values are decremented to very low negative values while the workqueue is empty. This issue will cause spurious unexpected high CPU use, but will not lead to data corruption. Cause ===== From futex(5): FUTEX_WAIT Returns 0 if the caller was woken up. Note that a wake-up can also be caused by common futex usage patterns in unrelated code that happened to have previously used the futex word's memory location (e.g., typical futex-based implementations of Pthreads mutexes can cause this under some conditions). Therefore, call‐ ers should always conservatively assume that a return value of 0 can mean a spurious wake-up, and use the futex word's value (i.e., the user-space synchronization scheme) to decide whether to continue to block or not. Solution ======== We therefore need to validate whether the value differs from -1 in user-space after the call to FUTEX_WAIT returns 0. Known drawbacks =============== None. Signed-off-by: Mathieu Desnoyers Change-Id: Id024e7d3b2dab75d30fc01280fd27e5f2d8af0d1 --- src/workqueue.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/workqueue.c b/src/workqueue.c index e5931b5..c9f03b5 100644 --- a/src/workqueue.c +++ b/src/workqueue.c @@ -129,18 +129,29 @@ static int set_thread_cpu_affinity(struct urcu_workqueue *workqueue __attribute_ static void futex_wait(int32_t *futex) { + int ret; + /* Read condition before read futex */ cmm_smp_mb(); - if (uatomic_read(futex) != -1) - return; - while (futex_async(futex, FUTEX_WAIT, -1, NULL, NULL, 0)) { + while (uatomic_read(futex) == -1) { + if (!futex_async(futex, FUTEX_WAIT, -1, NULL, NULL, 0)) { + /* + * Prior queued wakeups queued by unrelated code + * using the same address can cause futex wait to + * return 0 even through the futex value is still + * -1 (spurious wakeups). Check the value again + * in user-space to validate whether it really + * differs from -1. + */ + continue; + } switch (errno) { - case EWOULDBLOCK: + case EAGAIN: /* Value already changed. */ return; case EINTR: /* Retry if interrupted by signal. */ - break; /* Get out of switch. */ + break; /* Get out of switch. Check again. */ default: /* Unexpected error. */ urcu_die(errno); -- 2.34.1