* [PATCH][2.6] Allow x86_64 to reenable interrupts on contention
@ 2004-07-27 9:29 Zwane Mwaikambo
2004-07-27 11:26 ` Andi Kleen
0 siblings, 1 reply; 16+ messages in thread
From: Zwane Mwaikambo @ 2004-07-27 9:29 UTC (permalink / raw)
To: Linux Kernel; +Cc: Andrew Morton, Andi Kleen
This is a follow up to the previous patches for ia64 and i386, it will
allow x86_64 to reenable interrupts during contested locks depending on
previous interrupt enable status. It has been runtime and compile tested
on UP and 2x SMP Linux-tiny/x86_64.
Signed-off-by: Zwane Mwaikambo <zwane@fsmlabs.com>
--- linux-2.6-amd64/include/asm-x86_64/spinlock.h.orig 2004-07-27 03:32:18.689949016 -0400
+++ linux-2.6-amd64/include/asm-x86_64/spinlock.h 2004-07-27 03:45:19.610231088 -0400
@@ -41,7 +41,6 @@ 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))
-#define _raw_spin_lock_flags(lock, flags) _raw_spin_lock(lock)
#define spin_lock_string \
"\n1:\t" \
@@ -55,6 +54,23 @@ typedef struct {
"jmp 1b\n" \
LOCK_SECTION_END
+#define spin_lock_string_flags \
+ "\n1:\t" \
+ "lock ; decb %0\n\t" \
+ "js 2f\n\t" \
+ LOCK_SECTION_START("") \
+ "2:\t" \
+ "test $0x200, %1\n\t" \
+ "jz 3f\n\t" \
+ "sti\n\t" \
+ "3:\t" \
+ "rep;nop\n\t" \
+ "cmpb $0, %0\n\t" \
+ "jle 3b\n\t" \
+ "cli\n\t" \
+ "jmp 1b\n" \
+ LOCK_SECTION_END
+
/*
* This works. Despite all the confusion.
* (except on PPro SMP or if we are using OOSTORE)
@@ -125,6 +141,20 @@ printk("eip: %p\n", &&here);
:"=m" (lock->lock) : : "memory");
}
+static inline void _raw_spin_lock_flags (spinlock_t *lock, unsigned long flags)
+{
+#ifdef CONFIG_DEBUG_SPINLOCK
+ __label__ here;
+here:
+ if (unlikely(lock->magic != SPINLOCK_MAGIC)) {
+ printk("eip: %p\n", &&here);
+ BUG();
+ }
+#endif
+ __asm__ __volatile__(spin_lock_string_flags
+ :"=m" (lock->lock) : "r" (flags) : "memory");
+}
+
/*
* Read-write spinlocks, allowing multiple readers
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 9:29 [PATCH][2.6] Allow x86_64 to reenable interrupts on contention Zwane Mwaikambo @ 2004-07-27 11:26 ` Andi Kleen 2004-07-27 14:31 ` Zwane Mwaikambo 0 siblings, 1 reply; 16+ messages in thread From: Andi Kleen @ 2004-07-27 11:26 UTC (permalink / raw) To: Zwane Mwaikambo; +Cc: linux-kernel, akpm On Tue, 27 Jul 2004 05:29:10 -0400 (EDT) Zwane Mwaikambo <zwane@linuxpower.ca> wrote: > This is a follow up to the previous patches for ia64 and i386, it will > allow x86_64 to reenable interrupts during contested locks depending on > previous interrupt enable status. It has been runtime and compile tested > on UP and 2x SMP Linux-tiny/x86_64. This will likely increase code size. Do you have numbers by how much? And is it really worth it? -Andi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 11:26 ` Andi Kleen @ 2004-07-27 14:31 ` Zwane Mwaikambo 2004-07-27 15:37 ` Andi Kleen 2004-07-27 19:01 ` Andrew Morton 0 siblings, 2 replies; 16+ messages in thread From: Zwane Mwaikambo @ 2004-07-27 14:31 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel, akpm On Tue, 27 Jul 2004, Andi Kleen wrote: > On Tue, 27 Jul 2004 05:29:10 -0400 (EDT) > Zwane Mwaikambo <zwane@linuxpower.ca> wrote: > > > This is a follow up to the previous patches for ia64 and i386, it will > > allow x86_64 to reenable interrupts during contested locks depending on > > previous interrupt enable status. It has been runtime and compile tested > > on UP and 2x SMP Linux-tiny/x86_64. > > This will likely increase code size. Do you have numbers by how much? And is it > really worth it? Yes there is a growth; text data bss dec hex filename 3655358 1340511 486128 5481997 53a60d vmlinux-after 3648445 1340511 486128 5475084 538b0c vmlinux-before And this was on i386; text data bss dec hex filename 2628024 921731 0 3549755 362a3b vmlinux-after 2621369 921731 0 3543100 36103c vmlinux-before Keith Owens managed to get increased throughput as the original patch was driven by poor performance from a workload. I think it's worth it just for the reduced interrupt latency, the code size issue can also be taken care of, but that requires benchmarking as the change is a bit more drastic. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 14:31 ` Zwane Mwaikambo @ 2004-07-27 15:37 ` Andi Kleen 2004-07-27 16:36 ` Ricky Beam 2004-07-28 1:38 ` Zwane Mwaikambo 2004-07-27 19:01 ` Andrew Morton 1 sibling, 2 replies; 16+ messages in thread From: Andi Kleen @ 2004-07-27 15:37 UTC (permalink / raw) To: Zwane Mwaikambo; +Cc: linux-kernel, akpm On Tue, 27 Jul 2004 10:31:27 -0400 (EDT) Zwane Mwaikambo <zwane@linuxpower.ca> wrote: > On Tue, 27 Jul 2004, Andi Kleen wrote: > > > On Tue, 27 Jul 2004 05:29:10 -0400 (EDT) > > Zwane Mwaikambo <zwane@linuxpower.ca> wrote: > > > > > This is a follow up to the previous patches for ia64 and i386, it will > > > allow x86_64 to reenable interrupts during contested locks depending on > > > previous interrupt enable status. It has been runtime and compile tested > > > on UP and 2x SMP Linux-tiny/x86_64. > > > > This will likely increase code size. Do you have numbers by how much? And is it > > really worth it? > > Yes there is a growth; > > text data bss dec hex filename > 3655358 1340511 486128 5481997 53a60d vmlinux-after > 3648445 1340511 486128 5475084 538b0c vmlinux-before That's significant. > > And this was on i386; > > text data bss dec hex filename > 2628024 921731 0 3549755 362a3b vmlinux-after > 2621369 921731 0 3543100 36103c vmlinux-before > > Keith Owens managed to get increased throughput as the original patch was > driven by poor performance from a workload. I think it's worth it just for What workload was that? I am not sure it is a good idea to "fix" workloads by making spinlocks better. It is likely better to just fix the locking for that workload, that would likely improve the workload a lot more. > the reduced interrupt latency, the code size issue can also be taken care > of, but that requires benchmarking as the change is a bit more drastic. Do you have numbers on that? Frankly if someone is spinning on irq disabled locks for a long time they should just fix their code. -Andi ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 15:37 ` Andi Kleen @ 2004-07-27 16:36 ` Ricky Beam 2004-07-28 8:47 ` Paul Jackson 2004-07-28 1:38 ` Zwane Mwaikambo 1 sibling, 1 reply; 16+ messages in thread From: Ricky Beam @ 2004-07-27 16:36 UTC (permalink / raw) To: Andi Kleen; +Cc: Linux Kernel Mail List On Tue, 27 Jul 2004, Andi Kleen wrote: >> Yes there is a growth; >> >> text data bss dec hex filename >> 3655358 1340511 486128 5481997 53a60d vmlinux-after >> 3648445 1340511 486128 5475084 538b0c vmlinux-before (diff: 6,913bytes, 0.19% increase) >That's significant. 6,913bytes is "significant"? *Now*, after a decade of bloat, you want to pinch pennies? (Do I need to start the flame war(s) again? Need I mention "numcpus"?) >> 2628024 921731 0 3549755 362a3b vmlinux-after >> 2621369 921731 0 3543100 36103c vmlinux-before (diff: 6,655bytes, 0.25% increase) --Ricky ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 16:36 ` Ricky Beam @ 2004-07-28 8:47 ` Paul Jackson 0 siblings, 0 replies; 16+ messages in thread From: Paul Jackson @ 2004-07-28 8:47 UTC (permalink / raw) To: Ricky Beam; +Cc: ak, linux-kernel Ricky: > Need I mention "numcpus"? Could some kind soul provide a link to that flamage? It sounds like a piece of lkml history before my time, but in an area that I work and should know of. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.650.933.1373 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 15:37 ` Andi Kleen 2004-07-27 16:36 ` Ricky Beam @ 2004-07-28 1:38 ` Zwane Mwaikambo 1 sibling, 0 replies; 16+ messages in thread From: Zwane Mwaikambo @ 2004-07-28 1:38 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel, akpm On Tue, 27 Jul 2004, Andi Kleen wrote: > > the reduced interrupt latency, the code size issue can also be taken care > > of, but that requires benchmarking as the change is a bit more drastic. > > Do you have numbers on that? Frankly if someone is spinning on irq disabled > locks for a long time they should just fix their code. It wasn't i who saw the contention but something Keith ran into, the contention probably wasn't bad enough to require serious locking changes. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 14:31 ` Zwane Mwaikambo 2004-07-27 15:37 ` Andi Kleen @ 2004-07-27 19:01 ` Andrew Morton 2004-07-28 0:14 ` William Lee Irwin III 1 sibling, 1 reply; 16+ messages in thread From: Andrew Morton @ 2004-07-27 19:01 UTC (permalink / raw) To: Zwane Mwaikambo; +Cc: ak, linux-kernel Zwane Mwaikambo <zwane@linuxpower.ca> wrote: > > On Tue, 27 Jul 2004, Andi Kleen wrote: > > > On Tue, 27 Jul 2004 05:29:10 -0400 (EDT) > > Zwane Mwaikambo <zwane@linuxpower.ca> wrote: > > > > > This is a follow up to the previous patches for ia64 and i386, it will > > > allow x86_64 to reenable interrupts during contested locks depending on > > > previous interrupt enable status. It has been runtime and compile tested > > > on UP and 2x SMP Linux-tiny/x86_64. > > > > This will likely increase code size. Do you have numbers by how much? And is it > > really worth it? > > Yes there is a growth; > > text data bss dec hex filename > 3655358 1340511 486128 5481997 53a60d vmlinux-after > 3648445 1340511 486128 5475084 538b0c vmlinux-before The growth is all in the out-of-line section, so there should be no significant additional icache pressure. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-27 19:01 ` Andrew Morton @ 2004-07-28 0:14 ` William Lee Irwin III 2004-07-28 0:35 ` Keith Owens 0 siblings, 1 reply; 16+ messages in thread From: William Lee Irwin III @ 2004-07-28 0:14 UTC (permalink / raw) To: Andrew Morton; +Cc: Zwane Mwaikambo, ak, linux-kernel On Tue, 27 Jul 2004, Andi Kleen wrote: >>> This will likely increase code size. Do you have numbers by how >>> much? And is it really worth it? Zwane Mwaikambo <zwane@linuxpower.ca> wrote: >> Yes there is a growth; >> text data bss dec hex filename >> 3655358 1340511 486128 5481997 53a60d vmlinux-after >> 3648445 1340511 486128 5475084 538b0c vmlinux-before On Tue, Jul 27, 2004 at 12:01:25PM -0700, Andrew Morton wrote: > The growth is all in the out-of-line section, so there should be no > significant additional icache pressure. There are also flash and similar absolute space footprints to consider. Experiments seem to suggest that consolidating the lock sections and other spinlock code can reduce kernel image size by as much as 220KB on ia32 with no performance impact (rigorous benchmarks still in progress). That said, I agree with zwane's interrupt enablement change. -- wli ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 0:14 ` William Lee Irwin III @ 2004-07-28 0:35 ` Keith Owens 2004-07-28 0:40 ` William Lee Irwin III 2004-07-28 1:30 ` Zwane Mwaikambo 0 siblings, 2 replies; 16+ messages in thread From: Keith Owens @ 2004-07-28 0:35 UTC (permalink / raw) To: William Lee Irwin III; +Cc: Andrew Morton, Zwane Mwaikambo, ak, linux-kernel On Tue, 27 Jul 2004 17:14:15 -0700, William Lee Irwin III <wli@holomorphy.com> wrote: >On Tue, 27 Jul 2004, Andi Kleen wrote: >>>> This will likely increase code size. Do you have numbers by how >>>> much? And is it really worth it? > >Zwane Mwaikambo <zwane@linuxpower.ca> wrote: >>> Yes there is a growth; >>> text data bss dec hex filename >>> 3655358 1340511 486128 5481997 53a60d vmlinux-after >>> 3648445 1340511 486128 5475084 538b0c vmlinux-before > >On Tue, Jul 27, 2004 at 12:01:25PM -0700, Andrew Morton wrote: >> The growth is all in the out-of-line section, so there should be no >> significant additional icache pressure. > >There are also flash and similar absolute space footprints to consider. >Experiments seem to suggest that consolidating the lock sections and >other spinlock code can reduce kernel image size by as much as 220KB on >ia32 with no performance impact (rigorous benchmarks still in progress). I consolidated the spinlock contention path to a single routine on ia64, with big space savings. The problem with the ia64 consolidation was backtracing through a contended lock; the ia64 unwind API is not designed for code that is shared between multiple code paths but uses non-standard entry and exit conventions. In the end, David Mosberger did a patch to gcc to do lightweight calls to the out of line contention code, just to get reliable backtraces. kdb has workarounds for backtracing through ia64 contended locks when the kernel is built with older versions of gcc. gdb (and hence kgdb) has no idea about the special out of line code. Mind you, the same is true right now with the out of line i386 code, you need special heuristics to backtrace the existing spinlock code reliably. That will only get worse with Zwane's patch, interrupts can now occur in the out of line code. Are you are planning to consolidate the out of line code for i386? Is there a patch (even work in progress) so I can start thinking about doing reliable backtraces? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 0:35 ` Keith Owens @ 2004-07-28 0:40 ` William Lee Irwin III 2004-07-28 1:48 ` Keith Owens 2004-07-28 1:30 ` Zwane Mwaikambo 1 sibling, 1 reply; 16+ messages in thread From: William Lee Irwin III @ 2004-07-28 0:40 UTC (permalink / raw) To: Keith Owens; +Cc: Andrew Morton, Zwane Mwaikambo, ak, linux-kernel On Tue, 27 Jul 2004 17:14:15 -0700, >> There are also flash and similar absolute space footprints to consider. >> Experiments seem to suggest that consolidating the lock sections and >> other spinlock code can reduce kernel image size by as much as 220KB on >> ia32 with no performance impact (rigorous benchmarks still in progress). On Wed, Jul 28, 2004 at 10:35:08AM +1000, Keith Owens wrote: > I consolidated the spinlock contention path to a single routine on > ia64, with big space savings. The problem with the ia64 consolidation > was backtracing through a contended lock; the ia64 unwind API is not > designed for code that is shared between multiple code paths but uses > non-standard entry and exit conventions. In the end, David Mosberger > did a patch to gcc to do lightweight calls to the out of line > contention code, just to get reliable backtraces. > kdb has workarounds for backtracing through ia64 contended locks when > the kernel is built with older versions of gcc. gdb (and hence kgdb) > has no idea about the special out of line code. Mind you, the same is > true right now with the out of line i386 code, you need special > heuristics to backtrace the existing spinlock code reliably. That will > only get worse with Zwane's patch, interrupts can now occur in the out > of line code. It's good to know there is a precedent and that the backtrace issue has been looked at on other architectures. On Wed, Jul 28, 2004 at 10:35:08AM +1000, Keith Owens wrote: > Are you are planning to consolidate the out of line code for i386? Is > there a patch (even work in progress) so I can start thinking about > doing reliable backtraces? The experiments were carried out using the standard calling convention. We may investigate less standard calling conventions, but they should actually already be there given __write_lock_failed/__read_lock_failed. i.e. if reliable backtraces are going to be an issue they should already be an issue for rwlocks. -- wli ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 0:40 ` William Lee Irwin III @ 2004-07-28 1:48 ` Keith Owens 2004-07-28 2:21 ` William Lee Irwin III 0 siblings, 1 reply; 16+ messages in thread From: Keith Owens @ 2004-07-28 1:48 UTC (permalink / raw) To: William Lee Irwin III; +Cc: Andrew Morton, Zwane Mwaikambo, ak, linux-kernel On Tue, 27 Jul 2004 17:40:30 -0700, William Lee Irwin III <wli@holomorphy.com> wrote: >It's good to know there is a precedent and that the backtrace issue has >been looked at on other architectures. > >On Wed, Jul 28, 2004 at 10:35:08AM +1000, Keith Owens wrote: >> Are you are planning to consolidate the out of line code for i386? Is >> there a patch (even work in progress) so I can start thinking about >> doing reliable backtraces? > >The experiments were carried out using the standard calling convention. >We may investigate less standard calling conventions, but they should >actually already be there given __write_lock_failed/__read_lock_failed. >i.e. if reliable backtraces are going to be an issue they should >already be an issue for rwlocks. rwlocks are already a issue on i386, but not a big one. The return address from __write_lock_failed/__read_lock_failed is pointing into the out of line code, not the code that really took the rwlock. The nearest label is LOCK_SECTION_NAME, i.e. .text.lock.KBUILD_BASENAME. A backtrace through a contended rwlock on i386 looks like this interrupt __write_lock_failed/__read_lock_failed .text.lock.KBUILD_BASENAME caller of routine that took the lock ... when it should really be interrupt __write_lock_failed/__read_lock_failed .text.lock.KBUILD_BASENAME routine that took the look <=== missing information caller of routine that took the lock ... IOW we only get the name of the object, not the function within the object that took the lock. i386 gets away with this because .text.lock.KBUILD_BASENAME is usually enough information to determine which lock is the problem, and the i386 backtrace algorithm has enough redundancy to get back in sync for the rest of the trace, even with the missing function entry. OTOH, ia64 unwind is incredibly sensitive to the exact instruction pointer and there is zero redundancy in the unwind data. If the return ip is not known to the unwind code, then the ia64 unwinder cannot backtrace correctly. Which meant that I had to get the ia64 out of line code exactly right, close enough was not good enough. With Zwane's patch, any contended spinlock on i386 will look like rwsem, it will be missing the routine that took the look. Good enough for most cases. kdb does unwind through out of line spinlock code exactly right, simply because I added extra heuristics to kdb to cope with this special case. When people complain that the kdb i386 backtrace code is too messy, they are really saying that they do not care about getting exact data for all the hand crafted assembler in the kernel. BTW, if anybody is planning to switch to dwarf for debugging the i386 kernel then you _must_ have valid dwarf data for the out of line code. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 1:48 ` Keith Owens @ 2004-07-28 2:21 ` William Lee Irwin III 2004-07-28 2:24 ` David S. Miller 0 siblings, 1 reply; 16+ messages in thread From: William Lee Irwin III @ 2004-07-28 2:21 UTC (permalink / raw) To: Keith Owens; +Cc: Andrew Morton, Zwane Mwaikambo, ak, linux-kernel On Wed, Jul 28, 2004 at 11:48:05AM +1000, Keith Owens wrote: > rwlocks are already a issue on i386, but not a big one. The return > address from __write_lock_failed/__read_lock_failed is pointing into > the out of line code, not the code that really took the rwlock. The > nearest label is LOCK_SECTION_NAME, i.e. .text.lock.KBUILD_BASENAME. A > backtrace through a contended rwlock on i386 looks like this > interrupt > __write_lock_failed/__read_lock_failed > .text.lock.KBUILD_BASENAME > caller of routine that took the lock > ... > when it should really be > interrupt > __write_lock_failed/__read_lock_failed > .text.lock.KBUILD_BASENAME > routine that took the look <=== missing information > caller of routine that took the lock > ... > IOW we only get the name of the object, not the function within the > object that took the lock. i386 gets away with this because > .text.lock.KBUILD_BASENAME is usually enough information to determine > which lock is the problem, and the i386 backtrace algorithm has enough > redundancy to get back in sync for the rest of the trace, even with the > missing function entry. IMHO the out-of-line call to the rwlock contention code in the lock section is completely worthless bloat and rwlocks should just call the out-of-line contention case directly from the inline path. The patch I used to determine the maximum aggregate text size reduction achievable literally made normal C function entrypoints for spin_lock(), read_lock(), and all the various spinlocking functions for spinlocks and rwlocks, moving all of the spinlocking code from include/linux/spinlock.h to a new kernel/spinlock.c, using the normal inline _raw_spin_lock()/etc. to avoid unnecessary call frames. This is what resulted in the aggregate kernel size reduction of 220KB, i.e. almost 1/4 of 1MB, and had no effect on performance, positive or negative, in some non-rigorous benchmarking. This obviously interoperates quite well with backtracing and so on, but doesn't appear to be under consideration at the moment, and may also have issues with architectures (e.g. sparc/sparc64) which need the interrupt disablement in e.g. spin_lock_irqsave() to be done in the same call frame as the spin_unlock_irqrestore() etc. On Wed, Jul 28, 2004 at 11:48:05AM +1000, Keith Owens wrote: > OTOH, ia64 unwind is incredibly sensitive to the exact instruction > pointer and there is zero redundancy in the unwind data. If the return > ip is not known to the unwind code, then the ia64 unwinder cannot > backtrace correctly. Which meant that I had to get the ia64 out of > line code exactly right, close enough was not good enough. > With Zwane's patch, any contended spinlock on i386 will look like > rwsem, it will be missing the routine that took the look. Good enough > for most cases. > kdb does unwind through out of line spinlock code exactly right, simply > because I added extra heuristics to kdb to cope with this special case. > When people complain that the kdb i386 backtrace code is too messy, > they are really saying that they do not care about getting exact data > for all the hand crafted assembler in the kernel. > BTW, if anybody is planning to switch to dwarf for debugging the i386 > kernel then you _must_ have valid dwarf data for the out of line code. This is a different issue. That should definitely be looked at for the consolidated contention case out-of-line strategies now being considered or others involving nonstandard calling conventions. IMHO we are truly best off using normal calling conventions to invoke the out-of-line cases regardless of their role. Confusing debuggers and the extra bloat to push the return address on the stack and the extra jump are just not worth it. -- wli ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 2:21 ` William Lee Irwin III @ 2004-07-28 2:24 ` David S. Miller 2004-07-28 8:20 ` William Lee Irwin III 0 siblings, 1 reply; 16+ messages in thread From: David S. Miller @ 2004-07-28 2:24 UTC (permalink / raw) To: William Lee Irwin III; +Cc: kaos, akpm, zwane, ak, linux-kernel On Tue, 27 Jul 2004 19:21:31 -0700 William Lee Irwin III <wli@holomorphy.com> wrote: > and may also have issues with > architectures (e.g. sparc/sparc64) which need the interrupt disablement > in e.g. spin_lock_irqsave() to be done in the same call frame as the > spin_unlock_irqrestore() etc. This only was a problem on sparc, and it no longer exists at all in 2.6.x kernels. It was too much of a pain to keep teaching people that violated this, and it resulted in some contorted code as well. So don't worry about this at all in 2.6.x and later. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 2:24 ` David S. Miller @ 2004-07-28 8:20 ` William Lee Irwin III 0 siblings, 0 replies; 16+ messages in thread From: William Lee Irwin III @ 2004-07-28 8:20 UTC (permalink / raw) To: David S. Miller; +Cc: kaos, akpm, zwane, ak, linux-kernel On Tue, 27 Jul 2004 19:21:31 -0700 William Lee Irwin III wrote: >> and may also have issues with >> architectures (e.g. sparc/sparc64) which need the interrupt disablement >> in e.g. spin_lock_irqsave() to be done in the same call frame as the >> spin_unlock_irqrestore() etc. On Tue, Jul 27, 2004 at 07:24:54PM -0700, David S. Miller wrote: > This only was a problem on sparc, and it no longer exists at all > in 2.6.x kernels. It was too much of a pain to keep teaching > people that violated this, and it resulted in some contorted > code as well. > So don't worry about this at all in 2.6.x and later. Thanks; that explains whether it worked by coincidence or design. No idea why I thought sparc64 did similar. -- wli ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][2.6] Allow x86_64 to reenable interrupts on contention 2004-07-28 0:35 ` Keith Owens 2004-07-28 0:40 ` William Lee Irwin III @ 2004-07-28 1:30 ` Zwane Mwaikambo 1 sibling, 0 replies; 16+ messages in thread From: Zwane Mwaikambo @ 2004-07-28 1:30 UTC (permalink / raw) To: Keith Owens Cc: William Lee Irwin III, Andrew Morton, Andi Kleen, Linux Kernel, Matt Mackall On Wed, 28 Jul 2004, Keith Owens wrote: > I consolidated the spinlock contention path to a single routine on > ia64, with big space savings. The problem with the ia64 consolidation > was backtracing through a contended lock; the ia64 unwind API is not > designed for code that is shared between multiple code paths but uses > non-standard entry and exit conventions. In the end, David Mosberger > did a patch to gcc to do lightweight calls to the out of line > contention code, just to get reliable backtraces. > > kdb has workarounds for backtracing through ia64 contended locks when > the kernel is built with older versions of gcc. gdb (and hence kgdb) > has no idea about the special out of line code. Mind you, the same is > true right now with the out of line i386 code, you need special > heuristics to backtrace the existing spinlock code reliably. That will > only get worse with Zwane's patch, interrupts can now occur in the out > of line code. > > Are you are planning to consolidate the out of line code for i386? Is > there a patch (even work in progress) so I can start thinking about > doing reliable backtraces? Another problem i ran into was profiling contended locks. Here is a patch (against 2.6.8-rc2, should apply to recent -mm) which does completely out of line spinlocks on i386 with shared lock text, it's stable and has been tested on 32x. This patch can be modified so that we have the entire spinlock text out of line or simply during contention, the former has significantly more space savings, something like; text data bss dec hex filename 5437496 831862 323792 6593150 649a7e vmlinux-ool 5449254 834525 323792 6607571 64d2d3 vmlinux-before 5431258 831862 323792 6586912 648220 vmlinux-cool -ool is only the contended path out of line in shared text, -before is mainline and -cool is completely out of line shared spinlock text. Index: linux-2.6.8-rc2/arch/i386/kernel/i386_ksyms.c =================================================================== RCS file: /home/cvsroot/linux-2.6.8-rc2/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-rc2/arch/i386/kernel/i386_ksyms.c 20 Jul 2004 18:14:54 -0000 1.1.1.1 +++ linux-2.6.8-rc2/arch/i386/kernel/i386_ksyms.c 28 Jul 2004 00:29:05 -0000 @@ -49,6 +49,14 @@ EXPORT_SYMBOL(default_idle); #ifdef CONFIG_SMP extern void FASTCALL( __write_lock_failed(rwlock_t *rw)); extern void FASTCALL( __read_lock_failed(rwlock_t *rw)); +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) Index: linux-2.6.8-rc2/arch/i386/lib/Makefile =================================================================== RCS file: /home/cvsroot/linux-2.6.8-rc2/arch/i386/lib/Makefile,v retrieving revision 1.1.1.1 diff -u -p -B -r1.1.1.1 Makefile --- linux-2.6.8-rc2/arch/i386/lib/Makefile 20 Jul 2004 18:14:54 -0000 1.1.1.1 +++ linux-2.6.8-rc2/arch/i386/lib/Makefile 28 Jul 2004 00:29:05 -0000 @@ -6,5 +6,6 @@ lib-y = checksum.o delay.o usercopy.o getuser.o memcpy.o strstr.o \ bitops.o +lib-$(CONFIG_SMP) += spinlock.o lib-$(CONFIG_X86_USE_3DNOW) += mmx.o lib-$(CONFIG_HAVE_DEC_LOCK) += dec_and_lock.o Index: linux-2.6.8-rc2/arch/i386/lib/spinlock.c =================================================================== RCS file: linux-2.6.8-rc2/arch/i386/lib/spinlock.c diff -N linux-2.6.8-rc2/arch/i386/lib/spinlock.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ linux-2.6.8-rc2/arch/i386/lib/spinlock.c 28 Jul 2004 00:29:05 -0000 @@ -0,0 +1,53 @@ +#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" + "ret\n" +); + +asm (PROC(__spin_lock_loop_flags) + "lock; decb (%eax)\n\t" + "js 1f\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" + "ret\n\t" +); + +asm (PROC(__spin_lock_loop) + "lock; decb (%eax)\n\t" + "js 1f\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-rc2/include/asm-i386/spinlock.h =================================================================== RCS file: /home/cvsroot/linux-2.6.8-rc2/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-rc2/include/asm-i386/spinlock.h 20 Jul 2004 18:15:14 -0000 1.1.1.1 +++ linux-2.6.8-rc2/include/asm-i386/spinlock.h 28 Jul 2004 00:37:44 -0000 @@ -43,34 +43,25 @@ 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)) -#define spin_lock_string \ - "\n1:\t" \ - "lock ; decb %0\n\t" \ - "js 2f\n" \ - LOCK_SECTION_START("") \ - "2:\t" \ - "rep;nop\n\t" \ - "cmpb $0,%0\n\t" \ - "jle 2b\n\t" \ - "jmp 1b\n" \ - LOCK_SECTION_END - -#define spin_lock_string_flags \ - "\n1:\t" \ - "lock ; decb %0\n\t" \ - "js 2f\n\t" \ - LOCK_SECTION_START("") \ - "2:\t" \ - "testl $0x200, %1\n\t" \ - "jz 3f\n\t" \ - "sti\n\t" \ - "3:\t" \ - "rep;nop\n\t" \ - "cmpb $0, %0\n\t" \ - "jle 3b\n\t" \ - "cli\n\t" \ - "jmp 1b\n" \ - LOCK_SECTION_END +#if 0 + #define spin_lock_string \ + "lock ; decb (%0)\n\t" \ + "jns 1f\n\t" \ + "call __spin_lock_failed\n\t" \ + "1:\n\t" + + #define spin_lock_string_flags \ + "lock ; decb (%0)\n\t" \ + "jns 1f\n\t" \ + "call __spin_lock_failed_flags\n\t" \ + "1:\n\t" +#else + #define spin_lock_string \ + "call __spin_lock_loop\n\t" + + #define spin_lock_string_flags \ + "call __spin_lock_loop_flags\n\t" +#endif /* * This works. Despite all the confusion. @@ -139,7 +130,7 @@ here: #endif __asm__ __volatile__( spin_lock_string - :"=m" (lock->lock) : : "memory"); + : : "a" (&lock->lock) : "memory"); } static inline void _raw_spin_lock_flags (spinlock_t *lock, unsigned long flags) @@ -154,7 +145,7 @@ here: #endif __asm__ __volatile__( spin_lock_string_flags - :"=m" (lock->lock) : "r" (flags) : "memory"); + : : "a" (&lock->lock), "b" (flags) : "memory"); } /* ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2004-07-28 8:47 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-07-27 9:29 [PATCH][2.6] Allow x86_64 to reenable interrupts on contention Zwane Mwaikambo 2004-07-27 11:26 ` Andi Kleen 2004-07-27 14:31 ` Zwane Mwaikambo 2004-07-27 15:37 ` Andi Kleen 2004-07-27 16:36 ` Ricky Beam 2004-07-28 8:47 ` Paul Jackson 2004-07-28 1:38 ` Zwane Mwaikambo 2004-07-27 19:01 ` Andrew Morton 2004-07-28 0:14 ` William Lee Irwin III 2004-07-28 0:35 ` Keith Owens 2004-07-28 0:40 ` William Lee Irwin III 2004-07-28 1:48 ` Keith Owens 2004-07-28 2:21 ` William Lee Irwin III 2004-07-28 2:24 ` David S. Miller 2004-07-28 8:20 ` William Lee Irwin III 2004-07-28 1:30 ` Zwane Mwaikambo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox