From f2ebb6626f3e3a00932bf1f4f75265f826c7fba9 Mon Sep 17 00:00:00 2001 From: Stefan Bader Date: Thu, 18 Oct 2012 21:40:37 +0200 Subject: [PATCH 1/2] xen/pv-spinlock: Never enable interrupts in xen_spin_lock_slow() I am not sure what exactly the spin_lock_flags variant of the pv-spinlocks (or even in the arch spinlocks) should be used for. But it should not be used as an invitation to enable irqs. The only high-level variant that seems to end up there is the spinlock_irqsave one and that would always be used in a context that expects the interrupts to be disabled. The generic paravirt-spinlock code just maps the flags variant to the one without flags, so just do the same and get rid of all the stuff that is not needed anymore. This seems to be resolving a weird locking issue seen when having a high i/o database load on a PV Xen guest with multiple (8+ in local experiments) CPUs. Well, thinking about it a second time it seems like one of those "how did that ever work?" cases. Signed-off-by: Stefan Bader --- arch/x86/xen/spinlock.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c index 83e866d..3330a1d 100644 --- a/arch/x86/xen/spinlock.c +++ b/arch/x86/xen/spinlock.c @@ -24,7 +24,6 @@ static struct xen_spinlock_stats u32 taken_slow_nested; u32 taken_slow_pickup; u32 taken_slow_spurious; - u32 taken_slow_irqenable; u64 released; u32 released_slow; @@ -197,7 +196,7 @@ static inline void unspinning_lock(struct xen_spinlock *xl, struct xen_spinlock __this_cpu_write(lock_spinners, prev); } -static noinline int xen_spin_lock_slow(struct arch_spinlock *lock, bool irq_enable) +static noinline int xen_spin_lock_slow(struct arch_spinlock *lock) { struct xen_spinlock *xl = (struct xen_spinlock *)lock; struct xen_spinlock *prev; @@ -218,8 +217,6 @@ static noinline int xen_spin_lock_slow(struct arch_spinlock *lock, bool irq_enab ADD_STATS(taken_slow_nested, prev != NULL); do { - unsigned long flags; - /* clear pending */ xen_clear_irq_pending(irq); @@ -239,12 +236,6 @@ static noinline int xen_spin_lock_slow(struct arch_spinlock *lock, bool irq_enab goto out; } - flags = arch_local_save_flags(); - if (irq_enable) { - ADD_STATS(taken_slow_irqenable, 1); - raw_local_irq_enable(); - } - /* * Block until irq becomes pending. If we're * interrupted at this point (after the trylock but @@ -256,8 +247,6 @@ static noinline int xen_spin_lock_slow(struct arch_spinlock *lock, bool irq_enab */ xen_poll_irq(irq); - 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 */ @@ -270,7 +259,7 @@ out: return ret; } -static inline void __xen_spin_lock(struct arch_spinlock *lock, bool irq_enable) +static inline void __xen_spin_lock(struct arch_spinlock *lock) { struct xen_spinlock *xl = (struct xen_spinlock *)lock; unsigned timeout; @@ -302,19 +291,19 @@ static inline void __xen_spin_lock(struct arch_spinlock *lock, bool irq_enable) spin_time_accum_spinning(start_spin_fast); } while (unlikely(oldval != 0 && - (TIMEOUT == ~0 || !xen_spin_lock_slow(lock, irq_enable)))); + (TIMEOUT == ~0 || !xen_spin_lock_slow(lock)))); spin_time_accum_total(start_spin); } static void xen_spin_lock(struct arch_spinlock *lock) { - __xen_spin_lock(lock, false); + __xen_spin_lock(lock); } static void xen_spin_lock_flags(struct arch_spinlock *lock, unsigned long flags) { - __xen_spin_lock(lock, !raw_irqs_disabled_flags(flags)); + __xen_spin_lock(lock); } static noinline void xen_spin_unlock_slow(struct xen_spinlock *xl) @@ -424,8 +413,6 @@ static int __init xen_spinlock_debugfs(void) &spinlock_stats.taken_slow_pickup); debugfs_create_u32("taken_slow_spurious", 0444, d_spin_debug, &spinlock_stats.taken_slow_spurious); - debugfs_create_u32("taken_slow_irqenable", 0444, d_spin_debug, - &spinlock_stats.taken_slow_irqenable); debugfs_create_u64("released", 0444, d_spin_debug, &spinlock_stats.released); debugfs_create_u32("released_slow", 0444, d_spin_debug, -- 1.7.9.5