From: tip-bot for Peter Zijlstra <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: hpa@zytor.com, tglx@linutronix.de, torvalds@linux-foundation.org,
mingo@kernel.org, david@fromorbit.com, Waiman.Long@hp.com,
linux-kernel@vger.kernel.org, peterz@infradead.org
Subject: [tip:locking/core] locking/qspinlock/x86: Fix performance regression under unaccelerated VMs
Date: Sun, 13 Sep 2015 03:55:22 -0700 [thread overview]
Message-ID: <tip-43b3f02899f74ae9914a39547cc5492156f0027a@git.kernel.org> (raw)
In-Reply-To: <20150904152523.GR18673@twins.programming.kicks-ass.net>
Commit-ID: 43b3f02899f74ae9914a39547cc5492156f0027a
Gitweb: http://git.kernel.org/tip/43b3f02899f74ae9914a39547cc5492156f0027a
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Fri, 4 Sep 2015 17:25:23 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 11 Sep 2015 07:49:42 +0200
locking/qspinlock/x86: Fix performance regression under unaccelerated VMs
Dave ran into horrible performance on a VM without PARAVIRT_SPINLOCKS
set and Linus noted that the test-and-set implementation was retarded.
One should spin on the variable with a load, not a RMW.
While there, remove 'queued' from the name, as the lock isn't queued
at all, but a simple test-and-set.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Dave Chinner <david@fromorbit.com>
Tested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Waiman Long <Waiman.Long@hp.com>
Cc: stable@vger.kernel.org # v4.2+
Link: http://lkml.kernel.org/r/20150904152523.GR18673@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/qspinlock.h | 16 ++++++++++++----
include/asm-generic/qspinlock.h | 4 ++--
kernel/locking/qspinlock.c | 2 +-
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index 9d51fae..8dde3bd 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -39,15 +39,23 @@ static inline void queued_spin_unlock(struct qspinlock *lock)
}
#endif
-#define virt_queued_spin_lock virt_queued_spin_lock
+#define virt_spin_lock virt_spin_lock
-static inline bool virt_queued_spin_lock(struct qspinlock *lock)
+static inline bool virt_spin_lock(struct qspinlock *lock)
{
if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
return false;
- while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0)
- cpu_relax();
+ /*
+ * On hypervisors without PARAVIRT_SPINLOCKS support we fall
+ * back to a Test-and-Set spinlock, because fair locks have
+ * horrible lock 'holder' preemption issues.
+ */
+
+ do {
+ while (atomic_read(&lock->val) != 0)
+ cpu_relax();
+ } while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0);
return true;
}
diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index 83bfb87..e2aadbc 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -111,8 +111,8 @@ static inline void queued_spin_unlock_wait(struct qspinlock *lock)
cpu_relax();
}
-#ifndef virt_queued_spin_lock
-static __always_inline bool virt_queued_spin_lock(struct qspinlock *lock)
+#ifndef virt_spin_lock
+static __always_inline bool virt_spin_lock(struct qspinlock *lock)
{
return false;
}
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 337c881..87e9ce6a 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -289,7 +289,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
if (pv_enabled())
goto queue;
- if (virt_queued_spin_lock(lock))
+ if (virt_spin_lock(lock))
return;
/*
next prev parent reply other threads:[~2015-09-13 10:56 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-04 5:48 [4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner
2015-09-04 6:39 ` Linus Torvalds
2015-09-04 7:11 ` Dave Chinner
2015-09-04 7:31 ` Juergen Gross
2015-09-04 7:55 ` Peter Zijlstra
2015-09-04 8:29 ` Dave Chinner
2015-09-04 15:05 ` Linus Torvalds
2015-09-04 15:14 ` Peter Zijlstra
2015-09-04 15:21 ` Linus Torvalds
2015-09-04 15:30 ` Peter Zijlstra
2015-09-04 15:54 ` Peter Zijlstra
2015-09-10 2:06 ` Waiman Long
2015-09-04 15:58 ` Linus Torvalds
2015-09-05 17:45 ` Peter Zijlstra
2015-09-04 15:25 ` Peter Zijlstra
2015-09-06 23:32 ` Dave Chinner
2015-09-07 0:05 ` Davidlohr Bueso
2015-09-07 6:57 ` Peter Zijlstra
2015-09-07 20:45 ` Linus Torvalds
2015-09-08 6:37 ` Davidlohr Bueso
2015-09-08 10:05 ` Peter Zijlstra
2015-09-08 17:45 ` Linus Torvalds
2015-09-13 10:55 ` tip-bot for Peter Zijlstra [this message]
2015-09-04 7:39 ` Peter Zijlstra
2015-09-04 8:12 ` Dave Chinner
2015-09-04 11:32 ` Peter Zijlstra
2015-09-04 22:03 ` Dave Chinner
2015-09-06 23:47 ` Dave Chinner
2015-09-10 2:09 ` Waiman Long
[not found] ` <CAC=cRTOraeOeu3Z8C1qx6w=GMSzD_4VevrEzn0mMhrqy=7n3wQ@mail.gmail.com>
[not found] ` <56094F05.4090809@hpe.com>
2015-09-29 0:47 ` huang ying
2015-09-29 2:57 ` Waiman Long
2015-09-10 2:01 ` Waiman Long
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=tip-43b3f02899f74ae9914a39547cc5492156f0027a@git.kernel.org \
--to=tipbot@zytor.com \
--cc=Waiman.Long@hp.com \
--cc=david@fromorbit.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tip-commits@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.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 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).