Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1219116
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression |
| Date | 2015-09-04 17:30 +0200 |
| Message-ID | <q516c-61Z-29@gated-at.bofh.it> (permalink) |
| References | (1 earlier) <q4SPg-2xi-3@gated-at.bofh.it> <q4TrX-3vn-9@gated-at.bofh.it> <q4UHo-5cD-15@gated-at.bofh.it> <q50MO-5FD-9@gated-at.bofh.it> <q50Wu-5QK-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, Sep 04, 2015 at 05:14:27PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 04, 2015 at 08:05:16AM -0700, Linus Torvalds wrote:
> > So at the very *minimum*, that second issue should be fixed, and the
> > loop in virt_queued_spin_lock() should look something like
> >
> > do {
> > while (READ_ONCE(lock->val) != 0)
> > cpu_relax();
> > } while (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) != 0);
> >
> > which at least has a chance in hell of behaving well on the bus and in
> > a HT environment.
>
> True.
Something like so...
---
Subject: locking: Fix virt test-and-set lock implementation
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 the queued from the name, as the lock isn't queued
at all, but a simple test-and-set.
Reported-by: Dave Chinner <david@fromorbit.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.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 9d51fae1cba3..8dde3bdc4a05 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 83bfb87f5bf1..e2aadbc7151f 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 337c8818541d..87e9ce6a63c5 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;
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner <david@fromorbit.com> - 2015-09-04 07:50 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-04 08:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner <david@fromorbit.com> - 2015-09-04 09:20 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Juergen Gross <jgross@suse.com> - 2015-09-04 09:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 10:00 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner <david@fromorbit.com> - 2015-09-04 10:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-04 17:10 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 17:20 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 17:30 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner <david@fromorbit.com> - 2015-09-07 01:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Davidlohr Bueso <dave@stgolabs.net> - 2015-09-07 02:10 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-07 09:00 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-04 17:30 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 17:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 18:00 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-04 18:00 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-05 19:50 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 09:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Peter Zijlstra <peterz@infradead.org> - 2015-09-04 13:40 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner <david@fromorbit.com> - 2015-09-05 00:10 +0200
Re: [4.2, Regression] Queued spinlocks cause major XFS performance regression Dave Chinner <david@fromorbit.com> - 2015-09-07 01:50 +0200
csiph-web