linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Davidlohr Bueso <dave@stgolabs.net>
To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@kernel.org>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Jason Low <jason.low2@hp.com>,
	Michel Lespinasse <walken@google.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	linux-kernel@vger.kernel.org, Davidlohr Bueso <dave@stgolabs.net>,
	Davidlohr Bueso <dbueso@suse.de>
Subject: [PATCH 4/5] locking/rwsem: Avoid deceiving lock spinners
Date: Fri, 30 Jan 2015 01:14:26 -0800	[thread overview]
Message-ID: <1422609267-15102-5-git-send-email-dave@stgolabs.net> (raw)
In-Reply-To: <1422609267-15102-1-git-send-email-dave@stgolabs.net>

When readers hold the semaphore, the ->owner is nil. As such,
and unlike mutexes, '!owner' does not necessarily imply that
the lock is free. This will cause writers to potentially spin
excessively as they've been mislead to thinking they have a
chance of acquiring the lock, instead of blocking.

This patch therefore enhances the counter check when the owner
is not set by the time we've broken out of the loop. Otherwise
we can return true as a new owner has the lock and thus we want
to continue spinning. While at it, we can make rwsem_spin_on_owner()
less ambiguos and return right away under need_resched conditions.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 kernel/locking/rwsem-xadd.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 07713e5..1c0d11e 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -337,21 +337,30 @@ static inline bool owner_running(struct rw_semaphore *sem,
 static noinline
 bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
 {
+	long count;
+
 	rcu_read_lock();
 	while (owner_running(sem, owner)) {
-		if (need_resched())
-			break;
+		/* abort spinning when need_resched */
+		if (need_resched()) {
+			rcu_read_unlock();
+			return false;
+		}
 
 		cpu_relax_lowlatency();
 	}
 	rcu_read_unlock();
 
+	if (READ_ONCE(sem->owner))
+		return true; /* new owner, continue spinning */
+
 	/*
-	 * We break out the loop above on need_resched() or when the
-	 * owner changed, which is a sign for heavy contention. Return
-	 * success only when sem->owner is NULL.
+	 * When the owner is not set, the lock could be free or
+	 * held by readers. Check the counter to verify the
+	 * state.
 	 */
-	return sem->owner == NULL;
+	count = READ_ONCE(sem->count);
+	return (count == 0 || count == RWSEM_WAITING_BIAS);
 }
 
 static bool rwsem_optimistic_spin(struct rw_semaphore *sem)
-- 
2.1.4


  parent reply	other threads:[~2015-01-30  9:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-30  9:14 [PATCH -tip v2 0/5] rwsem: Fine tuning Davidlohr Bueso
2015-01-30  9:14 ` [PATCH 1/5] locking/rwsem: Use task->state helpers Davidlohr Bueso
2015-01-30  9:14 ` [PATCH 2/5] locking/rwsem: Document barrier need when waking tasks Davidlohr Bueso
2015-02-18 17:11   ` [tip:locking/core] " tip-bot for Davidlohr Bueso
2015-01-30  9:14 ` [PATCH 3/5] locking/rwsem: Set lock ownership ASAP Davidlohr Bueso
2015-02-18 17:11   ` [tip:locking/core] " tip-bot for Davidlohr Bueso
2015-01-30  9:14 ` Davidlohr Bueso [this message]
2015-01-31  1:51   ` [PATCH 4/5] locking/rwsem: Avoid deceiving lock spinners Tim Chen
2015-01-31  2:28     ` Davidlohr Bueso
2015-02-03 17:16       ` Tim Chen
2015-02-03 17:54         ` Jason Low
2015-02-03 19:43           ` Tim Chen
2015-02-03 21:04             ` Jason Low
2015-02-03 21:48               ` Tim Chen
2015-02-04 12:06             ` Peter Zijlstra
2015-02-04 17:39               ` Tim Chen
2015-01-31  9:29   ` Peter Zijlstra
2015-01-31 21:14     ` Davidlohr Bueso
2015-01-31 21:17       ` Davidlohr Bueso
2015-02-18 17:12   ` [tip:locking/core] " tip-bot for Davidlohr Bueso
2015-01-30  9:14 ` [PATCH 5/5] locking/rwsem: Check for active lock before bailing on spinning Davidlohr Bueso
2015-02-18 17:12   ` [tip:locking/core] " tip-bot for Davidlohr Bueso

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=1422609267-15102-5-git-send-email-dave@stgolabs.net \
    --to=dave@stgolabs.net \
    --cc=dbueso@suse.de \
    --cc=jason.low2@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tim.c.chen@linux.intel.com \
    --cc=walken@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).