* [PATCH 3/3] x86: enlightenment for ticket spinlocks - remove NOPs from unlock path
@ 2010-01-29 8:02 Jan Beulich
2010-02-01 22:54 ` H. Peter Anvin
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2010-01-29 8:02 UTC (permalink / raw)
To: mingo, tglx, hpa; +Cc: Jeremy Fitzhardinge, linux-kernel
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.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
arch/x86/Makefile | 3 +
arch/x86/include/asm/alternative-asm.h | 59 +++++++++++++++++++++++++++++++++
arch/x86/include/asm/alternative.h | 5 ++
arch/x86/include/asm/spinlock.h | 27 ++++-----------
arch/x86/kernel/symdefs.lds | 1
arch/x86/kernel/vmlinux.lds.S | 2 +
6 files changed, 78 insertions(+), 19 deletions(-)
--- 2.6.33-rc5-virt-spinlocks.orig/arch/x86/Makefile
+++ 2.6.33-rc5-virt-spinlocks/arch/x86/Makefile
@@ -87,6 +87,9 @@ ifeq ($(CONFIG_KMEMCHECK),y)
KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
endif
+KBUILD_CFLAGS += -Wa,-I$(srctree)/arch/x86/include
+LDFLAGS_MODULE += -T $(srctree)/arch/x86/kernel/symdefs.lds
+
# Stackpointer is addressed different for 32 bit and 64 bit x86
sp-$(CONFIG_X86_32) := esp
sp-$(CONFIG_X86_64) := rsp
--- 2.6.33-rc5-virt-spinlocks.orig/arch/x86/include/asm/alternative.h
+++ 2.6.33-rc5-virt-spinlocks/arch/x86/include/asm/alternative.h
@@ -6,6 +6,11 @@
#include <linux/stringify.h>
#include <asm/asm.h>
+#if !defined(__ASSEMBLY__) && !defined(__PIC__)
+#include <asm/alternative-asm.h> /* just for tracking the build dependency */
+__asm__(".include \"asm/alternative-asm.h\"");
+#endif
+
/*
* Alternative inline assembly for SMP.
*
--- 2.6.33-rc5-virt-spinlocks.orig/arch/x86/include/asm/alternative-asm.h
+++ 2.6.33-rc5-virt-spinlocks/arch/x86/include/asm/alternative-asm.h
@@ -1,3 +1,7 @@
+#if 0 /* Hide this from compiler. */
+ .if 0 # Hide assembly source stuff when assembling compiler output.
+#endif
+
#ifdef __ASSEMBLY__
#include <asm/asm.h>
@@ -16,3 +20,58 @@
#endif
#endif /* __ASSEMBLY__ */
+
+#if 0 /* Hide this from compiler. */
+ .else # Code to be used in compiler output:
+
+ .weak _$.zero
+
+ .macro unary opc arg1 arg2 arg3
+ .Lempty=2
+ .irpc c,"\arg2"
+ .Lempty=3
+ .endr
+ .irpc c,"\arg3"
+ .Lempty=0
+ .endr
+ .Lsym=1
+ .Lnum=0
+ .irpc c,"\arg1"
+ .irpc m,"(123456789-0"
+ .ifeqs "\c","\m"
+ .Lsym=0
+ .exitm
+ .endif
+ .Lnum=1
+ .endr
+ .exitm
+ .endr
+ .if .Lempty == 2
+ .if .Lsym
+ \opc \arg1
+ .elseif .Lnum
+ \opc _$.zero+\arg1
+ .else
+ \opc _$.zero\arg1
+ .endif
+ .elseif .Lempty == 3
+ .if .Lsym
+ \opc \arg1,\arg2
+ .elseif .Lnum
+ \opc _$.zero+\arg1,\arg2
+ .else
+ \opc _$.zero\arg1,\arg2
+ .endif
+ .else
+ .if .Lsym
+ \opc \arg1,\arg2,\arg3
+ .elseif .Lnum
+ \opc _$.zero+\arg1,\arg2,\arg3
+ .else
+ \opc _$.zero\arg1,\arg2,\arg3
+ .endif
+ .endif
+ .endm
+
+ .endif
+#endif
--- 2.6.33-rc5-virt-spinlocks.orig/arch/x86/include/asm/spinlock.h
+++ 2.6.33-rc5-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);
@@ -155,20 +154,15 @@ static __always_inline int __ticket_spin
static __always_inline void __ticket_spin_unlock(arch_spinlock_t *lock)
{
#ifndef CONFIG_ENLIGHTEN_SPINLOCKS
- asm volatile(
+ asm volatile(UNLOCK_LOCK_PREFIX "incb %0"
+ : "+m" (lock->slock)
+ :
#else
unsigned int token;
alternative_io(
".L%=orig:\n\t"
-#endif
- UNLOCK_LOCK_PREFIX "incb %0"
-#ifndef CONFIG_ENLIGHTEN_SPINLOCKS
- : "+m" (lock->slock)
- :
-#else
- "\n\t"
- ASM_NOP3
+ UNLOCK_LOCK_PREFIX "unary incb %0\n\t"
".L%=done:",
".L%=alt:\n\t"
/* jmp .L%=callout */
@@ -286,20 +280,15 @@ static __always_inline int __ticket_spin
static __always_inline void __ticket_spin_unlock(arch_spinlock_t *lock)
{
#ifndef CONFIG_ENLIGHTEN_SPINLOCKS
- asm volatile(
+ asm volatile(UNLOCK_LOCK_PREFIX "incw %0"
+ : "+m" (lock->slock)
+ :
#else
unsigned int token, tmp;
alternative_io(
".L%=orig:\n\t"
-#endif
- UNLOCK_LOCK_PREFIX "incw %0"
-#ifndef CONFIG_ENLIGHTEN_SPINLOCKS
- : "+m" (lock->slock)
- :
-#else
- "\n\t"
- ASM_NOP2
+ UNLOCK_LOCK_PREFIX "unary incw %0\n\t"
".L%=done:",
".L%=alt:\n\t"
/* jmp .L%=callout */
--- /dev/null
+++ 2.6.33-rc5-virt-spinlocks/arch/x86/kernel/symdefs.lds
@@ -0,0 +1 @@
+_$.zero = 0;
--- 2.6.33-rc5-virt-spinlocks.orig/arch/x86/kernel/vmlinux.lds.S
+++ 2.6.33-rc5-virt-spinlocks/arch/x86/kernel/vmlinux.lds.S
@@ -27,6 +27,8 @@
#include <asm/cache.h>
#include <asm/boot.h>
+#include "symdefs.lds"
+
#undef i386 /* in case the preprocessor is a 32bit one */
OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT, CONFIG_OUTPUT_FORMAT, CONFIG_OUTPUT_FORMAT)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/3] x86: enlightenment for ticket spinlocks - remove NOPs from unlock path
2010-01-29 8:02 [PATCH 3/3] x86: enlightenment for ticket spinlocks - remove NOPs from unlock path Jan Beulich
@ 2010-02-01 22:54 ` H. Peter Anvin
2010-02-02 8:30 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: H. Peter Anvin @ 2010-02-01 22:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: mingo, tglx, Jeremy Fitzhardinge, linux-kernel
On 01/29/2010 12:02 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.33-rc5-virt-spinlocks.orig/arch/x86/include/asm/alternative-asm.h
> +++ 2.6.33-rc5-virt-spinlocks/arch/x86/include/asm/alternative-asm.h
> @@ -1,3 +1,7 @@
> +#if 0 /* Hide this from compiler. */
> + .if 0 # Hide assembly source stuff when assembling compiler output.
> +#endif
> +
> #ifdef __ASSEMBLY__
>
> #include <asm/asm.h>
> @@ -16,3 +20,58 @@
> #endif
>
> #endif /* __ASSEMBLY__ */
> +
> +#if 0 /* Hide this from compiler. */
> + .else # Code to be used in compiler output:
> +
> + .weak _$.zero
> +
> + .macro unary opc arg1 arg2 arg3
> + .Lempty=2
> + .irpc c,"\arg2"
> + .Lempty=3
> + .endr
> + .irpc c,"\arg3"
> + .Lempty=0
> + .endr
> + .Lsym=1
> + .Lnum=0
> + .irpc c,"\arg1"
> + .irpc m,"(123456789-0"
> + .ifeqs "\c","\m"
> + .Lsym=0
> + .exitm
> + .endif
> + .Lnum=1
> + .endr
> + .exitm
> + .endr
> + .if .Lempty == 2
> + .if .Lsym
> + \opc \arg1
> + .elseif .Lnum
> + \opc _$.zero+\arg1
> + .else
> + \opc _$.zero\arg1
> + .endif
> + .elseif .Lempty == 3
> + .if .Lsym
> + \opc \arg1,\arg2
> + .elseif .Lnum
> + \opc _$.zero+\arg1,\arg2
> + .else
> + \opc _$.zero\arg1,\arg2
> + .endif
> + .else
> + .if .Lsym
> + \opc \arg1,\arg2,\arg3
> + .elseif .Lnum
> + \opc _$.zero+\arg1,\arg2,\arg3
> + .else
> + \opc _$.zero\arg1,\arg2,\arg3
> + .endif
> + .endif
> + .endm
> +
> + .endif
> +#endif
Okay, I have absolutely no idea what this macro either *does* or what
it's *supposed to do* nor if it matches... you kind of forgot to
describe that. The other bit is that this whole handling with .if and
#if is just too ugly to live. Create two include files at the very minimum.
I'd like to figure out if there isn't a better idea to do what you're
trying to do here, though...
-hpa
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/3] x86: enlightenment for ticket spinlocks - remove NOPs from unlock path
2010-02-01 22:54 ` H. Peter Anvin
@ 2010-02-02 8:30 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2010-02-02 8:30 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: mingo, Jeremy Fitzhardinge, tglx, linux-kernel
>>> "H. Peter Anvin" <hpa@zytor.com> 01.02.10 23:54 >>>
>> --- 2.6.33-rc5-virt-spinlocks.orig/arch/x86/include/asm/alternative-asm.h
>> +++ 2.6.33-rc5-virt-spinlocks/arch/x86/include/asm/alternative-asm.h
>> @@ -1,3 +1,7 @@
>> +#if 0 /* Hide this from compiler. */
>> + .if 0 # Hide assembly source stuff when assembling compiler output.
>> +#endif
>> +
>> #ifdef __ASSEMBLY__
>>
>> #include <asm/asm.h>
>> @@ -16,3 +20,58 @@
>> #endif
>>
>> #endif /* __ASSEMBLY__ */
>> +
>> +#if 0 /* Hide this from compiler. */
>> + .else # Code to be used in compiler output:
>> +
>> + .weak _$.zero
>> +
>> + .macro unary opc arg1 arg2 arg3
>...
>> + .endm
>> +
>> + .endif
>> +#endif
>
>Okay, I have absolutely no idea what this macro either *does* or what
>it's *supposed to do* nor if it matches... you kind of forgot to
>describe that.
It does what the patch description says - extend the (memory) operand
of a unary opcode (inc in the case where it's being used) to maximum
width (i.e. to include a 32-bit displacement regardless of whether one
really is needed). It just needs to consider the different cases of what
operands gcc may output. And it gets more complicated because
commas in the operand are being treated as macro operand separators.
>The other bit is that this whole handling with .if and
>#if is just too ugly to live. Create two include files at the very minimum.
The new one ought to use the same construct then anyway, as inclusion
of the file visible to gcc is desirable for dependency tracking, while
wrapping the whole construct in an __asm__() doesn't seem desirable
to me due to it making the whole thing even less readable.
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-02-02 8:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-29 8:02 [PATCH 3/3] x86: enlightenment for ticket spinlocks - remove NOPs from unlock path Jan Beulich
2010-02-01 22:54 ` H. Peter Anvin
2010-02-02 8:30 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).