From: Jason Low <jason.low2@hp.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <peterz@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
Tim Chen <tim.c.chen@linux.intel.com>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Michel Lespinasse <walken@google.com>,
Sasha Levin <sasha.levin@oracle.com>,
LKML <linux-kernel@vger.kernel.org>,
Dave Jones <davej@codemonkey.org.uk>,
Ming Lei <ming.lei@canonical.com>, Jason Low <jason.low2@hp.com>
Subject: [PATCH] locking/rwsem: Fix lock optimistic spinning when owner is not running
Date: Fri, 06 Mar 2015 23:45:31 -0800 [thread overview]
Message-ID: <1425714331.2475.388.camel@j-VirtualBox> (raw)
Fixes tip commit b3fd4f03ca0b (locking/rwsem: Avoid deceiving lock spinners).
Ming reported soft lockups occurring when running xfstest due to
commit b3fd4f03ca0b.
When doing optimistic spinning in rwsem, threads should stop spinning when
the lock owner is not running. While a thread is spinning on owner, if
the owner reschedules, owner->on_cpu returns false and we stop spinning.
However, commit b3fd4f03ca0b essentially caused the check to get ignored
because when we break out of the spin loop due to !on_cpu, we continue
spinning if sem->owner != NULL.
This patch fixes this by making sure we stop spinning if the owner is not
running. Furthermore, just like with mutexes, refactor the code such that
we don't have separate checks for owner_running(). This makes it more
straightforward in terms of why we exit the spin on owner loop and we
would also avoid needing to "guess" why we broke out of the loop to make
this more readable.
Reported-and-tested-by: Ming Lei <ming.lei@canonical.com>
Acked-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Jason Low <jason.low2@hp.com>
---
kernel/locking/rwsem-xadd.c | 31 +++++++++++--------------------
1 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 06e2214..3417d01 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -324,32 +324,23 @@ done:
return ret;
}
-static inline bool owner_running(struct rw_semaphore *sem,
- struct task_struct *owner)
-{
- if (sem->owner != owner)
- return false;
-
- /*
- * Ensure we emit the owner->on_cpu, dereference _after_ checking
- * sem->owner still matches owner, if that fails, owner might
- * point to free()d memory, if it still matches, the rcu_read_lock()
- * ensures the memory stays valid.
- */
- barrier();
-
- return owner->on_cpu;
-}
-
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)) {
- /* abort spinning when need_resched */
- if (need_resched()) {
+ while (sem->owner == owner) {
+ /*
+ * Ensure we emit the owner->on_cpu, dereference _after_
+ * checking sem->owner still matches owner, if that fails,
+ * owner might point to free()d memory, if it still matches,
+ * the rcu_read_lock() ensures the memory stays valid.
+ */
+ barrier();
+
+ /* abort spinning when need_resched or owner is not running */
+ if (!owner->on_cpu || need_resched()) {
rcu_read_unlock();
return false;
}
--
1.7.2.5
next reply other threads:[~2015-03-07 7:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-07 7:45 Jason Low [this message]
2015-03-07 9:21 ` [PATCH] locking/rwsem: Fix lock optimistic spinning when owner is not running Peter Zijlstra
2015-03-09 17:42 ` Jason Low
2015-03-07 16:43 ` [tip:locking/core] " tip-bot for Jason Low
2015-03-07 17:13 ` Oleg Nesterov
2015-03-10 10:59 ` Peter Zijlstra
2015-03-10 16:04 ` Linus Torvalds
2015-03-10 16:16 ` Peter Zijlstra
2015-03-10 17:28 ` Oleg Nesterov
2015-03-10 17:45 ` Linus Torvalds
2015-03-10 18:33 ` Oleg Nesterov
2015-03-07 18:17 ` [PATCH] " Sasha Levin
2015-03-09 17:37 ` Jason Low
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=1425714331.2475.388.camel@j-VirtualBox \
--to=jason.low2@hp.com \
--cc=dave@stgolabs.net \
--cc=davej@codemonkey.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.lei@canonical.com \
--cc=mingo@kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=sasha.levin@oracle.com \
--cc=tim.c.chen@linux.intel.com \
--cc=torvalds@linux-foundation.org \
--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