Re: [PATCH 4/4, v2] x86: enlightenment for ticket spin locks - improveyield behavior on Xen
From: Jeremy Fitzhardinge
Date: Wed Jun 30 2010 - 06:08:54 EST
On 06/29/2010 04:35 PM, Jan Beulich wrote:
> This optional patch improves yielding behavior in that the acquire
> function now checks whether the vCPU owning the lock is actually
> running, yielding immediately if it isn't.
>
> The (only) additional overhead this introduces for native execution is
> the writing of the owning CPU in the lock acquire paths. If this is
> considered a problem but the patch otherwise is deemed useful, even
> that code could be eliminated for native execution (by further
> alternative instruction patching).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> Cc: KY Srinivasan <ksrinivasan@xxxxxxxxxx>
>
> ---
> arch/x86/include/asm/spinlock.h | 13 +++++++++++++
> arch/x86/include/asm/spinlock_types.h | 5 +++++
> arch/x86/kernel/cpu/xen.c | 5 ++++-
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> --- 2.6.35-rc3-virt-spinlocks.orig/arch/x86/include/asm/spinlock.h
> +++ 2.6.35-rc3-virt-spinlocks/arch/x86/include/asm/spinlock.h
> @@ -85,6 +85,15 @@ extern void virt_spin_unlock_stub(void);
> # define UNLOCK_LOCK_PREFIX
> #endif
>
> +static __always_inline void __ticket_spin_set_owner(arch_spinlock_t *lock,
> + int owned)
> +{
> +#ifdef CONFIG_ENLIGHTEN_SPINLOCKS
> + if (owned)
> + lock->owner = percpu_read(cpu_number);
>
Why not smp_processor_id()? Is this different in some way?
> +#endif
> +}
> +
> /*
> * Ticket locks are conceptually two parts, one indicating the current head of
> * the queue, and the other indicating the current tail. The lock is acquired
> @@ -124,6 +133,7 @@ static __always_inline void __ticket_spi
> ASM_OUTPUT2("+Q" (inc), "+m" (lock->slock)),
> [stub] "i" (virt_spin_lock_stub)
> : "memory", "cc");
> + __ticket_spin_set_owner(lock, true);
> }
>
> static __always_inline int __ticket_spin_trylock(arch_spinlock_t *lock)
> @@ -141,6 +151,7 @@ static __always_inline int __ticket_spin
> : "=&a" (tmp), "=&q" (new), "+m" (lock->slock)
> :
> : "memory", "cc");
> + __ticket_spin_set_owner(lock, tmp);
>
> return tmp;
> }
> @@ -192,6 +203,7 @@ static __always_inline void __ticket_spi
> ASM_OUTPUT2("+r" (inc), "+m" (lock->slock), "=&r" (tmp)),
> [stub] "i" (virt_spin_lock_stub)
> : "memory", "cc");
> + __ticket_spin_set_owner(lock, true);
> }
>
> static __always_inline int __ticket_spin_trylock(arch_spinlock_t *lock)
> @@ -212,6 +224,7 @@ static __always_inline int __ticket_spin
> : "=&a" (tmp), "=&q" (new), "+m" (lock->slock)
> :
> : "memory", "cc");
> + __ticket_spin_set_owner(lock, tmp);
>
> return tmp;
> }
> --- 2.6.35-rc3-virt-spinlocks.orig/arch/x86/include/asm/spinlock_types.h
> +++ 2.6.35-rc3-virt-spinlocks/arch/x86/include/asm/spinlock_types.h
> @@ -17,6 +17,11 @@ typedef struct arch_spinlock {
> # else
> u16 cur, seq;
> # endif
> +# if CONFIG_NR_CPUS <= 256
> + u8 owner;
> +# else
> + u16 owner;
> +# endif
> };
> #endif
> };
> --- 2.6.35-rc3-virt-spinlocks.orig/arch/x86/kernel/cpu/xen.c
> +++ 2.6.35-rc3-virt-spinlocks/arch/x86/kernel/cpu/xen.c
> @@ -79,7 +79,8 @@ static void xen_spin_lock(struct arch_sp
>
> for (count = spin_count; ({ barrier(); lock->cur != token; }); )
> if (likely(cpu_online(raw_smp_processor_id()))
> - && unlikely(!--count)) {
> + && (per_cpu(runstate.state, lock->owner) != RUNSTATE_running
> + || unlikely(!--count))) {
> struct sched_poll sched_poll;
>
> set_xen_guest_handle(sched_poll.ports,
> @@ -91,6 +92,8 @@ static void xen_spin_lock(struct arch_sp
> } else
> cpu_relax();
>
> + lock->owner = raw_smp_processor_id();
> +
> /*
> * If we interrupted another spinlock while it was blocking, make
> * sure it doesn't block (again) without re-checking the lock.
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/