[PATCH][2.6] Completely out of line spinlocks / i386

[PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

Pulled from the -tiny tree, the focus of this patch is for reduced kernel
image size but in the process we benefit from improved cache performance
since it's possible for the common text to be present in cache. This is
probably more of a win on shared cache multiprocessor systems like
P4/Xeon HT. It's been benchmarked with bonnie++ on 2x and 4x PIII (my
ideal target would be a 4x+ logical cpu Xeon).

The bonnie++ results are here, the hostnames are of the form stpN-000 with
N denoting how many processors in the system. In a nutshell there doesn't
appear to be any performance regressions.

http://www.zwane.ca/results/cool-locks-stp

   text    data     bss     dec     hex filename
5527214  873510  321872 6722596  669424 vmlinux-before
5480308  867964  321872 6670144  65c740 vmlinux-after

 arch/i386/Kconfig             |   10 +++++++
 arch/i386/kernel/i386_ksyms.c |   11 ++++++++
 arch/i386/lib/Makefile        |    1
 arch/i386/lib/spinlock.c      |   57 ++++++++++++++++++++++++++++++++++++++++++
 include/asm-i386/spinlock.h   |   22 ++++++++++++++--
 5 files changed, 99 insertions(+), 2 deletions(-)

Signed-off-by: Zwane Mwaikambo <zwane@fsmlabs.com>

Index: linux-2.6.8-rc3-mm1/arch/i386/Kconfig
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/arch/i386/Kconfig,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 Kconfig
--- linux-2.6.8-rc3-mm1/arch/i386/Kconfig	5 Aug 2004 16:37:39 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/arch/i386/Kconfig	7 Aug 2004 23:07:05 -0000
@@ -1262,6 +1262,16 @@ config DEBUG_SPINLOCK
 	  best used in conjunction with the NMI watchdog so that spinlock
 	  deadlocks are also debuggable.

+config COOL_SPINLOCK
+	bool "Completely out of line spinlocks"
+	depends on SMP
+	default y
+	help
+          Say Y here to build spinlocks which have common text for contended
+          and uncontended paths. This reduces kernel text size by at least
+          50k on most configurations, plus there is the additional benefit
+          of better cache utilisation.
+
 config DEBUG_PAGEALLOC
 	bool "Page alloc debugging"
 	depends on DEBUG_KERNEL
Index: linux-2.6.8-rc3-mm1/arch/i386/kernel/i386_ksyms.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/arch/i386/kernel/i386_ksyms.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 i386_ksyms.c
--- linux-2.6.8-rc3-mm1/arch/i386/kernel/i386_ksyms.c	5 Aug 2004 16:37:39 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/arch/i386/kernel/i386_ksyms.c	7 Aug 2004 22:51:22 -0000
@@ -51,6 +51,17 @@ extern void FASTCALL( __write_lock_faile
 extern void FASTCALL( __read_lock_failed(rwlock_t *rw));
 #endif

+#ifdef CONFIG_COOL_SPINLOCK
+extern void asmlinkage __spin_lock_failed(spinlock_t *);
+extern void asmlinkage __spin_lock_failed_flags(spinlock_t *, unsigned long);
+extern void asmlinkage __spin_lock_loop(spinlock_t *);
+extern void asmlinkage __spin_lock_loop_flags(spinlock_t *, unsigned long);
+EXPORT_SYMBOL(__spin_lock_failed);
+EXPORT_SYMBOL(__spin_lock_failed_flags);
+EXPORT_SYMBOL(__spin_lock_loop);
+EXPORT_SYMBOL(__spin_lock_loop_flags);
+#endif
+
 #if defined(CONFIG_BLK_DEV_IDE) || defined(CONFIG_BLK_DEV_HD) || defined(CONFIG_BLK_DEV_IDE_MODULE) || defined(CONFIG_BLK_DEV_HD_MODULE)
 extern struct drive_info_struct drive_info;
 EXPORT_SYMBOL(drive_info);
Index: linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 Makefile
--- linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile	5 Aug 2004 16:37:39 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile	7 Aug 2004 22:51:37 -0000
@@ -6,6 +6,7 @@
 lib-y = checksum.o delay.o usercopy.o getuser.o memcpy.o strstr.o \
 	bitops.o

+lib-$(CONFIG_COOL_SPINLOCK) += spinlock.o
 lib-$(CONFIG_X86_USE_3DNOW) += mmx.o
 lib-$(CONFIG_HAVE_DEC_LOCK) += dec_and_lock.o
 lib-$(CONFIG_KGDB) += kgdb_serial.o
Index: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
===================================================================
RCS file: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
diff -N linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c	8 Aug 2004 01:53:01 -0000
@@ -0,0 +1,57 @@
+#define PROC(name)	\
+	".align 4\n" \
+	".globl " #name"\n" \
+	#name":\n"
+
+asm (PROC(__spin_lock_failed_flags)
+	"testl $0x200, %ebx\n"
+	"jz 1f\n"
+	"sti\n"
+	"1:\n\t"
+	"rep; nop\n"
+	"cmpb $0, (%eax)\n"
+	"jle 1b\n"
+	"cli\n"
+	"lock; decb (%eax)\n\t"
+	"js __spin_lock_failed_flags\n\t"
+	"nop\n\t"
+	"ret\n"
+);
+
+asm (PROC(__spin_lock_loop_flags)
+	"lock; decb (%eax)\n\t"
+	"js 1f\n\t"
+	"nop\n\t"
+	"ret\n\t"
+	"1:\n\t"
+	"testl $0x200, %ebx\n\t"
+	"jz 1f\n\t"
+	"sti\n\t"
+	"2: rep; nop\n\t"
+	"cmpb $0, (%eax)\n\t"
+	"jle 2b\n\t"
+	"cli\n\t"
+	"jmp __spin_lock_loop_flags\n\t"
+);
+
+asm (PROC(__spin_lock_failed)
+	"rep; nop\n\t"
+	"cmpb $0, (%eax)\n\t"
+	"jle __spin_lock_failed\n\t"
+	"lock; decb (%eax)\n\t"
+	"js __spin_lock_failed\n\t"
+	"nop\n\t"
+	"ret\n\t"
+);
+
+asm (PROC(__spin_lock_loop)
+	"lock; decb (%eax)\n\t"
+	"js 1f\n\t"
+	"nop\n\t"
+	"ret\n\t"
+	"1: rep; nop\n\t"
+	"cmpb $0, (%eax)\n\t"
+	"jle 1b\n\t"
+	"jmp __spin_lock_loop\n\t"
+);
+
Index: linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 spinlock.h
--- linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h	5 Aug 2004 16:37:51 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h	7 Aug 2004 22:56:46 -0000
@@ -43,6 +43,13 @@ typedef struct {
 #define spin_is_locked(x)	(*(volatile signed char *)(&(x)->lock) <= 0)
 #define spin_unlock_wait(x)	do { barrier(); } while(spin_is_locked(x))

+#ifdef CONFIG_COOL_SPINLOCK
+	#define spin_lock_string \
+		"call __spin_lock_loop\n\t"
+
+	#define spin_lock_string_flags \
+		"call __spin_lock_loop_flags\n\t"
+#else
 #define spin_lock_string \
 	"\n1:\t" \
 	"lock ; decb %0\n\t" \
@@ -71,6 +78,7 @@ typedef struct {
 	"cli\n\t" \
 	"jmp 1b\n" \
 	LOCK_SECTION_END
+#endif

 /*
  * This works. Despite all the confusion.
@@ -139,7 +147,12 @@ here:
 #endif
 	__asm__ __volatile__(
 		spin_lock_string
-		:"=m" (lock->lock) : : "memory");
+#ifdef CONFIG_COOL_SPINLOCK
+		: : "a" (&lock->lock) : "memory"
+#else
+		:"=m" (lock->lock) : : "memory"
+#endif
+	);
 }

 static inline void _raw_spin_lock_flags (spinlock_t *lock, unsigned long flags)
@@ -154,7 +167,12 @@ here:
 #endif
 	__asm__ __volatile__(
 		spin_lock_string_flags
-		:"=m" (lock->lock) : "r" (flags) : "memory");
+#ifdef CONFIG_COOL_SPINLOCK
+		: : "a" (&lock->lock), "b" (flags) : "memory"
+#else
+		:"=m" (lock->lock) : "r" (flags) : "memory"
+#endif
+	);
 }

 /*

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Linus Torvalds]

On Sun, 8 Aug 2004, Zwane Mwaikambo wrote:
>
> Pulled from the -tiny tree,

Hmm. 

You really shouldn't use %ebx for flags. Use %edx instead. 

%ebx is call-save, so by forcing gcc to use %edx, you're guaranteeing that 
the compiler has to save/restore the register even for a simple function 
that wouldn't otherwise need it.

Also, why export the "failed" ones:

> +#ifdef CONFIG_COOL_SPINLOCK
> +extern void asmlinkage __spin_lock_failed(spinlock_t *);
> +extern void asmlinkage __spin_lock_failed_flags(spinlock_t *, unsigned long);
> +extern void asmlinkage __spin_lock_loop(spinlock_t *);
> +extern void asmlinkage __spin_lock_loop_flags(spinlock_t *, unsigned long);
> +EXPORT_SYMBOL(__spin_lock_failed);
> +EXPORT_SYMBOL(__spin_lock_failed_flags);
> +EXPORT_SYMBOL(__spin_lock_loop);
> +EXPORT_SYMBOL(__spin_lock_loop_flags);
> +#endif

that looks just broken.

		Linus

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Sat, 7 Aug 2004, Linus Torvalds wrote:

> On Sun, 8 Aug 2004, Zwane Mwaikambo wrote:
> >
> > Pulled from the -tiny tree,
>
> Hmm.
>
> You really shouldn't use %ebx for flags. Use %edx instead.
>
> %ebx is call-save, so by forcing gcc to use %edx, you're guaranteeing that
> the compiler has to save/restore the register even for a simple function
> that wouldn't otherwise need it.

Thanks i'll change that.

> Also, why export the "failed" ones:

Actually that's now dead code since i had two variants but trimmed it, the
other had the fast path acquisition code inline and called the failed
procedures on contention. It'll have to go.

> > +#ifdef CONFIG_COOL_SPINLOCK
> > +extern void asmlinkage __spin_lock_failed(spinlock_t *);
> > +extern void asmlinkage __spin_lock_failed_flags(spinlock_t *, unsigned long);
> > +extern void asmlinkage __spin_lock_loop(spinlock_t *);
> > +extern void asmlinkage __spin_lock_loop_flags(spinlock_t *, unsigned long);
> > +EXPORT_SYMBOL(__spin_lock_failed);
> > +EXPORT_SYMBOL(__spin_lock_failed_flags);
> > +EXPORT_SYMBOL(__spin_lock_loop);
> > +EXPORT_SYMBOL(__spin_lock_loop_flags);
> > +#endif
>
> that looks just broken.

_raw_spin_lock will have the symbol __spin_lock_loop{,_flags} when used in
symbols, modules won't load otherwise.

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Linus Torvalds]

On Sun, 8 Aug 2004, Zwane Mwaikambo wrote:
> >
> > that looks just broken.
> 
> _raw_spin_lock will have the symbol __spin_lock_loop{,_flags} when used in
> symbols, modules won't load otherwise.

Yes, I was talking about the "failed" things only. The non-failure-cases 
obviously do have to be exported.

		Linus

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Sat, 7 Aug 2004, Linus Torvalds wrote:

> On Sun, 8 Aug 2004, Zwane Mwaikambo wrote:
> > >
> > > that looks just broken.
> >
> > _raw_spin_lock will have the symbol __spin_lock_loop{,_flags} when used in
> > symbols, modules won't load otherwise.
>
> Yes, I was talking about the "failed" things only. The non-failure-cases
> obviously do have to be exported.

Thanks, here is a cleaned up version, we seem to have even managed to save
some text (~5k), gcc probably managed to do the asm call setup without too
much register shuffling. I still find the decrease in data odd.

   text    data     bss     dec     hex filename
5527214  873510  321872 6722596  669424 vmlinux-before
5480308  867964  321872 6670144  65c740 vmlinux-after
5474492  867930  321872 6664294  65b066 vmlinux-after2

 arch/i386/Kconfig           |   10 ++++++++++
 arch/i386/lib/Makefile      |    1 +
 arch/i386/lib/spinlock.c    |   38 ++++++++++++++++++++++++++++++++++++++
 include/asm-i386/spinlock.h |   22 ++++++++++++++++++++--
 4 files changed, 69 insertions(+), 2 deletions(-)

Index: linux-2.6.8-rc3-mm1/arch/i386/Kconfig
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/arch/i386/Kconfig,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 Kconfig
--- linux-2.6.8-rc3-mm1/arch/i386/Kconfig	5 Aug 2004 16:37:39 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/arch/i386/Kconfig	7 Aug 2004 23:07:05 -0000
@@ -1262,6 +1262,16 @@ config DEBUG_SPINLOCK
 	  best used in conjunction with the NMI watchdog so that spinlock
 	  deadlocks are also debuggable.

+config COOL_SPINLOCK
+	bool "Completely out of line spinlocks"
+	depends on SMP
+	default y
+	help
+          Say Y here to build spinlocks which have common text for contended
+          and uncontended paths. This reduces kernel text size by at least
+          50k on most configurations, plus there is the additional benefit
+          of better cache utilisation.
+
 config DEBUG_PAGEALLOC
 	bool "Page alloc debugging"
 	depends on DEBUG_KERNEL
Index: linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 Makefile
--- linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile	5 Aug 2004 16:37:39 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/arch/i386/lib/Makefile	7 Aug 2004 22:51:37 -0000
@@ -6,6 +6,7 @@
 lib-y = checksum.o delay.o usercopy.o getuser.o memcpy.o strstr.o \
 	bitops.o

+lib-$(CONFIG_COOL_SPINLOCK) += spinlock.o
 lib-$(CONFIG_X86_USE_3DNOW) += mmx.o
 lib-$(CONFIG_HAVE_DEC_LOCK) += dec_and_lock.o
 lib-$(CONFIG_KGDB) += kgdb_serial.o
Index: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
===================================================================
RCS file: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
diff -N linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c	8 Aug 2004 05:39:13 -0000
@@ -0,0 +1,38 @@
+#include <linux/module.h>
+
+#define PROC(name)	\
+	".align 4\n" \
+	".globl " #name"\n" \
+	#name":\n"
+
+asm (PROC(__spin_lock_loop_flags)
+	"lock; decb (%eax)\n\t"
+	"js 1f\n\t"
+	"nop\n\t"
+	"ret\n\t"
+	"1:\n\t"
+	"testl $0x200, %edx\n\t"
+	"jz 1f\n\t"
+	"sti\n\t"
+	"2: rep; nop\n\t"
+	"cmpb $0, (%eax)\n\t"
+	"jle 2b\n\t"
+	"cli\n\t"
+	"jmp __spin_lock_loop_flags\n\t"
+);
+
+asm (PROC(__spin_lock_loop)
+	"lock; decb (%eax)\n\t"
+	"js 1f\n\t"
+	"nop\n\t"
+	"ret\n\t"
+	"1: rep; nop\n\t"
+	"cmpb $0, (%eax)\n\t"
+	"jle 1b\n\t"
+	"jmp __spin_lock_loop\n\t"
+);
+
+void __spin_lock_loop_flags(void);
+void __spin_lock_loop(void);
+EXPORT_SYMBOL(__spin_lock_loop_flags);
+EXPORT_SYMBOL(__spin_lock_loop);
Index: linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h
===================================================================
RCS file: /home/cvsroot/linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 spinlock.h
--- linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h	5 Aug 2004 16:37:51 -0000	1.1.1.1
+++ linux-2.6.8-rc3-mm1/include/asm-i386/spinlock.h	8 Aug 2004 05:19:10 -0000
@@ -43,6 +43,13 @@ typedef struct {
 #define spin_is_locked(x)	(*(volatile signed char *)(&(x)->lock) <= 0)
 #define spin_unlock_wait(x)	do { barrier(); } while(spin_is_locked(x))

+#ifdef CONFIG_COOL_SPINLOCK
+	#define spin_lock_string \
+		"call __spin_lock_loop\n\t"
+
+	#define spin_lock_string_flags \
+		"call __spin_lock_loop_flags\n\t"
+#else
 #define spin_lock_string \
 	"\n1:\t" \
 	"lock ; decb %0\n\t" \
@@ -71,6 +78,7 @@ typedef struct {
 	"cli\n\t" \
 	"jmp 1b\n" \
 	LOCK_SECTION_END
+#endif

 /*
  * This works. Despite all the confusion.
@@ -139,7 +147,12 @@ here:
 #endif
 	__asm__ __volatile__(
 		spin_lock_string
-		:"=m" (lock->lock) : : "memory");
+#ifdef CONFIG_COOL_SPINLOCK
+		: : "a" (&lock->lock) : "memory"
+#else
+		:"=m" (lock->lock) : : "memory"
+#endif
+	);
 }

 static inline void _raw_spin_lock_flags (spinlock_t *lock, unsigned long flags)
@@ -154,7 +167,12 @@ here:
 #endif
 	__asm__ __volatile__(
 		spin_lock_string_flags
-		:"=m" (lock->lock) : "r" (flags) : "memory");
+#ifdef CONFIG_COOL_SPINLOCK
+		: : "a" (&lock->lock), "d" (flags) : "memory"
+#else
+		:"=m" (lock->lock) : "r" (flags) : "memory"
+#endif
+	);
 }

 /*

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Andreas Schwab]

Zwane Mwaikambo <zwane@linuxpower.ca> writes:

> Index: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
> ===================================================================
> RCS file: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
> diff -N linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c	8 Aug 2004 05:39:13 -0000

Why not just make this an assembler source?  It contains no real C code.
The only downside is that EXPORT_SYMBOL must be moved elsewhere, but on
the other hand it would make the assembler code more readable,

> +#define PROC(name)	\
> +	".align 4\n" \
> +	".globl " #name"\n" \
> +	#name":\n"

and you could use ENTRY from <linux/linkage.h>.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Sun, 8 Aug 2004, Andreas Schwab wrote:

> Zwane Mwaikambo <zwane@linuxpower.ca> writes:
>
> > Index: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
> > ===================================================================
> > RCS file: linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
> > diff -N linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c
> > --- /dev/null	1 Jan 1970 00:00:00 -0000
> > +++ linux-2.6.8-rc3-mm1/arch/i386/lib/spinlock.c	8 Aug 2004 05:39:13 -0000
>
> Why not just make this an assembler source?  It contains no real C code.
> The only downside is that EXPORT_SYMBOL must be moved elsewhere, but on
> the other hand it would make the assembler code more readable,

I thought about doing that, but this version touched the least amount of
files.

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Pavel Machek]

Hi!

> Pulled from the -tiny tree, the focus of this patch is for reduced kernel
> image size but in the process we benefit from improved cache performance
> since it's possible for the common text to be present in cache. This is
> probably more of a win on shared cache multiprocessor systems like
> P4/Xeon HT. It's been benchmarked with bonnie++ on 2x and 4x PIII (my
> ideal target would be a 4x+ logical cpu Xeon).
> 
> The bonnie++ results are here, the hostnames are of the form stpN-000 with
> N denoting how many processors in the system. In a nutshell there doesn't
> appear to be any performance regressions.

Fine, so perhaps we do not want config option?
									Pavel

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Linus Torvalds]

On Wed, 11 Aug 2004, Pavel Machek wrote:
> 
> Fine, so perhaps we do not want config option?

The inline spinlocks are _wonderful_ for seeing where the contention is in 
a simple profile.

In contrast, in a profile the out-of-lines ones will show "x% was spent on 
spinlocks". Which doesn't help much when you want to see where the problem 
is.

This was _hugely_ useful, at least for me, for seeing what locks were 
problematic.

		Linus

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Wed, 11 Aug 2004, Pavel Machek wrote:
>> Fine, so perhaps we do not want config option?

On Wed, Aug 11, 2004 at 03:13:15PM -0700, Linus Torvalds wrote:
> The inline spinlocks are _wonderful_ for seeing where the contention is in 
> a simple profile.
> In contrast, in a profile the out-of-lines ones will show "x% was spent on 
> spinlocks". Which doesn't help much when you want to see where the problem 
> is.
> This was _hugely_ useful, at least for me, for seeing what locks were 
> problematic.

Well, one trick with the kinda-sorta inline spinlocks is that they need
additional diagnostics (which are *REALLY* painful to get out of users)
to find where the overhead was, hence there were CONFIG_SPINLINE
patches to get rid of the lock section bits.


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Keith Owens]

On Wed, 11 Aug 2004 15:13:15 -0700 (PDT), 
Linus Torvalds <torvalds@osdl.org> wrote:
>
>
>On Wed, 11 Aug 2004, Pavel Machek wrote:
>> 
>> Fine, so perhaps we do not want config option?
>
>The inline spinlocks are _wonderful_ for seeing where the contention is in 
>a simple profile.
>
>In contrast, in a profile the out-of-lines ones will show "x% was spent on 
>spinlocks". Which doesn't help much when you want to see where the problem 
>is.
>
>This was _hugely_ useful, at least for me, for seeing what locks were 
>problematic.

Tweak the profile code to detect that the instruction pointer is in the
out of line spinlock code and replace the ip with the caller's ip.  We
already do that for ia64, where the out of line spinlock code is a big
win.  A kdb backtrace on an ia64 contended lock will even decode the
address of the lock, which is only possible because the lock address is
in a known location for this case.

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[David S. Miller]

On Thu, 12 Aug 2004 10:01:50 +1000
Keith Owens <kaos@ocs.com.au> wrote:

> Tweak the profile code to detect that the instruction pointer is in the
> out of line spinlock code and replace the ip with the caller's ip.  We
> already do that for ia64, where the out of line spinlock code is a big
> win.  A kdb backtrace on an ia64 contended lock will even decode the
> address of the lock, which is only possible because the lock address is
> in a known location for this case.

We were doing this on sparc64 as well.

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Thu, 12 Aug 2004, Keith Owens wrote:

> Tweak the profile code to detect that the instruction pointer is in the
> out of line spinlock code and replace the ip with the caller's ip.  We
> already do that for ia64, where the out of line spinlock code is a big
> win.  A kdb backtrace on an ia64 contended lock will even decode the
> address of the lock, which is only possible because the lock address is
> in a known location for this case.

Yes this was one of the downsides of using the code, same thing would have
to be done for oprofile. I'll have to give this a go.

Thanks,
	Zwane

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Wed, 11 Aug 2004 15:13:15 -0700 (PDT), Linus Torvalds wrote:
>> The inline spinlocks are _wonderful_ for seeing where the contention is in 
>> a simple profile.
>> In contrast, in a profile the out-of-lines ones will show "x% was spent on 
>> spinlocks". Which doesn't help much when you want to see where the problem 
>> is.
>> This was _hugely_ useful, at least for me, for seeing what locks were 
>> problematic.

On Thu, Aug 12, 2004 at 10:01:50AM +1000, Keith Owens wrote:
> Tweak the profile code to detect that the instruction pointer is in the
> out of line spinlock code and replace the ip with the caller's ip.  We
> already do that for ia64, where the out of line spinlock code is a big
> win.  A kdb backtrace on an ia64 contended lock will even decode the
> address of the lock, which is only possible because the lock address is
> in a known location for this case.

This would be a useful extension for other architectures also, if/when
they convert to doing likewise.

I actually favored making C language spin_lock() (i.e. the goddamn
thing is declared as a C function void spin_lock(spinlock_t *) and is
called like a normal C function -- no inline asm or inline C functions
at all) entrypoints beyond merely conslidating contention loops, but I
feared that would be too extreme of a reversal of the status quo to
ever get traction to post it. It did, however, shrink the kernel text
the most of any of the out-of-line spinlock patches by a large margin,
something completely absurd-sounding, like 220KB vs. 20KB-60KB. =)


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Wed, 11 Aug 2004, William Lee Irwin III wrote:

> I actually favored making C language spin_lock() (i.e. the goddamn
> thing is declared as a C function void spin_lock(spinlock_t *) and is
> called like a normal C function -- no inline asm or inline C functions
> at all) entrypoints beyond merely conslidating contention loops, but I
> feared that would be too extreme of a reversal of the status quo to
> ever get traction to post it. It did, however, shrink the kernel text
> the most of any of the out-of-line spinlock patches by a large margin,
> something completely absurd-sounding, like 220KB vs. 20KB-60KB. =)

Could you post the patch and the results? It'd also be interesting to see
the function call setup in a number of cases.

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Wed, 11 Aug 2004, William Lee Irwin III wrote:
>> I actually favored making C language spin_lock() (i.e. the goddamn
>> thing is declared as a C function void spin_lock(spinlock_t *) and is
>> called like a normal C function -- no inline asm or inline C functions
>> at all) entrypoints beyond merely conslidating contention loops, but I
>> feared that would be too extreme of a reversal of the status quo to
>> ever get traction to post it. It did, however, shrink the kernel text
>> the most of any of the out-of-line spinlock patches by a large margin,
>> something completely absurd-sounding, like 220KB vs. 20KB-60KB. =)

On Wed, Aug 11, 2004 at 09:37:36PM -0400, Zwane Mwaikambo wrote:
> Could you post the patch and the results? It'd also be interesting to see
> the function call setup in a number of cases.

Odd, it was either you or mpm who told me the results. I personally
never even tried running the thing. I was merely told some other, prior
attempt at doing some kind of spinlock uninlining failed to run, this
thing did, and that it shaved that memorable amount off of .text size.
I recall I compiled it myself and saw about half as much reduction
(120KB instead of 220KB), possibly due to .config or compiler differences.
I'll dust things off and so on.

-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Wed, Aug 11, 2004 at 07:04:24PM -0700, William Lee Irwin III wrote:
> Odd, it was either you or mpm who told me the results. I personally
> never even tried running the thing. I was merely told some other, prior
> attempt at doing some kind of spinlock uninlining failed to run, this
> thing did, and that it shaved that memorable amount off of .text size.
> I recall I compiled it myself and saw about half as much reduction
> (120KB instead of 220KB), possibly due to .config or compiler differences.
> I'll dust things off and so on.

Okay, the results on 2.6.8-rc4 (COOL had a bit of porting, basically
dropping the hunks associated with spin_lock_flags_string or whatever
it is). Chose the .config largely to be vaguely deterministic, but had
to nuke the "System is too big" check in arch/x86_64/boot/tools/build.c.

              text    data     bss     dec     hex filename
mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux

x86-64, -O2, allyesconfig minus the following:

# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
# CONFIG_MPSC is not set
# CONFIG_GENERIC_CPU is not set
# CONFIG_CPU_FREQ_DEFAULT_GOV_USERSPACE is not set
# CONFIG_MTD_CFI_BE_BYTE_SWAP is not set
# CONFIG_MTD_CFI_LE_BYTE_SWAP is not set
# CONFIG_IDE_ARM is not set
# CONFIG_AIC7XXX_BUILD_FIRMWARE is not set
# CONFIG_AIC79XX_BUILD_FIRMWARE is not set
# CONFIG_SCTP_HMAC_NONE is not set
# CONFIG_SCTP_HMAC_SHA1 is not set
# CONFIG_NET_SCH_CLK_GETTIMEOFDAY is not set
# CONFIG_NET_SCH_CLK_CPU is not set
# CONFIG_HPET_RTC_IRQ is not set
# CONFIG_AEDSP16_SBPRO is not set
# CONFIG_USB_GADGET_PXA2XX is not set
# CONFIG_USB_GADGET_GOKU is not set
# CONFIG_USB_GADGET_SA1100 is not set
# CONFIG_USB_GADGET_DUMMY_HCD is not set
# CONFIG_USB_ZERO is not set
# CONFIG_USB_GADGETFS is not set
# CONFIG_USB_FILE_STORAGE is not set
# CONFIG_USB_G_SERIAL is not set
# CONFIG_JFFS2_CMODE_NONE is not set
# CONFIG_JFFS2_CMODE_SIZE is not set
# CONFIG_DEBUG_SPINLOCK is not set

-- wli

Index: spinlock-2.6.8-rc1/include/linux/spinlock.h
===================================================================
--- spinlock-2.6.8-rc1.orig/include/linux/spinlock.h	2004-07-11 10:34:38.000000000 -0700
+++ spinlock-2.6.8-rc1/include/linux/spinlock.h	2004-07-14 21:38:47.000000000 -0700
@@ -60,90 +60,35 @@
 } spinlock_t;
 #define SPIN_LOCK_UNLOCKED (spinlock_t) { SPINLOCK_MAGIC, 0, 10, __FILE__ , NULL, 0}
 
-#define spin_lock_init(x) \
-	do { \
-		(x)->magic = SPINLOCK_MAGIC; \
-		(x)->lock = 0; \
-		(x)->babble = 5; \
-		(x)->module = __FILE__; \
-		(x)->owner = NULL; \
-		(x)->oline = 0; \
-	} while (0)
-
-#define CHECK_LOCK(x) \
-	do { \
-	 	if ((x)->magic != SPINLOCK_MAGIC) { \
-			printk(KERN_ERR "%s:%d: spin_is_locked on uninitialized spinlock %p.\n", \
-					__FILE__, __LINE__, (x)); \
-		} \
-	} while(0)
-
-#define _raw_spin_lock(x)		\
-	do { \
-	 	CHECK_LOCK(x); \
-		if ((x)->lock&&(x)->babble) { \
-			(x)->babble--; \
-			printk("%s:%d: spin_lock(%s:%p) already locked by %s/%d\n", \
-					__FILE__,__LINE__, (x)->module, \
-					(x), (x)->owner, (x)->oline); \
-		} \
-		(x)->lock = 1; \
-		(x)->owner = __FILE__; \
-		(x)->oline = __LINE__; \
-	} while (0)
+#define spin_lock_init(x)	__spin_lock_init(x, __FILE__)
+#define _raw_spin_lock(x)	__raw_spin_lock(x, __FILE__, __LINE__)
 
 /* without debugging, spin_is_locked on UP always says
  * FALSE. --> printk if already locked. */
-#define spin_is_locked(x) \
-	({ \
-	 	CHECK_LOCK(x); \
-		if ((x)->lock&&(x)->babble) { \
-			(x)->babble--; \
-			printk("%s:%d: spin_is_locked(%s:%p) already locked by %s/%d\n", \
-					__FILE__,__LINE__, (x)->module, \
-					(x), (x)->owner, (x)->oline); \
-		} \
-		0; \
-	})
+#define spin_is_locked(x)	__spin_is_locked(x, __FILE__, __LINE__)
 
 /* without debugging, spin_trylock on UP always says
  * TRUE. --> printk if already locked. */
-#define _raw_spin_trylock(x) \
-	({ \
-	 	CHECK_LOCK(x); \
-		if ((x)->lock&&(x)->babble) { \
-			(x)->babble--; \
-			printk("%s:%d: spin_trylock(%s:%p) already locked by %s/%d\n", \
-					__FILE__,__LINE__, (x)->module, \
-					(x), (x)->owner, (x)->oline); \
-		} \
-		(x)->lock = 1; \
-		(x)->owner = __FILE__; \
-		(x)->oline = __LINE__; \
-		1; \
-	})
-
-#define spin_unlock_wait(x)	\
-	do { \
-	 	CHECK_LOCK(x); \
-		if ((x)->lock&&(x)->babble) { \
-			(x)->babble--; \
-			printk("%s:%d: spin_unlock_wait(%s:%p) owned by %s/%d\n", \
-					__FILE__,__LINE__, (x)->module, (x), \
-					(x)->owner, (x)->oline); \
-		}\
-	} while (0)
-
-#define _raw_spin_unlock(x) \
-	do { \
-	 	CHECK_LOCK(x); \
-		if (!(x)->lock&&(x)->babble) { \
-			(x)->babble--; \
-			printk("%s:%d: spin_unlock(%s:%p) not locked\n", \
-					__FILE__,__LINE__, (x)->module, (x));\
-		} \
-		(x)->lock = 0; \
-	} while (0)
+#define _raw_spin_trylock(x)	__raw_spin_trylock(x, __FILE__, __LINE__)
+#define spin_unlock_wait(x)	__spin_unlock_wait(x, __FILE__, __LINE__)
+#define _raw_spin_unlock(x)	__raw_spin_unlock(x, __FILE__, __LINE__)
+#define spin_trylock(x)		__spin_trylock(x, __FILE__, __LINE__)
+#define spin_lock(x)		__spin_lock(x, __FILE__, __LINE__)
+#define spin_unlock(x)		__spin_unlock(x, __FILE__, __LINE__)
+
+int __spin_trylock(spinlock_t *, const char *, int);
+void __spin_lock(spinlock_t *, const char *, int);
+void __spin_lock_irq(spinlock_t *, const char *, int);
+void __spin_lock_irqsave(spinlock_t *, unsigned long *, const char *, int);
+void __spin_unlock(spinlock_t *, const char *, int);
+void __spin_unlock_irq(spinlock_t *, const char *, int);
+void __spin_unlock_irqrestore(spinlock_t *, unsigned long *, const char *, int);
+void __spin_lock_init(spinlock_t *, const char *);
+void __raw_spin_lock(spinlock_t *, const char *, int);
+int __spin_is_locked(spinlock_t *, const char *, int);
+int __raw_spin_trylock(spinlock_t *, const char *, int);
+void __spin_unlock_wait(spinlock_t *, const char *, int);
+void __raw_spin_unlock(spinlock_t *, const char *, int);
 #else
 /*
  * gcc versions before ~2.95 have a nasty bug with empty initializers.
@@ -176,226 +121,78 @@
   typedef struct { int gcc_is_buggy; } rwlock_t;
   #define RW_LOCK_UNLOCKED (rwlock_t) { 0 }
 #endif
+#endif /* !SMP */
 
+#if (!defined(CONFIG_SMP) && !defined(CONFIG_PREEMPT)) || \
+					defined(CONFIG_DEBUG_SPINLOCK)
 #define rwlock_init(lock)	do { (void)(lock); } while(0)
 #define _raw_read_lock(lock)	do { (void)(lock); } while(0)
 #define _raw_read_unlock(lock)	do { (void)(lock); } while(0)
 #define _raw_write_lock(lock)	do { (void)(lock); } while(0)
 #define _raw_write_unlock(lock)	do { (void)(lock); } while(0)
 #define _raw_write_trylock(lock) ({ (void)(lock); (1); })
+#endif
 
-#endif /* !SMP */
+/* "lock on reference count zero" */
+#if (defined(CONFIG_SMP) || defined(CONFIG_PREEMPT) || \
+	defined(CONFIG_DEBUG_SPINLOCK)) && !defined(ATOMIC_DEC_AND_LOCK)
+#include <asm/atomic.h>
+int atomic_dec_and_lock(atomic_t *, spinlock_t *);
+#endif
 
 /*
  * Define the various spin_lock and rw_lock methods.  Note we define these
  * regardless of whether CONFIG_SMP or CONFIG_PREEMPT are set. The various
  * methods are defined as nops in the case they are not required.
  */
-#define spin_trylock(lock)	({preempt_disable(); _raw_spin_trylock(lock) ? \
-				1 : ({preempt_enable(); 0;});})
-
-#define write_trylock(lock)	({preempt_disable();_raw_write_trylock(lock) ? \
-				1 : ({preempt_enable(); 0;});})
-
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
+int spin_trylock(spinlock_t *);
+int write_trylock(rwlock_t *);
 /* Where's read_trylock? */
 
+void spin_lock(spinlock_t *);
+void write_lock(rwlock_t *);
+
 #if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
 void __preempt_spin_lock(spinlock_t *lock);
 void __preempt_write_lock(rwlock_t *lock);
-
-#define spin_lock(lock) \
-do { \
-	preempt_disable(); \
-	if (unlikely(!_raw_spin_trylock(lock))) \
-		__preempt_spin_lock(lock); \
-} while (0)
-
-#define write_lock(lock) \
-do { \
-	preempt_disable(); \
-	if (unlikely(!_raw_write_trylock(lock))) \
-		__preempt_write_lock(lock); \
-} while (0)
-
-#else
-#define spin_lock(lock)	\
-do { \
-	preempt_disable(); \
-	_raw_spin_lock(lock); \
-} while(0)
-
-#define write_lock(lock) \
-do { \
-	preempt_disable(); \
-	_raw_write_lock(lock); \
-} while(0)
 #endif
 
-#define read_lock(lock)	\
-do { \
-	preempt_disable(); \
-	_raw_read_lock(lock); \
-} while(0)
-
-#define spin_unlock(lock) \
-do { \
-	_raw_spin_unlock(lock); \
-	preempt_enable(); \
-} while (0)
-
-#define write_unlock(lock) \
-do { \
-	_raw_write_unlock(lock); \
-	preempt_enable(); \
-} while(0)
-
-#define read_unlock(lock) \
-do { \
-	_raw_read_unlock(lock); \
-	preempt_enable(); \
-} while(0)
-
-#define spin_lock_irqsave(lock, flags) \
-do { \
-	local_irq_save(flags); \
-	preempt_disable(); \
-	_raw_spin_lock_flags(lock, flags); \
-} while (0)
-
-#define spin_lock_irq(lock) \
-do { \
-	local_irq_disable(); \
-	preempt_disable(); \
-	_raw_spin_lock(lock); \
-} while (0)
-
-#define spin_lock_bh(lock) \
-do { \
-	local_bh_disable(); \
-	preempt_disable(); \
-	_raw_spin_lock(lock); \
-} while (0)
-
-#define read_lock_irqsave(lock, flags) \
-do { \
-	local_irq_save(flags); \
-	preempt_disable(); \
-	_raw_read_lock(lock); \
-} while (0)
-
-#define read_lock_irq(lock) \
-do { \
-	local_irq_disable(); \
-	preempt_disable(); \
-	_raw_read_lock(lock); \
-} while (0)
-
-#define read_lock_bh(lock) \
-do { \
-	local_bh_disable(); \
-	preempt_disable(); \
-	_raw_read_lock(lock); \
-} while (0)
-
-#define write_lock_irqsave(lock, flags) \
-do { \
-	local_irq_save(flags); \
-	preempt_disable(); \
-	_raw_write_lock(lock); \
-} while (0)
-
-#define write_lock_irq(lock) \
-do { \
-	local_irq_disable(); \
-	preempt_disable(); \
-	_raw_write_lock(lock); \
-} while (0)
-
-#define write_lock_bh(lock) \
-do { \
-	local_bh_disable(); \
-	preempt_disable(); \
-	_raw_write_lock(lock); \
-} while (0)
-
-#define spin_unlock_irqrestore(lock, flags) \
-do { \
-	_raw_spin_unlock(lock); \
-	local_irq_restore(flags); \
-	preempt_enable(); \
-} while (0)
-
-#define _raw_spin_unlock_irqrestore(lock, flags) \
-do { \
-	_raw_spin_unlock(lock); \
-	local_irq_restore(flags); \
-} while (0)
-
-#define spin_unlock_irq(lock) \
-do { \
-	_raw_spin_unlock(lock); \
-	local_irq_enable(); \
-	preempt_enable(); \
-} while (0)
-
-#define spin_unlock_bh(lock) \
-do { \
-	_raw_spin_unlock(lock); \
-	preempt_enable(); \
-	local_bh_enable(); \
-} while (0)
-
-#define read_unlock_irqrestore(lock, flags) \
-do { \
-	_raw_read_unlock(lock); \
-	local_irq_restore(flags); \
-	preempt_enable(); \
-} while (0)
-
-#define read_unlock_irq(lock) \
-do { \
-	_raw_read_unlock(lock); \
-	local_irq_enable(); \
-	preempt_enable(); \
-} while (0)
-
-#define read_unlock_bh(lock) \
-do { \
-	_raw_read_unlock(lock); \
-	preempt_enable(); \
-	local_bh_enable(); \
-} while (0)
-
-#define write_unlock_irqrestore(lock, flags) \
-do { \
-	_raw_write_unlock(lock); \
-	local_irq_restore(flags); \
-	preempt_enable(); \
-} while (0)
-
-#define write_unlock_irq(lock) \
-do { \
-	_raw_write_unlock(lock); \
-	local_irq_enable(); \
-	preempt_enable(); \
-} while (0)
-
-#define write_unlock_bh(lock) \
-do { \
-	_raw_write_unlock(lock); \
-	preempt_enable(); \
-	local_bh_enable(); \
-} while (0)
-
-#define spin_trylock_bh(lock)	({ local_bh_disable(); preempt_disable(); \
-				_raw_spin_trylock(lock) ? 1 : \
-				({preempt_enable(); local_bh_enable(); 0;});})
-
-/* "lock on reference count zero" */
-#ifndef ATOMIC_DEC_AND_LOCK
-#include <asm/atomic.h>
-extern int atomic_dec_and_lock(atomic_t *atomic, spinlock_t *lock);
-#endif
+void read_lock(rwlock_t *);
+void spin_unlock(spinlock_t *);
+void write_unlock(rwlock_t *);
+void read_unlock(rwlock_t *);
+void __spin_lock_irqsave(spinlock_t *, unsigned long *);
+void spin_lock_irq(spinlock_t *);
+void spin_lock_bh(spinlock_t *);
+void __read_lock_irqsave(rwlock_t *, unsigned long *);
+void read_lock_irq(rwlock_t *);
+void read_lock_bh(rwlock_t *);
+void __write_lock_irqsave(rwlock_t *, unsigned long *);
+void write_lock_irq(rwlock_t *);
+void write_lock_bh(rwlock_t *);
+void __spin_unlock_irqrestore(spinlock_t *, unsigned long *);
+void __raw_spin_unlock_irqrestore(spinlock_t *, unsigned long *);
+void spin_unlock_irq(spinlock_t *);
+void spin_unlock_bh(spinlock_t *);
+void __read_unlock_irqrestore(rwlock_t *, unsigned long *);
+void read_unlock_irq(rwlock_t *);
+void read_unlock_bh(rwlock_t *);
+void __write_unlock_irqrestore(rwlock_t *, unsigned long *);
+void write_unlock_irq(rwlock_t *);
+void write_unlock_bh(rwlock_t *);
+int spin_trylock_bh(spinlock_t *);
+
+#define spin_lock_irqsave(lock, flags)	__spin_lock_irqsave(lock, &(flags))
+#define read_lock_irqsave(lock, flags)	__read_lock_irqsave(lock, &(flags))
+#define write_lock_irqsave(lock, flags)	__write_lock_irqsave(lock, &(flags))
+#define spin_unlock_irqrestore(lock, flags) __spin_unlock_irqrestore(lock, &(flags))
+#define _raw_spin_unlock_irqrestore(lock, flags)			\
+	__raw_spin_unlock_irqrestore(lock, &(flags))
+#define read_unlock_irqrestore(lock, flags)				\
+	__read_unlock_irqrestore(lock, &(flags))
+#define write_unlock_irqrestore(lock, flags)				\
+	__write_unlock_irqrestore(lock, &(flags))
 
 /*
  *  bit-based spin_lock()
@@ -403,68 +200,48 @@
  * Don't use this unless you really need to: spin_lock() and spin_unlock()
  * are significantly faster.
  */
-static inline void bit_spin_lock(int bitnum, unsigned long *addr)
-{
-	/*
-	 * Assuming the lock is uncontended, this never enters
-	 * the body of the outer loop. If it is contended, then
-	 * within the inner loop a non-atomic test is used to
-	 * busywait with less bus contention for a good time to
-	 * attempt to acquire the lock bit.
-	 */
-	preempt_disable();
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-	while (test_and_set_bit(bitnum, addr)) {
-		while (test_bit(bitnum, addr))
-			cpu_relax();
-	}
-#endif
-}
-
-/*
- * Return true if it was acquired
- */
-static inline int bit_spin_trylock(int bitnum, unsigned long *addr)
-{
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-	int ret;
-
-	preempt_disable();
-	ret = !test_and_set_bit(bitnum, addr);
-	if (!ret)
-		preempt_enable();
-	return ret;
-#else
-	preempt_disable();
-	return 1;
-#endif
-}
-
-/*
- *  bit-based spin_unlock()
- */
-static inline void bit_spin_unlock(int bitnum, unsigned long *addr)
-{
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-	BUG_ON(!test_bit(bitnum, addr));
-	smp_mb__before_clear_bit();
-	clear_bit(bitnum, addr);
-#endif
-	preempt_enable();
-}
-
-/*
- * Return true if the lock is held.
- */
-static inline int bit_spin_is_locked(int bitnum, unsigned long *addr)
-{
-#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
-	return test_bit(bitnum, addr);
-#elif defined CONFIG_PREEMPT
-	return preempt_count();
-#else
-	return 1;
+void bit_spin_lock(int, unsigned long *);
+int bit_spin_trylock(int, unsigned long *);
+void bit_spin_unlock(int, unsigned long *);
+int bit_spin_is_locked(int, unsigned long *);
+#else
+#define bit_spin_lock(bit, lock)	do { (void)(bit); (void)(lock); } while (0)
+#define bit_spin_unlock(bit, lock)	do { (void)(bit); (void)(lock); } while (0)
+#define bit_spin_trylock(bit, lock)	({ (void)(bit); (void)(lock); 1; })
+#define bit_spin_is_locked(bit, lock)	({ (void)(bit); (void)(lock); 1; })
+
+#ifndef CONFIG_DEBUG_SPINLOCK
+#define spin_trylock(lock)		_raw_spin_trylock(lock)
+#define spin_lock(lock)			_raw_spin_lock(lock)
+#define spin_unlock(lock)		_raw_spin_lock(lock)
+#endif
+
+#define read_lock(lock)			_raw_read_lock(lock)
+#define read_unlock(lock)		_raw_read_lock(lock)
+#define write_lock(lock)		_raw_write_lock(lock)
+#define write_unlock(lock)		_raw_write_lock(lock)
+
+#define spin_lock_irq(lock)		do { local_irq_disable(); _raw_spin_lock(lock); } while (0)
+#define spin_unlock_irq(lock)		do { _raw_spin_lock(lock); local_irq_disable(); } while (0)
+#define read_lock_irq(lock)		do { local_irq_disable(); _raw_read_lock(lock); } while (0)
+#define read_unlock_irq(lock)		do { _raw_read_lock(lock); local_irq_enable(); } while (0)
+#define write_lock_irq(lock)		do { local_irq_disable(); _raw_write_lock(lock); } while (0)
+#define write_unlock_irq(lock)		do { _raw_write_lock(lock); local_irq_enable(); } while (0)
+
+#define spin_lock_irqsave(lock, flags)	do { local_irq_save(flags); _raw_spin_lock(lock); } while (0)
+#define spin_unlock_irqrestore(lock, flags) do { _raw_spin_lock(lock); local_irq_restore(flags); } while (0)
+#define read_lock_irqsave(lock, flags)	do { _raw_read_lock(lock); local_irq_save(flags); } while (0)
+#define read_unlock_irqrestore(lock, flags) do { _raw_read_lock(lock); local_irq_restore(flags); } while (0)
+#define write_lock_irqsave(lock, flags)	do { local_irq_save(flags); _raw_write_lock(lock); } while (0)
+#define write_unlock_irqrestore(lock, flags) do { _raw_write_lock(lock); local_irq_restore(flags); } while (0)
+
+#define spin_trylock_bh(lock)		({ local_bh_disable(); _raw_spin_trylock(lock); })
+#define spin_lock_bh(lock)		do { local_bh_disable(); _raw_spin_lock(lock); } while (0)
+#define spin_unlock_bh(lock)		do { _raw_spin_lock(lock); local_bh_enable(); } while (0)
+#define read_lock_bh(lock)		do { local_bh_disable(); _raw_read_lock(lock); } while (0)
+#define read_unlock_bh(lock)		do { _raw_read_lock(lock); local_bh_enable(); } while (0)
+#define write_lock_bh(lock)		do { local_bh_disable(); _raw_write_lock(lock); } while (0)
+#define write_unlock_bh(lock)		do { _raw_write_lock(lock); local_bh_enable(); } while (0)
 #endif
-}
 
 #endif /* __LINUX_SPINLOCK_H */
Index: spinlock-2.6.8-rc1/kernel/spinlock.c
===================================================================
--- spinlock-2.6.8-rc1.orig/kernel/spinlock.c	2004-06-07 00:42:31.000000000 -0700
+++ spinlock-2.6.8-rc1/kernel/spinlock.c	2004-07-14 18:23:46.000000000 -0700
@@ -0,0 +1,448 @@
+/*
+ * kernel/spinlock.c - generic locking
+ * (c) William Irwin, Oracle, July 2004
+ */
+#include <linux/config.h>
+#include <linux/preempt.h>
+#include <linux/linkage.h>
+#include <linux/compiler.h>
+#include <linux/thread_info.h>
+#include <linux/kernel.h>
+#include <linux/stringify.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+
+#include <asm/processor.h>	/* for cpu relax */
+#include <asm/system.h>
+
+/*
+ * If CONFIG_SMP is set, pull in the _raw_* definitions
+ */
+#ifdef CONFIG_SMP
+#include <asm/spinlock.h>
+
+#else
+
+#ifdef CONFIG_DEBUG_SPINLOCK
+
+void __spin_lock_init(spinlock_t *lock, const char *file)
+{
+	lock->magic = SPINLOCK_MAGIC;
+	lock->lock = 0;
+	lock->babble = 5;
+	lock->module = (char *)file;
+	lock->owner = NULL;
+	lock->oline = 0;
+}
+EXPORT_SYMBOL(__spin_lock_init);
+
+static void check_lock(spinlock_t *lock, const char *file, int line)
+{
+ 	if (lock->magic != SPINLOCK_MAGIC)
+		printk(KERN_ERR "%s:%d: spin_is_locked on "
+			"uninitialized spinlock %p.\n",
+			file, line, lock);
+}
+EXPORT_SYMBOL(check_lock);
+
+void __raw_spin_lock(spinlock_t *lock, const char *file, int line)
+{
+	check_lock(lock, file, line);
+	if (lock->lock && lock->babble) {
+		lock->babble--;
+		printk("%s:%d: spin_lock(%s:%p) already locked by %s/%d\n",
+				file, line, lock->module,
+				lock, lock->owner, lock->oline);
+	}
+	lock->lock = 1;
+	lock->owner = (char *)file;
+	lock->oline = line;
+}
+
+/* without debugging, spin_is_locked on UP always says
+ * FALSE. --> printk if already locked. */
+int __spin_is_locked(spinlock_t *lock, const char *file, int line)
+{
+	check_lock(lock, file, line);
+	if (lock->lock && lock->babble) {
+		lock->babble--;
+		printk("%s:%d: spin_is_locked(%s:%p) already locked by %s/%d\n",
+			file, line, lock->module,
+			lock, lock->owner, lock->oline);
+		}
+		return 0;
+}
+EXPORT_SYMBOL(__spin_is_locked);
+
+/* without debugging, spin_trylock on UP always says
+ * TRUE. --> printk if already locked. */
+int __raw_spin_trylock(spinlock_t *lock, const char *file, int line)
+{
+	check_lock(lock, file, line);
+	if (lock->lock && lock->babble) {
+		lock->babble--;
+		printk("%s:%d: spin_trylock(%s:%p) already locked by %s/%d\n",
+				file, line, lock->module,
+				lock, lock->owner, lock->oline);
+	}
+	lock->lock = 1;
+	lock->owner = (char *)file;
+	lock->oline = line;
+	return 1;
+}
+EXPORT_SYMBOL(__raw_spin_trylock);
+
+void __spin_unlock_wait(spinlock_t *lock, const char *file, int line)
+{
+ 	check_lock(lock, file, line);
+	if (lock->lock && lock->babble) {
+		lock->babble--;
+		printk("%s:%d: spin_unlock_wait(%s:%p) owned by %s/%d\n",
+				file, line, lock->module, lock,
+				lock->owner, lock->oline);
+	}
+}
+EXPORT_SYMBOL(__spin_unlock_wait);
+
+void __raw_spin_unlock(spinlock_t *lock, const char *file, int line)
+{
+ 	check_lock(lock, file, line);
+	if (!lock->lock && lock->babble) {
+		lock->babble--;
+		printk("%s:%d: spin_unlock(%s:%p) not locked\n",
+				file, line, lock->module, lock);
+	}
+	lock->lock = 0;
+}
+EXPORT_SYMBOL(__raw_spin_unlock);
+#endif /* CONFIG_DEBUG_SPINLOCK */
+
+#endif /* !SMP */
+
+/*
+ * Define the various spin_lock and rw_lock methods.  Note we define these
+ * regardless of whether CONFIG_SMP or CONFIG_PREEMPT are set. The various
+ * methods are defined as nops in the case they are not required.
+ */
+int spin_trylock(spinlock_t *lock)
+{
+	preempt_disable();
+	if (_raw_spin_trylock(lock))
+		return 1;
+	preempt_enable();
+	return 0;
+}
+EXPORT_SYMBOL(spin_trylock);
+
+int write_trylock(rwlock_t *lock)
+{
+	preempt_disable();
+	if (_raw_write_trylock(lock))
+		return 1;
+	preempt_enable();
+	return 0;
+}
+EXPORT_SYMBOL(write_trylock);
+
+/* Where's read_trylock? */
+
+#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
+void __preempt_spin_lock(spinlock_t *lock);
+void __preempt_write_lock(rwlock_t *lock);
+
+void spin_lock(spinlock_t *lock)
+{
+	preempt_disable();
+	if (unlikely(!_raw_spin_trylock(lock)))
+		__preempt_spin_lock(lock);
+}
+EXPORT_SYMBOL(spin_lock);
+
+void write_lock(rwlock_t *lock)
+{
+	preempt_disable();
+	if (unlikely(!_raw_write_trylock(lock)))
+		__preempt_write_lock(lock);
+}
+EXPORT_SYMBOL(write_lock);
+
+#else
+void spin_lock(spinlock_t *lock)
+{
+	preempt_disable();
+	_raw_spin_lock(lock);
+}
+EXPORT_SYMBOL(spin_lock);
+
+void write_lock(rwlock_t *lock)
+{
+	preempt_disable();
+	_raw_write_lock(lock);
+}
+EXPORT_SYMBOL(write_lock);
+#endif
+
+void read_lock(rwlock_t *lock)
+{
+	preempt_disable();
+	_raw_read_lock(lock);
+}
+EXPORT_SYMBOL(read_lock);
+
+void spin_unlock(spinlock_t *lock)
+{
+	_raw_spin_unlock(lock);
+	preempt_enable();
+}
+EXPORT_SYMBOL(spin_unlock);
+
+void write_unlock(rwlock_t *lock)
+{
+	_raw_write_unlock(lock);
+	preempt_enable();
+}
+EXPORT_SYMBOL(write_unlock);
+
+void read_unlock(rwlock_t *lock)
+{
+	_raw_read_unlock(lock);
+	preempt_enable();
+}
+EXPORT_SYMBOL(read_unlock);
+
+void __spin_lock_irqsave(spinlock_t *lock, unsigned long *flags)
+{
+	local_irq_save(*flags);
+	preempt_disable();
+	_raw_spin_lock_flags(lock, *flags);
+}
+EXPORT_SYMBOL(__spin_lock_irqsave);
+
+void spin_lock_irq(spinlock_t *lock)
+{
+	local_irq_disable();
+	preempt_disable();
+	_raw_spin_lock(lock);
+}
+EXPORT_SYMBOL(spin_lock_irq);
+
+void spin_lock_bh(spinlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	_raw_spin_lock(lock);
+}
+EXPORT_SYMBOL(spin_lock_bh);
+
+void __read_lock_irqsave(rwlock_t *lock, unsigned long *flags)
+{
+	local_irq_save(*flags);
+	preempt_disable();
+	_raw_read_lock(lock);
+}
+EXPORT_SYMBOL(__read_lock_irqsave);
+
+void read_lock_irq(rwlock_t *lock)
+{
+	local_irq_disable();
+	preempt_disable();
+	_raw_read_lock(lock);
+}
+EXPORT_SYMBOL(read_lock_irq);
+
+void read_lock_bh(rwlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	_raw_read_lock(lock);
+}
+EXPORT_SYMBOL(read_lock_bh);
+
+void __write_lock_irqsave(rwlock_t *lock, unsigned long *flags)
+{
+	local_irq_save(*flags);
+	preempt_disable();
+	_raw_write_lock(lock);
+}
+EXPORT_SYMBOL(__write_lock_irqsave);
+
+void write_lock_irq(rwlock_t *lock)
+{
+	local_irq_disable();
+	preempt_disable();
+	_raw_write_lock(lock);
+}
+EXPORT_SYMBOL(write_lock_irq);
+
+void write_lock_bh(rwlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	_raw_write_lock(lock);
+}
+EXPORT_SYMBOL(write_lock_bh);
+
+void __spin_unlock_irqrestore(spinlock_t *lock, unsigned long *flags)
+{
+	_raw_spin_unlock(lock);
+	local_irq_restore(*flags);
+	preempt_enable();
+}
+EXPORT_SYMBOL(__spin_unlock_irqrestore);
+
+void __raw_spin_unlock_irqrestore(spinlock_t *lock, unsigned long *flags)
+{
+	_raw_spin_unlock(lock);
+	local_irq_restore(*flags);
+}
+EXPORT_SYMBOL(__raw_spin_unlock_irqrestore);
+
+void spin_unlock_irq(spinlock_t *lock)
+{
+	_raw_spin_unlock(lock);
+	local_irq_enable();
+	preempt_enable();
+}
+EXPORT_SYMBOL(spin_unlock_irq);
+
+void spin_unlock_bh(spinlock_t *lock)
+{
+	_raw_spin_unlock(lock);
+	preempt_enable();
+	local_bh_enable();
+}
+EXPORT_SYMBOL(spin_unlock_bh);
+
+void __read_unlock_irqrestore(rwlock_t *lock, unsigned long *flags)
+{
+	_raw_read_unlock(lock);
+	local_irq_restore(*flags);
+	preempt_enable();
+}
+EXPORT_SYMBOL(__read_unlock_irqrestore);
+
+void read_unlock_irq(rwlock_t *lock)
+{
+	_raw_read_unlock(lock);
+	local_irq_enable();
+	preempt_enable();
+}
+EXPORT_SYMBOL(read_unlock_irq);
+
+void read_unlock_bh(rwlock_t *lock)
+{
+	_raw_read_unlock(lock);
+	preempt_enable();
+	local_bh_enable();
+}
+EXPORT_SYMBOL(read_unlock_bh);
+
+void __write_unlock_irqrestore(rwlock_t *lock, unsigned long *flags)
+{
+	_raw_write_unlock(lock);
+	local_irq_restore(*flags);
+	preempt_enable();
+}
+EXPORT_SYMBOL(__write_unlock_irqrestore);
+
+void write_unlock_irq(rwlock_t *lock)
+{
+	_raw_write_unlock(lock);
+	local_irq_enable();
+	preempt_enable();
+}
+EXPORT_SYMBOL(write_unlock_irq);
+
+void write_unlock_bh(rwlock_t *lock)
+{
+	_raw_write_unlock(lock);
+	preempt_enable();
+	local_bh_enable();
+}
+EXPORT_SYMBOL(write_unlock_bh);
+
+int spin_trylock_bh(spinlock_t *lock)
+{
+	local_bh_disable();
+	preempt_disable();
+	if (_raw_spin_trylock(lock))
+		return 1;
+	preempt_enable();
+	local_bh_enable();
+	return 0;
+}
+EXPORT_SYMBOL(spin_trylock_bh);
+
+/*
+ *  bit-based spin_lock()
+ *
+ * Don't use this unless you really need to: spin_lock() and spin_unlock()
+ * are significantly faster.
+ */
+void bit_spin_lock(int bitnum, unsigned long *addr)
+{
+	/*
+	 * Assuming the lock is uncontended, this never enters
+	 * the body of the outer loop. If it is contended, then
+	 * within the inner loop a non-atomic test is used to
+	 * busywait with less bus contention for a good time to
+	 * attempt to acquire the lock bit.
+	 */
+	preempt_disable();
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+	while (test_and_set_bit(bitnum, addr)) {
+		while (test_bit(bitnum, addr))
+			cpu_relax();
+	}
+#endif
+}
+EXPORT_SYMBOL(bit_spin_lock);
+
+/*
+ * Return true if it was acquired
+ */
+int bit_spin_trylock(int bitnum, unsigned long *addr)
+{
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+	int ret;
+
+	preempt_disable();
+	ret = !test_and_set_bit(bitnum, addr);
+	if (!ret)
+		preempt_enable();
+	return ret;
+#else
+	preempt_disable();
+	return 1;
+#endif
+}
+EXPORT_SYMBOL(bit_spin_trylock);
+
+/*
+ *  bit-based spin_unlock()
+ */
+void bit_spin_unlock(int bitnum, unsigned long *addr)
+{
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+	BUG_ON(!test_bit(bitnum, addr));
+	smp_mb__before_clear_bit();
+	clear_bit(bitnum, addr);
+#endif
+	preempt_enable();
+}
+EXPORT_SYMBOL(bit_spin_unlock);
+
+/*
+ * Return true if the lock is held.
+ */
+int bit_spin_is_locked(int bitnum, unsigned long *addr)
+{
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+	return test_bit(bitnum, addr);
+#elif defined CONFIG_PREEMPT
+	return preempt_count();
+#else
+	return 1;
+#endif
+}
+EXPORT_SYMBOL(bit_spin_is_locked);
Index: spinlock-2.6.8-rc1/kernel/Makefile
===================================================================
--- spinlock-2.6.8-rc1.orig/kernel/Makefile	2004-07-11 10:34:10.000000000 -0700
+++ spinlock-2.6.8-rc1/kernel/Makefile	2004-07-14 05:00:48.000000000 -0700
@@ -23,6 +23,17 @@
 obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
 obj-$(CONFIG_AUDIT) += audit.o
 obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
+ifeq ($(CONFIG_PREEMPT),y)
+	obj-y += spinlock.o
+else
+	ifeq ($(CONFIG_SMP),y)
+		obj-y += spinlock.o
+	else
+		ifeq ($(CONFIG_DEBUG_SPINLOCK),y)
+			obj-y += spinlock.o
+		endif
+	endif
+endif
 
 ifneq ($(CONFIG_IA64),y)
 # According to Alan Modra <alan@linuxcare.com.au>, the -fno-omit-frame-pointer is

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Thu, Aug 12, 2004 at 12:20:58AM -0700, William Lee Irwin III wrote:
> Okay, the results on 2.6.8-rc4 (COOL had a bit of porting, basically
> dropping the hunks associated with spin_lock_flags_string or whatever
> it is). Chose the .config largely to be vaguely deterministic, but had
> to nuke the "System is too big" check in arch/x86_64/boot/tools/build.c.
> 
>               text    data     bss     dec     hex filename
> mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
> cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
> C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux
> x86-64, -O2, allyesconfig minus the following:
[...]

The precise COOL patch used:


Index: spinlock-2.6.8-rc4/arch/x86_64/Kconfig
===================================================================
--- spinlock-2.6.8-rc4.orig/arch/x86_64/Kconfig	2004-06-15 22:19:44.000000000 -0700
+++ spinlock-2.6.8-rc4/arch/x86_64/Kconfig	2004-08-11 22:24:47.944933072 -0700
@@ -436,6 +436,16 @@
 	  best used in conjunction with the NMI watchdog so that spinlock
 	  deadlocks are also debuggable.
 
+config COOL_SPINLOCK
+	bool "Completely out of line spinlocks"
+	depends on SMP
+	default y
+	help
+	  Say Y here to build spinlocks which have common text for contended
+	  and uncontended paths. This reduces kernel text size by at least
+	  50k on most configurations, plus there is the additional benefit
+	  of better cache utilisation.
+
 # !SMP for now because the context switch early causes GPF in segment reloading
 # and the GS base checking does the wrong thing then, causing a hang.
 config CHECKING
Index: spinlock-2.6.8-rc4/arch/x86_64/kernel/x8664_ksyms.c
===================================================================
--- spinlock-2.6.8-rc4.orig/arch/x86_64/kernel/x8664_ksyms.c	2004-08-10 23:00:19.712651608 -0700
+++ spinlock-2.6.8-rc4/arch/x86_64/kernel/x8664_ksyms.c	2004-08-11 22:24:47.944933072 -0700
@@ -218,3 +218,14 @@
 EXPORT_SYMBOL_GPL(flush_tlb_all);
 #endif
 
+#ifdef CONFIG_COOL_SPINLOCK
+extern void asmlinkage __spin_lock_failed(spinlock_t *);
+extern void asmlinkage __spin_lock_failed_flags(spinlock_t *, unsigned long);
+extern void asmlinkage __spin_lock_loop(spinlock_t *);
+extern void asmlinkage __spin_lock_loop_flags(spinlock_t *, unsigned long);
+EXPORT_SYMBOL(__spin_lock_failed);
+EXPORT_SYMBOL(__spin_lock_failed_flags);
+EXPORT_SYMBOL(__spin_lock_loop);
+EXPORT_SYMBOL(__spin_lock_loop_flags);
+#endif
+
Index: spinlock-2.6.8-rc4/arch/x86_64/lib/Makefile
===================================================================
--- spinlock-2.6.8-rc4.orig/arch/x86_64/lib/Makefile	2004-06-15 22:18:59.000000000 -0700
+++ spinlock-2.6.8-rc4/arch/x86_64/lib/Makefile	2004-08-11 22:25:22.127736496 -0700
@@ -12,3 +12,4 @@
 lib-y += memcpy.o memmove.o memset.o copy_user.o
 
 lib-$(CONFIG_HAVE_DEC_LOCK) += dec_and_lock.o
+lib-$(CONFIG_COOL_SPINLOCK) += spinlock.o
Index: spinlock-2.6.8-rc4/arch/x86_64/lib/spinlock.c
===================================================================
--- spinlock-2.6.8-rc4.orig/arch/x86_64/lib/spinlock.c	2004-04-06 10:56:48.000000000 -0700
+++ spinlock-2.6.8-rc4/arch/x86_64/lib/spinlock.c	2004-08-11 22:24:47.945932920 -0700
@@ -0,0 +1,57 @@
+#define PROC(name)	\
+	".align 4\n"	\
+	".globl " #name"\n" \
+	#name":\n"
+
+asm (PROC(__spin_lock_failed_flags)
+	"test $0x200, %rbx\n"
+	"jz 1f\n"
+	"sti\n"
+	"1:\n\t"
+	"rep; nop\n"
+	"cmpb $0, (%rax)\n"
+	"jle 1b\n"
+	"cli\n"
+	"lock; decb (%rax)\n\t"
+	"js __spin_lock_failed_flags\n\t"
+	"nop\n"
+	"ret\n"
+);
+
+asm (PROC(__spin_lock_loop_flags)
+	"lock; decb (%rax)\n\t"
+	"js 1f\n\t"
+	"nop\n\t"
+	"ret\n\t"
+	"1:\n\t"
+	"test $0x200, %rbx\n\t"
+	"jz 1f\n\t"
+	"sti\n\t"
+	"2: rep; nop\n\t"
+	"cmpb $0, (%rax)\n\t"
+	"jle 2b\n\t"
+	"cli\n\t"
+	"jmp __spin_lock_loop_flags\n\t"
+);
+
+asm (PROC(__spin_lock_failed)
+	"rep; nop\n\t"
+	"cmpb $0, (%rax)\n\t"
+	"jle __spin_lock_failed\n\t"
+	"lock; decb (%rax)\n\t"
+	"js __spin_lock_failed\n\t"
+	"nop\n\t"
+	"ret\n\t"
+);
+
+asm (PROC(__spin_lock_loop)
+	"lock; decb (%rax)\n\t"
+	"js 1f\n\t"
+	"nop\n\t"
+	"ret\n\t"
+	"1: rep; nop\n\t"
+	"cmpb $0, (%rax)\n\t"
+	"jle 1b\n\t"
+	"jmp __spin_lock_loop\n\t"
+);
+
Index: spinlock-2.6.8-rc4/include/asm-x86_64/spinlock.h
===================================================================
--- spinlock-2.6.8-rc4.orig/include/asm-x86_64/spinlock.h	2004-06-15 22:20:04.000000000 -0700
+++ spinlock-2.6.8-rc4/include/asm-x86_64/spinlock.h	2004-08-11 22:24:47.945932920 -0700
@@ -43,6 +43,13 @@
 #define spin_unlock_wait(x)	do { barrier(); } while(spin_is_locked(x))
 #define _raw_spin_lock_flags(lock, flags) _raw_spin_lock(lock)
 
+#ifdef CONFIG_COOL_SPINLOCK
+#define spin_lock_string \
+	"call __spin_lock_loop\n\t"
+
+#define spin_lock_string_flags \
+	"call __spin_lock_loop_flags\n\t"
+#else
 #define spin_lock_string \
 	"\n1:\t" \
 	"lock ; decb %0\n\t" \
@@ -54,6 +61,7 @@
 	"jle 2b\n\t" \
 	"jmp 1b\n" \
 	LOCK_SECTION_END
+#endif
 
 /*
  * This works. Despite all the confusion.
@@ -122,7 +130,12 @@
 #endif
 	__asm__ __volatile__(
 		spin_lock_string
-		:"=m" (lock->lock) : : "memory");
+#ifdef CONFIG_COOL_SPINLOCK
+		: : "a" (&lock->lock) : "memory"
+#else
+		:"=m" (lock->lock) : : "memory"
+#endif
+	);
 }
 
 

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Thu, Aug 12, 2004 at 12:20:58AM -0700, William Lee Irwin III wrote:
>> Okay, the results on 2.6.8-rc4 (COOL had a bit of porting, basically
>> dropping the hunks associated with spin_lock_flags_string or whatever
>> it is). Chose the .config largely to be vaguely deterministic, but had
>> to nuke the "System is too big" check in arch/x86_64/boot/tools/build.c.
>>               text    data     bss     dec     hex filename
>> mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
>> cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
>> C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux
>> x86-64, -O2, allyesconfig minus the following:
> [...]

On Thu, Aug 12, 2004 at 12:23:38AM -0700, William Lee Irwin III wrote:
> The precise COOL patch used:

For completeness, the "System is too big" avoidance patch, too.
Please don't merge this. It's just "full disclosure" for how I ran this
particular compiletest.


Index: spinlock-2.6.8-rc4/arch/x86_64/boot/tools/build.c
===================================================================
--- spinlock-2.6.8-rc4.orig/arch/x86_64/boot/tools/build.c	2004-06-15 22:20:27.000000000 -0700
+++ spinlock-2.6.8-rc4/arch/x86_64/boot/tools/build.c	2004-08-11 23:22:43.242607520 -0700
@@ -151,9 +151,6 @@
 	fprintf (stderr, "System is %d kB\n", sz/1024);
 	sys_size = (sz + 15) / 16;
 	/* 0x40000*16 = 4.0 MB, reasonable estimate for the current maximum */
-	if (sys_size > (is_big_kernel ? 0x40000 : DEF_SYSSIZE))
-		die("System is too big. Try using %smodules.",
-			is_big_kernel ? "" : "bzImage or ");
 	while (sz > 0) {
 		int l, n;
 

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Thu, 12 Aug 2004, William Lee Irwin III wrote:
>> Okay, the results on 2.6.8-rc4 (COOL had a bit of porting, basically
>> dropping the hunks associated with spin_lock_flags_string or whatever
>> it is). Chose the .config largely to be vaguely deterministic, but had
>> to nuke the "System is too big" check in arch/x86_64/boot/tools/build.c.
>>               text    data     bss     dec     hex filename
>> mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
>> cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
>> C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux

On Thu, Aug 12, 2004 at 04:12:55AM -0400, Zwane Mwaikambo wrote:
> Shit that's quite the variance, which compiler are you using?

$ gcc --version
gcc (GCC) 3.3.3 (SuSE Linux)
Copyright (C) 2003 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

It's the compiler on residue; you (zwane) should be able to log in and
take a closer look if need be.

-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Thu, 12 Aug 2004, William Lee Irwin III wrote:

> On Wed, Aug 11, 2004 at 07:04:24PM -0700, William Lee Irwin III wrote:
> > Odd, it was either you or mpm who told me the results. I personally
> > never even tried running the thing. I was merely told some other, prior
> > attempt at doing some kind of spinlock uninlining failed to run, this
> > thing did, and that it shaved that memorable amount off of .text size.
> > I recall I compiled it myself and saw about half as much reduction
> > (120KB instead of 220KB), possibly due to .config or compiler differences.
> > I'll dust things off and so on.
>
> Okay, the results on 2.6.8-rc4 (COOL had a bit of porting, basically
> dropping the hunks associated with spin_lock_flags_string or whatever
> it is). Chose the .config largely to be vaguely deterministic, but had
> to nuke the "System is too big" check in arch/x86_64/boot/tools/build.c.
>
>               text    data     bss     dec     hex filename
> mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
> cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
> C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux

Shit that's quite the variance, which compiler are you using?

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

On Thu, 12 Aug 2004, William Lee Irwin III wrote:
>>>               text    data     bss     dec     hex filename
>>> mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
>>> cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
>>> C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux

On Thu, Aug 12, 2004 at 04:12:55AM -0400, Zwane Mwaikambo wrote:
>> Shit that's quite the variance, which compiler are you using?

On Thu, Aug 12, 2004 at 01:11:02AM -0700, William Lee Irwin III wrote:
> $ gcc --version
> gcc (GCC) 3.3.3 (SuSE Linux)
[...]
> It's the compiler on residue; you (zwane) should be able to log in and
> take a closer look if need be.

The .config in its entirety includes as a MIME attachment.


-- wli

[-- Attachment #2: config.spinlock.gz --]
[-- Type: application/octet-stream, Size: 12625 bytes --]

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Thu, Aug 12, 2004 at 12:20:58AM -0700, William Lee Irwin III wrote:
> Okay, the results on 2.6.8-rc4 (COOL had a bit of porting, basically
> dropping the hunks associated with spin_lock_flags_string or whatever
> it is). Chose the .config largely to be vaguely deterministic, but had
> to nuke the "System is too big" check in arch/x86_64/boot/tools/build.c.
>               text    data     bss     dec     hex filename
> mainline: 20708323        6603052 1878448 29189823        1bd66bf vmlinux
> cool:     20619594        6588166 1878448 29086208        1bbd200 vmlinux
> C-func:   19969264        6583128 1878384 28430776        1b1d1b8 vmlinux
> x86-64, -O2, allyesconfig minus the following:
[...]

Most of this is consolidating preempt_count()++ and preempt_count()--
when CONFIG_PREEMPT=y. Removing CONFIG_PREEMPT from the .config yields:

              text    data     bss     dec     hex filename
mainline: 19973522        6607761 1878448 28459731        1b242d3 vmlinux
cool:     19839487        6585707 1878448 28303642        1afe11a vmlinux
C-func:   19923848        6582771 1878384 28385003        1b11eeb vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, Aug 13, 2004 at 01:01:16AM -0700, William Lee Irwin III wrote:
> Most of this is consolidating preempt_count()++ and preempt_count()--
> when CONFIG_PREEMPT=y. Removing CONFIG_PREEMPT from the .config yields:
>               text    data     bss     dec     hex filename
> mainline: 19973522        6607761 1878448 28459731        1b242d3 vmlinux
> cool:     19839487        6585707 1878448 28303642        1afe11a vmlinux
> C-func:   19923848        6582771 1878384 28385003        1b11eeb vmlinux

Reinlining spin_unlock() yields:
unlock:     19895498        6582746 1878384 28356628        1b0b014 vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, Aug 13, 2004 at 01:01:16AM -0700, William Lee Irwin III wrote:
>> Most of this is consolidating preempt_count()++ and preempt_count()--
>> when CONFIG_PREEMPT=y. Removing CONFIG_PREEMPT from the .config yields:
>>               text    data     bss     dec     hex filename
>> mainline: 19973522        6607761 1878448 28459731        1b242d3 vmlinux
>> cool:     19839487        6585707 1878448 28303642        1afe11a vmlinux
>> C-func:   19923848        6582771 1878384 28385003        1b11eeb vmlinux

On Fri, Aug 13, 2004 at 02:16:40AM -0700, William Lee Irwin III wrote:
> Reinlining spin_unlock() yields:
> unlock:     19895498        6582746 1878384 28356628        1b0b014 vmlinux

Reinlining spin_unlock_irq() also yields:
unlock-irq:   19889858        6582721 1878384 28350963        1b099f3 vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, Aug 13, 2004 at 01:01:16AM -0700, William Lee Irwin III wrote:
>>> Most of this is consolidating preempt_count()++ and preempt_count()--
>>> when CONFIG_PREEMPT=y. Removing CONFIG_PREEMPT from the .config yields:
>>>               text    data     bss     dec     hex filename
>>> mainline: 19973522        6607761 1878448 28459731        1b242d3 vmlinux
>>> cool:     19839487        6585707 1878448 28303642        1afe11a vmlinux
>>> C-func:   19923848        6582771 1878384 28385003        1b11eeb vmlinux

On Fri, Aug 13, 2004 at 02:16:40AM -0700, William Lee Irwin III wrote:
>> Reinlining spin_unlock() yields:
>> unlock:     19895498        6582746 1878384 28356628        1b0b014 vmlinux

On Fri, Aug 13, 2004 at 02:30:02AM -0700, William Lee Irwin III wrote:
> Reinlining spin_unlock_irq() also yields:
> unlock-irq:   19889858        6582721 1878384 28350963        1b099f3 vmlinux

Reinlining read_unlock() also yields:
read-unlock:    19883858        6582674 1878384 28344916        1b08254 vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, Aug 13, 2004 at 01:01:16AM -0700, William Lee Irwin III wrote:
>>>>               text    data     bss     dec     hex filename
>>>> mainline: 19973522        6607761 1878448 28459731        1b242d3 vmlinux
>>>> cool:     19839487        6585707 1878448 28303642        1afe11a vmlinux
>>>> C-func:   19923848        6582771 1878384 28385003        1b11eeb vmlinux

On Fri, Aug 13, 2004 at 02:16:40AM -0700, William Lee Irwin III wrote:
>>> Reinlining spin_unlock() yields:
>>> unlock:    19895498        6582746 1878384 28356628        1b0b014 vmlinux

On Fri, Aug 13, 2004 at 02:30:02AM -0700, William Lee Irwin III wrote:
>> Reinlining spin_unlock_irq() also yields:
>> unlock-irq: 19889858        6582721 1878384 28350963        1b099f3 vmlinux

On Fri, Aug 13, 2004 at 02:46:14AM -0700, William Lee Irwin III wrote:
> Reinlining read_unlock() also yields:
> read-unlock: 19883858        6582674 1878384 28344916        1b08254 vmlinux

Reinlining spin_unlock_irqrestore() also yields:
irqrestore:    19855759        6582442 1878384 28316585        1b013a9 vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, Aug 13, 2004 at 01:01:16AM -0700, William Lee Irwin III wrote:
>>>>>               text    data     bss     dec     hex filename
>>>>> mainline: 19973522        6607761 1878448 28459731        1b242d3 vmlinux
>>>>> cool:     19839487        6585707 1878448 28303642        1afe11a vmlinux
>>>>> C-func:   19923848        6582771 1878384 28385003        1b11eeb vmlinux

On Fri, Aug 13, 2004 at 02:16:40AM -0700, William Lee Irwin III wrote:
>>>> unlock:    19895498        6582746 1878384 28356628        1b0b014 vmlinux

On Fri, Aug 13, 2004 at 02:30:02AM -0700, William Lee Irwin III wrote:
>>> unlock-irq: 19889858        6582721 1878384 28350963        1b099f3 vmlinux

On Fri, Aug 13, 2004 at 02:46:14AM -0700, William Lee Irwin III wrote:
>> read-unlock: 19883858        6582674 1878384 28344916        1b08254 vmlinux

On Fri, Aug 13, 2004 at 03:05:40AM -0700, William Lee Irwin III wrote:
> irqrestore:   19855759        6582442 1878384 28316585        1b013a9 vmlinux

Reinlining read_unlock_irq() also yields:
rdunlockirq:    19855255        6582369 1878384 28316008        1b01168 vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, Aug 13, 2004 at 03:23:34AM -0700, William Lee Irwin III wrote:
> Reinlining read_unlock_irq() also yields:
>               text    data     bss     dec     hex filename
> mainline:    19973522        6607761 1878448 28459731        1b242d3 vmlinux
> cool:        19839487        6585707 1878448 28303642        1afe11a vmlinux
> C-func:      19923848        6582771 1878384 28385003        1b11eeb vmlinux
> unlock:      19895498        6582746 1878384 28356628        1b0b014 vmlinux
> unlock-irq:  19889858        6582721 1878384 28350963        1b099f3 vmlinux
> read-unlock: 19883858        6582674 1878384 28344916        1b08254 vmlinux
> irqrestore:  19855759        6582442 1878384 28316585        1b013a9 vmlinux
> rdunlockirq: 19855255        6582369 1878384 28316008        1b01168 vmlinux

Reinlining read_unlock_irqrestore() also yields:
rdunlckrestor: 19855007        6582236 1878384 28315627        1b00feb vmlinux


-- wli

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[Zwane Mwaikambo]

On Fri, 13 Aug 2004, William Lee Irwin III wrote:

> On Fri, Aug 13, 2004 at 03:23:34AM -0700, William Lee Irwin III wrote:
> > Reinlining read_unlock_irq() also yields:
> >               text    data     bss     dec     hex filename
> > mainline:    19973522        6607761 1878448 28459731        1b242d3 vmlinux
> > cool:        19839487        6585707 1878448 28303642        1afe11a vmlinux
> > C-func:      19923848        6582771 1878384 28385003        1b11eeb vmlinux
> > unlock:      19895498        6582746 1878384 28356628        1b0b014 vmlinux
> > unlock-irq:  19889858        6582721 1878384 28350963        1b099f3 vmlinux
> > read-unlock: 19883858        6582674 1878384 28344916        1b08254 vmlinux
> > irqrestore:  19855759        6582442 1878384 28316585        1b013a9 vmlinux
> > rdunlockirq: 19855255        6582369 1878384 28316008        1b01168 vmlinux
>
> Reinlining read_unlock_irqrestore() also yields:
> rdunlckrestor: 19855007        6582236 1878384 28315627        1b00feb vmlinux

I was meaning to ask before, got ideas for lock profiling with this?

Re: [PATCH][2.6] Completely out of line spinlocks / i386

[William Lee Irwin III]

On Fri, 13 Aug 2004, William Lee Irwin III wrote:
>>               text    data     bss     dec     hex filename
>> mainline:    19973522        6607761 1878448 28459731        1b242d3 vmlinux
>> cool:        19839487        6585707 1878448 28303642        1afe11a vmlinux
>> C-func:      19923848        6582771 1878384 28385003        1b11eeb vmlinux
>> unlock:      19895498        6582746 1878384 28356628        1b0b014 vmlinux
>> unlock-irq:  19889858        6582721 1878384 28350963        1b099f3 vmlinux
>> read-unlock: 19883858        6582674 1878384 28344916        1b08254 vmlinux
>> irqrestore:  19855759        6582442 1878384 28316585        1b013a9 vmlinux
>> rdunlockirq: 19855255        6582369 1878384 28316008        1b01168 vmlinux
>> rdunlckrstr: 19855007        6582236 1878384 28315627        1b00feb vmlinux

On Fri, Aug 13, 2004 at 10:15:45AM -0400, Zwane Mwaikambo wrote:
> I was meaning to ask before, got ideas for lock profiling with this?

I don't have anything concrete, no. I suspect the same comments apply
generically to all architectures. One possible modification would be
for profile_tick() (in current -mm) to check for the text address being
in some ELF section dedicated to out-of-line locking functions and
unwind the stack one frame and account the tick to the caller.

The more problematic aspect of all this is that x86 is unique in its
code footprint for the unlock functions and IRQ masking being so small
as to merit inlining of these things. So the notion of a uniform API
that serves all architectures equally well is out the window.


-- wli