Patchwork [Precise,SRU,v2] Call xen_poll_irq with interrupts disabled

login
register
mail settings
Submitter Stefan Bader
Date Feb. 5, 2013, 5:17 p.m.
Message ID <51113EAD.4040507@canonical.com>
Download mbox | patch
Permalink /patch/218304/
State New
Headers show

Comments

Stefan Bader - Feb. 5, 2013, 5:17 p.m.
Ok this version also prevents the hang.
Tim Gardner - Feb. 5, 2013, 5:22 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512


- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBCgAGBQJRET/cAAoJED12yEX6FEfKMAIP/0RmadjHT32Bte1ddektKkDO
iCEuI5Cpdvry8HVGYSUpOcNrP1yRRE/msvBSFDkvHTCeXgwtseOB2BP1HPfVP6LJ
1WEJqBy9I9pDNBH6UEm08rMGaaT5Ce14w+qqi3A5LnLANFq0m1JkC0Il4RRfw5DI
4dkuOk6ni13+R8FC8p5qWrJ2LK89s3sg/NLFdpZUUL2fYz/j6l0qacx0F7C1uLcZ
d2/QqhODDpB/YK8BpESa0sEd+W6H6jsqkdYWNNrVeHiIRpkrPxxb3Rq34xBekjkD
hQZ0QULe8RrrVQ7BwGHR5dkpq3cb+8IYgomt/Tb5HH3ar410eHmk/ApdG3yjK37G
+TQL+F4E2CdzmkSAqimSTTtGe+qIRcnSp6ZfAirDZV3Pl0m/pJFr2hV2FoCYY667
HBH2qPsEem4B0mDa6/M4NnFvqjiefDCmxEccLi/L++BBGUWyqv0cNXF5BTN1WpXn
1wxf73p1jAyHDFNlHvRheu1zz46268X8g65pWEVrZbTVES6TRmRcNbG7gTDNCwMi
/bjI8Lg6Ptn1C0KhS/bLVzsn8ZOa/x02Hm37YGuDJkDIHD92LWqQ6VRxsMqbvVuA
H7R9Hn097plNRF0Mk9UGYdRONIjOE+nPRQ4T12gpXRbQ8uVtrU7xDvHNjfN7Mgjg
IAxQ/Mko6ZVAnuYTEJag
=QyIa
-----END PGP SIGNATURE-----
Tim Gardner - Feb. 5, 2013, 5:38 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

With an ACK from apw
- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBCgAGBQJREUOaAAoJED12yEX6FEfKeO8P/jb4Bez2kXs9yWDFg+EUy8Gj
kw2gmj5SyU9t6KD4VMFOEmzhyW6Rt2fdmXwaLd22Pw6/DTe95qwBBAtHPWItrkzf
8xDS53XmGLKAkjhVFztMulUCgR8MJbkihPgmsa/mNb//QYEWqK0ks+xQqTjYZyVX
JO/ZRzsg/YI9qz07TtNfFJwwNnxQafxT4d3lQnIW31irnhTaEIyAUnYYfZv+/meO
M8iL70GGPY46/GfpG9TVEt8xxx1HK+I3XHBRtHc7GoMbS3Gr9huQWd29sAQ27/Uv
DJXQxNdV34KSycYNzeU+F7FyW/GuF3rBCGg3n8SI7Gk+QAyu2FceB/rPF42pHVrO
invMbFpRTsZ6njzeXUbIupPSlvqqsChgtqzv5hV7sCpYZ0pRYEr4DuVu0eAd7GfR
pUYMl42gi73Ec3maihOWvwCHs/fxbL1LK/Y0E0VzBjBYdt3IqSfOzvd308sz0/q5
JdAfO5rfjFSuOk5hwlS19RVLSTCLvs8HZLO5GilHCPHonBeE8Pt5HVxtdHXfmysv
ksPIvIOP0CD94rHZCvvkNcnHJi608Pqj6zYjtSWQR3MREWuimLI4vSLYamHYVNgI
cuuogrktW/vrtij+k+yfWFL3KgLD8SvxHGlAW99+d3BMd7xPWPsDNrqM9tLZ6lKt
Xa3Dm3EUDOyMF1aSqMAS
=MjGY
-----END PGP SIGNATURE-----

Patch

From fca51db699b6254f51b1ef4cafc0751179a2dd4c Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Tue, 5 Feb 2013 15:22:37 +0100
Subject: [PATCH] UBUNTU: SAUCE: xen/pv-spinlock: Never enable interrupts in xen_spin_lock_slow()

The pv-ops spinlock code for Xen will call xen_spin_lock_slow()
if a lock cannot be obtained quickly by normal spinning. The
slow variant will put the VCPU onto a waiters list, then
enable interrupts (which are event channel messages in this
case) and calls a hypervisor function that is supposed to return
when the lock is released.

Using a test case which has a lot of threads running that cause
a high utilization of this spinlock code, it is observed that
the VCPU which is the first one on the waiters list seems to be
doing other things (no trace of the hv call on the stack) while
the waiters list still has it as the head. So other VCPUs which
try to acquire the lock are stuck in the hv call forever. And
that sooner than later end up in some circular locking.

By testing I can see this gets avoided when the interrupts remain
disabled for the duration of the poll_irq hypercall.

Xen PVM is the only affected setup because that is the only
one using the special spinlock api call which passes on the
flags as they were before (which is used to decide whether
interrupts get re-enabled in the Xen code).
KVM maps that call to the variant which will not look at the
previous state of interrupts and thus never re-enables them.

BugLink: http://bugs.launchpad.net/bugs/1011792

[v2: limited changes and reworded commit message]
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 arch/x86/xen/spinlock.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index d69cc6c..6051359 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -242,7 +242,7 @@  static noinline int xen_spin_lock_slow(struct arch_spinlock *lock, bool irq_enab
 		flags = arch_local_save_flags();
 		if (irq_enable) {
 			ADD_STATS(taken_slow_irqenable, 1);
-			raw_local_irq_enable();
+			/* raw_local_irq_enable(); */
 		}
 
 		/*
@@ -256,7 +256,7 @@  static noinline int xen_spin_lock_slow(struct arch_spinlock *lock, bool irq_enab
 		 */
 		xen_poll_irq(irq);
 
-		raw_local_irq_restore(flags);
+		/* raw_local_irq_restore(flags); */
 
 		ADD_STATS(taken_slow_spurious, !xen_test_irq_pending(irq));
 	} while (!xen_test_irq_pending(irq)); /* check for spurious wakeups */
-- 
1.7.9.5