From 71693131550753be0fcd2b2d4175b53c697be5a3 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 22 Jun 2022 16:44:12 -0400 Subject: [PATCH] Fix: urcu-wait: futex wait: handle spurious futex wakeups MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== The urcu-wait urcu_adaptative_busy_wait() implements a futex wait/wakeup scheme similar to the workqueue code, which has an issue with spurious wakeups. A spurious wakeup on urcu_adaptative_busy_wait can cause urcu_adaptative_busy_wait to reach label skip_futex_wait with a wait->state state of URCU_WAIT_WAITING, which is unexpected. It would cause busy-waiting on URCU_WAIT_TEARDOWN state to start early. The wait-teardown stage is done with URCU_WAIT_ATTEMPTS active attempts, following by attempts spaced by 10ms sleeps. I do not expect that these spurious wakeups will cause user-observable effects other than being slightly less efficient that it should be. urcu-wait is used by all urcu flavor's synchronize_rcu() to implement the grace period batching scheme. 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 URCU_WAIT_WAITING in user-space after the call to FUTEX_WAIT returns 0. Known drawbacks =============== None. Signed-off-by: Mathieu Desnoyers Change-Id: I8e9586597f091efc633f3310a68d18b0bd8de1e0 --- src/urcu-wait.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/urcu-wait.h b/src/urcu-wait.h index c586ec9..ef5f7ed 100644 --- a/src/urcu-wait.h +++ b/src/urcu-wait.h @@ -154,15 +154,26 @@ void urcu_adaptative_busy_wait(struct urcu_wait_node *wait) goto skip_futex_wait; caa_cpu_relax(); } - while (futex_noasync(&wait->state, FUTEX_WAIT, URCU_WAIT_WAITING, - NULL, NULL, 0)) { + while (uatomic_read(&wait->state) == URCU_WAIT_WAITING) { + if (!futex_noasync(&wait->state, FUTEX_WAIT, URCU_WAIT_WAITING, 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 + * URCU_WAIT_WAITING (spurious wakeups). Check + * the value again in user-space to validate + * whether it really differs from + * URCU_WAIT_WAITING. + */ + continue; + } switch (errno) { - case EWOULDBLOCK: + case EAGAIN: /* Value already changed. */ goto skip_futex_wait; 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