Re: [PATCH 3/4, v2] x86: enlightenment for ticket spin locks - eliminate NOPs introduced by first patch
From: Jan Beulich
Date: Wed Jun 30 2010 - 03:06:58 EST
>>> On 30.06.10 at 03:13, "H. Peter Anvin" <hpa@xxxxxxxxx> wrote:
> On 06/29/2010 07:33 AM, Jan Beulich wrote:
>> Under the assumption that the nop-s added by the base ticket spinlock
>> enlightenment patch might be considered undesirable (or worse), here
>> is an optional patch to eliminate these nop-s again. This is done
>> through extending the memory operands of the inc instructions used for
>> unlocking ticket locks to the necessary size, using assembler and
>> linker features.
>> --- 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
>> @@ -10,7 +10,6 @@
>> #ifdef CONFIG_ENLIGHTEN_SPINLOCKS
>> #include <asm/alternative.h>
>> -#include <asm/nops.h>
>> /* Including asm/smp.h here causes a cyclic include dependency. */
>> #include <asm/percpu.h>
>> DECLARE_PER_CPU(int, cpu_number);
>> @@ -156,8 +155,7 @@ static __always_inline void __ticket_spi
>> unsigned int token;
>> - alternative_io(UNLOCK_LOCK_PREFIX "incb %[lock]\n\t"
>> - ASM_NOP3,
>> + alternative_io(UNLOCK_LOCK_PREFIX "unary incb %[lock]\n\t",
>> UNLOCK_LOCK_PREFIX "incb %[lock]\n\t"
>> "movzwl %[lock], %[token]\n\t"
>> @@ -228,8 +226,7 @@ static __always_inline void __ticket_spi
>> unsigned int token, tmp;
>> - alternative_io(UNLOCK_LOCK_PREFIX "incw %[lock]\n\t"
>> - ASM_NOP2,
>> + alternative_io(UNLOCK_LOCK_PREFIX "unary incw %[lock]\n\t",
>> UNLOCK_LOCK_PREFIX "incw %[lock]\n\t"
>> "movl %[lock], %[token]\n\t"
> If you're stretching (bloating) them anyway, perhaps we should be using
> "add" instructions instead, with their better EFLAGS behavior?
Hmm, yes, that possibility I didn't even consider. Would have
the potential to get away without that admittedly ugly "unary"
assembler macro altogether, though at the price of growing all
instructions rather than just those that have a non-symbolic
and small displacement. Since unlock generally gets inlined, I'm
not certain this additional growth in code size would be
Please let me know, though before submitting an eventual third
version I'd appreciate knowing especially the first two patches
need further changes in order to get accepted.
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/