[PATCH 0/2] futex: updates for hb waiters

[PATCH 0/2] futex: updates for hb waiters

[Davidlohr Bueso]

Since we recently changed the way we account for pending waiters
back to the atomic ops variant, we can do some minor updates to
the code. The first is a necessary doc change, and the other a
small helper function. 

Please consider for 3.15.

Davidlohr Bueso (2):
  futex: update comments for ordering guarantees
  futex: add plist_del + atomic_dec helper

 kernel/futex.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

-- 
1.8.1.4

[PATCH 1/2] futex: update comments for ordering guarantees

[Davidlohr Bueso]

Commit 11d4616bd07f (futex: revert back to the explicit waiter counting code)
was a late fix that changed the way we account for waiters on a futex. However,
it made the general documentation we have regarding this topic slightly stale.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 kernel/futex.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 08ec814..395e285 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -119,7 +119,7 @@
  * sys_futex(WAIT, futex, val);
  *   futex_wait(futex, val);
  *
- *   waiters++;
+ *   waiters++; (a)
  *   mb(); (A) <-- paired with -.
  *                              |
  *   lock(hash_bucket(futex));  |
@@ -134,15 +134,14 @@
  *     queue();
  *     unlock(hash_bucket(futex));
  *     schedule();                         if (waiters)
- *                                           lock(hash_bucket(futex));
- *                                           wake_waiters(futex);
+ *   else                                    lock(hash_bucket(futex));
+ *     waiters--; (b)                        wake_waiters(futex);
  *                                           unlock(hash_bucket(futex));
  *
- * Where (A) orders the waiters increment and the futex value read -- this
- * is guaranteed by the head counter in the hb spinlock; and where (B)
- * orders the write to futex and the waiters read -- this is done by the
- * barriers in get_futex_key_refs(), through either ihold or atomic_inc,
- * depending on the futex type.
+ * Where (A) orders the waiters increment and the futex value read and
+ * where (B) orders the write to futex and the waiters read -- this is
+ * done by the barriers in get_futex_key_refs(), through either ihold
+ * or atomic_inc, depending on the futex type.
  *
  * This yields the following case (where X:=waiters, Y:=futex):
  *
@@ -155,6 +154,10 @@
  * Which guarantees that x==0 && y==0 is impossible; which translates back into
  * the guarantee that we cannot both miss the futex variable change and the
  * enqueue.
+ *
+ * Note that a new waiter is accounted for in (a) even when it is possible that
+ * the wait call can return error, in which case we backtrack from it in (b).
+ * Refer to the comment in queue_lock().
  */
 
 int __read_mostly futex_cmpxchg_enabled;
-- 
1.8.1.4

[PATCH 2/2] futex: add plist_del + atomic_dec helper

[Davidlohr Bueso]

When removing a waiter from the plist, we always decrement the
hb waiters to reflect such logic. Add an unqueue_waiter() helper,
thus preventing any future changes that might not do so.

For the enqueuing, this isn't worth it since we will increment the
hb waiters before doing the plist_add().

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 kernel/futex.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 395e285..b9ef521 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -291,6 +291,13 @@ static inline int hb_waiters_pending(struct futex_hash_bucket *hb)
 #endif
 }
 
+static inline void unqueue_waiter(struct futex_q *q,
+				  struct futex_hash_bucket *hb)
+{
+	plist_del(&q->list, &hb->chain);
+	hb_waiters_dec(hb);
+}
+
 /*
  * We hash on the keys returned from get_futex_key (see below).
  */
@@ -972,8 +979,7 @@ static void __unqueue_futex(struct futex_q *q)
 		return;
 
 	hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock);
-	plist_del(&q->list, &hb->chain);
-	hb_waiters_dec(hb);
+	unqueue_waiter(q, hb);
 }
 
 /*
@@ -1276,8 +1282,7 @@ void requeue_futex(struct futex_q *q, struct futex_hash_bucket *hb1,
 	 * requeue.
 	 */
 	if (likely(&hb1->chain != &hb2->chain)) {
-		plist_del(&q->list, &hb1->chain);
-		hb_waiters_dec(hb1);
+		unqueue_waiter(q, hb1);
 		plist_add(&q->list, &hb2->chain);
 		hb_waiters_inc(hb2);
 		q->lock_ptr = &hb2->lock;
@@ -2375,8 +2380,7 @@ int handle_early_requeue_pi_wakeup(struct futex_hash_bucket *hb,
 		 * We were woken prior to requeue by a timeout or a signal.
 		 * Unqueue the futex_q and determine which it was.
 		 */
-		plist_del(&q->list, &hb->chain);
-		hb_waiters_dec(hb);
+		unqueue_waiter(q, hb);
 
 		/* Handle spurious wakeups gracefully */
 		ret = -EWOULDBLOCK;
-- 
1.8.1.4