All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Darren Hart <darren@dvhart.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Bhuvanesh_Surachari@mentor.com, Andy Lowe <Andy_Lowe@mentor.com>
Subject: [patch 3/5] futex: Document pi_state refcounting in requeue code
Date: Sat, 19 Dec 2015 20:07:39 -0000	[thread overview]
Message-ID: <20151219200607.335938312@linutronix.de> (raw)
In-Reply-To: 20151219200501.563704646@linutronix.de

[-- Attachment #1: futex--Document-pi_state-refcounting-in-requeue-code --]
[-- Type: text/plain, Size: 2859 bytes --]

Documentation of the pi_state refcounting in the requeue code is non
existent. Add it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/futex.c |   36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1709,6 +1709,8 @@ static int futex_requeue(u32 __user *uad
 		 * exist yet, look it up one more time to ensure we have a
 		 * reference to it. If the lock was taken, ret contains the
 		 * vpid of the top waiter task.
+		 * If the lock was not taken, we have pi_state and an initial
+		 * refcount on it. In case of an error we have nothing.
 		 */
 		if (ret > 0) {
 			WARN_ON(pi_state);
@@ -1724,12 +1726,16 @@ static int futex_requeue(u32 __user *uad
 			 * it. So we rather use the known value than
 			 * rereading and handing potential crap to
 			 * lookup_pi_state.
+			 *
+			 * If that call succeeds then we have pi_state
+			 * and an initial refcount on it.
 			 */
 			ret = lookup_pi_state(ret, hb2, &key2, &pi_state);
 		}
 
 		switch (ret) {
 		case 0:
+			/* We hold a reference on the pi state. */
 			break;
 		case -EFAULT:
 			put_pi_state(pi_state);
@@ -1804,19 +1810,38 @@ static int futex_requeue(u32 __user *uad
 		 * of requeue_pi if we couldn't acquire the lock atomically.
 		 */
 		if (requeue_pi) {
-			/* Prepare the waiter to take the rt_mutex. */
+			/*
+			 * Prepare the waiter to take the rt_mutex. Take a
+			 * refcount on the pi_state and store the pointer in
+			 * the futex_q object of the waiter.
+			 */
 			atomic_inc(&pi_state->refcount);
 			this->pi_state = pi_state;
 			ret = rt_mutex_start_proxy_lock(&pi_state->pi_mutex,
 							this->rt_waiter,
 							this->task);
 			if (ret == 1) {
-				/* We got the lock. */
+				/*
+				 * We got the lock. We do neither drop
+				 * the refcount on pi_state nor clear
+				 * this->pi_state because the waiter
+				 * needs the pi_state for cleaning up
+				 * the user space value. It will drop
+				 * the refcount after doing so.
+				 */
 				requeue_pi_wake_futex(this, &key2, hb2);
 				drop_count++;
 				continue;
 			} else if (ret) {
-				/* -EDEADLK */
+				/*
+				 * rt_mutex_start_proxy_lock()
+				 * detected a potential deadlock when
+				 * we tried to queue that waiter. Drop
+				 * the pi_state reference which we
+				 * took above and remove the pointer
+				 * to the state from the waiters
+				 * futex_q object.
+				 */
 				this->pi_state = NULL;
 				put_pi_state(pi_state);
 				goto out_unlock;
@@ -1827,6 +1852,11 @@ static int futex_requeue(u32 __user *uad
 	}
 
 out_unlock:
+	/*
+	 * We took an extra initial reference to the pi_state either
+	 * in futex_proxy_trylock_atomic() or in lookup_pi_state(). We
+	 * need to drop it here again.
+	 */
 	put_pi_state(pi_state);
 	double_unlock_hb(hb1, hb2);
 	wake_up_q(&wake_q);



  parent reply	other threads:[~2015-12-19 20:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-19 20:07 [patch 0/5] futex: Plug a pi_state leak and clarify the refcounting Thomas Gleixner
2015-12-19 20:07 ` [patch 1/5] futex: Drop refcount if requeue_pi() acquired the rtmutex Thomas Gleixner
2015-12-20 13:18   ` [tip:locking/core] " tip-bot for Thomas Gleixner
2015-12-19 20:07 ` [patch 2/5] futex: Rename free_pi_state() to put_pi_state() Thomas Gleixner
2015-12-20 13:19   ` [tip:locking/core] futex: Rename free_pi_state() to put_pi_state( ) tip-bot for Thomas Gleixner
2015-12-19 20:07 ` Thomas Gleixner [this message]
2015-12-20  7:41   ` [patch 3/5] futex: Document pi_state refcounting in requeue code Darren Hart
2015-12-20 13:19   ` [tip:locking/core] " tip-bot for Thomas Gleixner
2015-12-19 20:07 ` [patch 4/5] futex: Remove pointless put_pi_state calls in requeue() Thomas Gleixner
2015-12-20 13:19   ` [tip:locking/core] " tip-bot for Thomas Gleixner
2015-12-19 20:07 ` [patch 5/5] futex: Cleanup the goto confusion in requeue_pi() Thomas Gleixner
2015-12-20  5:15   ` Darren Hart
2015-12-20  5:40     ` Mike Galbraith
2015-12-20  7:37       ` Darren Hart
2015-12-20  5:46     ` Darren Hart
2015-12-20 13:20   ` [tip:locking/core] " tip-bot for Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151219200607.335938312@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=Andy_Lowe@mentor.com \
    --cc=Bhuvanesh_Surachari@mentor.com \
    --cc=darren@dvhart.com \
    --cc=dave@stgolabs.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.