All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] futex: Ensure get_futex_key_refs() always implies a barrier
@ 2014-10-17 16:38 Catalin Marinas
  2014-10-18  6:54 ` Mike Galbraith
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Catalin Marinas @ 2014-10-17 16:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Matteo Franchin, Davidlohr Bueso, Linus Torvalds, Darren Hart,
	Thomas Gleixner, Peter Zijlstra, Ingo Molnar, Paul E. McKenney

Commit b0c29f79ecea (futexes: Avoid taking the hb->lock if there's
nothing to wake up) changes the futex code to avoid taking a lock when
there are no waiters. This code has been subsequently fixed in commit
11d4616bd07f (futex: revert back to the explicit waiter counting code).
Both the original commit and the fix-up rely on get_futex_key_refs() to
always imply a barrier.

However, for private futexes, none of the cases in the switch statement
of get_futex_key_refs() would be hit and the function completes without
a memory barrier as required before checking the "waiters" in
futex_wake() -> hb_waiters_pending(). The consequence is a race with a
thread waiting on a futex on another CPU, allowing the waker thread to
read "waiters == 0" while the waiter thread to have read "futex_val ==
locked" (in kernel).

Without this fix, the problem (user space deadlocks) can be seen with
Android bionic's mutex implementation on an arm64 multi-cluster system.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Matteo Franchin <Matteo.Franchin@arm.com>
Fixes: b0c29f79ecea (futexes: Avoid taking the hb->lock if there's nothing to wake up)
Cc: <stable@vger.kernel.org>
Cc: Davidlohr Bueso <davidlohr@hp.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Darren Hart <dvhart@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/futex.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/futex.c b/kernel/futex.c
index 815d7af2ffe8..f3a3a071283c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -343,6 +343,8 @@ static void get_futex_key_refs(union futex_key *key)
 	case FUT_OFF_MMSHARED:
 		futex_get_mm(key); /* implies MB (B) */
 		break;
+	default:
+		smp_mb(); /* explicit MB (B) */
 	}
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-10-26 15:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-17 16:38 [PATCH] futex: Ensure get_futex_key_refs() always implies a barrier Catalin Marinas
2014-10-18  6:54 ` Mike Galbraith
2014-10-18  7:09   ` Mike Galbraith
2014-10-18 15:28   ` Linus Torvalds
2014-10-18 16:15     ` Mike Galbraith
2014-10-18  7:33 ` Davidlohr Bueso
2014-10-18 19:58   ` Davidlohr Bueso
2014-10-18 20:50     ` Linus Torvalds
2014-10-19  2:16       ` Davidlohr Bueso
2014-10-20  9:11         ` Thomas Gleixner
2014-10-20 10:49           ` Catalin Marinas
2014-10-20 15:32             ` Linus Torvalds
2014-10-20 16:00               ` Catalin Marinas
2014-10-18 19:32 ` Darren Hart
2014-10-18 20:19   ` Davidlohr Bueso
2014-10-18 20:25     ` Darren Hart
2014-10-20 10:15     ` Catalin Marinas
2014-10-24  3:27 ` [PATCH] futex: Mention key referencing differences between shared and private futexes Davidlohr Bueso
2014-10-24  9:11   ` Catalin Marinas
2014-10-26 15:22   ` [tip:locking/urgent] " tip-bot for Davidlohr Bueso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.