summary |
shortlog |
log |
commit | commitdiff |
tree
raw |
patch |
inline | side by side (from parent 1:
879a3e1)
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
goto skip_futex_wait;
caa_cpu_relax();
}
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;
+ }
/* Value already changed. */
goto skip_futex_wait;
case EINTR:
/* Retry if interrupted by signal. */
/* 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);
default:
/* Unexpected error. */
urcu_die(errno);