95 lines
3.0 KiB
Diff
95 lines
3.0 KiB
Diff
|
From aafcd8f8692fb9e389608c1efad2e57c0bbb9362 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Linus Torvalds <torvalds@linux-foundation.org>
|
||
|
|
Date: Tue, 8 Apr 2014 15:30:07 -0700
|
||
|
|
Subject: [PATCH 08/10] futex: avoid race between requeue and wake
|
||
|
|
|
||
|
|
commit 69cd9eba38867a493a043bb13eb9b33cad5f1a9a upstream.
|
||
|
|
|
||
|
|
Jan Stancek reported:
|
||
|
|
"pthread_cond_broadcast/4-1.c testcase from openposix testsuite (LTP)
|
||
|
|
occasionally fails, because some threads fail to wake up.
|
||
|
|
|
||
|
|
Testcase creates 5 threads, which are all waiting on same condition.
|
||
|
|
Main thread then calls pthread_cond_broadcast() without holding mutex,
|
||
|
|
which calls:
|
||
|
|
|
||
|
|
futex(uaddr1, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, uaddr2, ..)
|
||
|
|
|
||
|
|
This immediately wakes up single thread A, which unlocks mutex and
|
||
|
|
tries to wake up another thread:
|
||
|
|
|
||
|
|
futex(uaddr2, FUTEX_WAKE_PRIVATE, 1)
|
||
|
|
|
||
|
|
If thread A manages to call futex_wake() before any waiters are
|
||
|
|
requeued for uaddr2, no other thread is woken up"
|
||
|
|
|
||
|
|
The ordering constraints for the hash bucket waiter counting are that
|
||
|
|
the waiter counts have to be incremented _before_ getting the spinlock
|
||
|
|
(because the spinlock acts as part of the memory barrier), but the
|
||
|
|
"requeue" operation didn't honor those rules, and nobody had even
|
||
|
|
thought about that case.
|
||
|
|
|
||
|
|
This fairly simple patch just increments the waiter count for the target
|
||
|
|
hash bucket (hb2) when requeing a futex before taking the locks. It
|
||
|
|
then decrements them again after releasing the lock - the code that
|
||
|
|
actually moves the futex(es) between hash buckets will do the additional
|
||
|
|
required waiter count housekeeping.
|
||
|
|
|
||
|
|
Reported-and-tested-by: Jan Stancek <jstancek@redhat.com>
|
||
|
|
Acked-by: Davidlohr Bueso <davidlohr@hp.com>
|
||
|
|
Cc: Peter Zijlstra <peterz@infradead.org>
|
||
|
|
Cc: Thomas Gleixner <tglx@linutronix.de>
|
||
|
|
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
||
|
|
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
||
|
|
---
|
||
|
|
kernel/futex.c | 5 +++++
|
||
|
|
1 file changed, 5 insertions(+)
|
||
|
|
|
||
|
|
diff --git a/kernel/futex.c b/kernel/futex.c
|
||
|
|
index 08ec814..16b1f2c 100644
|
||
|
|
--- a/kernel/futex.c
|
||
|
|
+++ b/kernel/futex.c
|
||
|
|
@@ -1450,6 +1450,7 @@ retry:
|
||
|
|
hb2 = hash_futex(&key2);
|
||
|
|
|
||
|
|
retry_private:
|
||
|
|
+ hb_waiters_inc(hb2);
|
||
|
|
double_lock_hb(hb1, hb2);
|
||
|
|
|
||
|
|
if (likely(cmpval != NULL)) {
|
||
|
|
@@ -1459,6 +1460,7 @@ retry_private:
|
||
|
|
|
||
|
|
if (unlikely(ret)) {
|
||
|
|
double_unlock_hb(hb1, hb2);
|
||
|
|
+ hb_waiters_dec(hb2);
|
||
|
|
|
||
|
|
ret = get_user(curval, uaddr1);
|
||
|
|
if (ret)
|
||
|
|
@@ -1508,6 +1510,7 @@ retry_private:
|
||
|
|
break;
|
||
|
|
case -EFAULT:
|
||
|
|
double_unlock_hb(hb1, hb2);
|
||
|
|
+ hb_waiters_dec(hb2);
|
||
|
|
put_futex_key(&key2);
|
||
|
|
put_futex_key(&key1);
|
||
|
|
ret = fault_in_user_writeable(uaddr2);
|
||
|
|
@@ -1517,6 +1520,7 @@ retry_private:
|
||
|
|
case -EAGAIN:
|
||
|
|
/* The owner was exiting, try again. */
|
||
|
|
double_unlock_hb(hb1, hb2);
|
||
|
|
+ hb_waiters_dec(hb2);
|
||
|
|
put_futex_key(&key2);
|
||
|
|
put_futex_key(&key1);
|
||
|
|
cond_resched();
|
||
|
|
@@ -1592,6 +1596,7 @@ retry_private:
|
||
|
|
|
||
|
|
out_unlock:
|
||
|
|
double_unlock_hb(hb1, hb2);
|
||
|
|
+ hb_waiters_dec(hb2);
|
||
|
|
|
||
|
|
/*
|
||
|
|
* drop_futex_key_refs() must be called outside the spinlocks. During
|
||
|
|
--
|
||
|
|
1.9.2
|
||
|
|
|