From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Wed, 22 Jun 2022 20:44:12 +0000 (-0400)
Subject: Fix: urcu-wait: futex wait: handle spurious futex wakeups
X-Git-Tag: v0.14.0~28
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=71693131550753be0fcd2b2d4175b53c697be5a3;p=userspace-rcu.git

Fix: urcu-wait: futex wait: handle spurious futex wakeups

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 <mathieu.desnoyers@efficios.com>
Change-Id: I8e9586597f091efc633f3310a68d18b0bd8de1e0
---

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);