Messages in this thread Patch in this message | | | Date | Fri, 29 Oct 2004 13:22:52 -0400 (EDT) | From | linux-os <> | Subject | Re: Semaphore assembly-code bug |
| |
On Fri, 29 Oct 2004, Linus Torvalds wrote:
> > > On Fri, 29 Oct 2004, linux-os wrote: >> >> Linus, please check this out. > > Yes, I concur. However, I'd suggest changing the "addl $4,%esp" into a > "popl %ecx", which is smaller and apparently faster on some CPU's (ecx > obviously gets immediately overwritten by the next popl). > > Btw, this is another case where we _really_ want "asmlinkage" to mean that > the compiler does not own the argument stack. Is there any chance of > getting a function attribute like that into future versions of gcc? > Richard, Jan, Andi? Or does it already exist somewhere? > > Linus >
Here's a version that uses `leal 4(esp), esp` to add 4 to the stack-pointer. Since this 'address-calculation` is done in an different portion of Intel CPUs, there is some parallel operation that can occur. The 'pop ecx' would access memory and, therefore be slower than simple register operations.
FYI I'm running a kernel with this patch already.
--- linux-2.6.9/arch/i386/kernel/semaphore.c.orig 2004-10-29 13:00:17.961579368 -0400 +++ linux-2.6.9/arch/i386/kernel/semaphore.c 2004-10-29 13:03:35.046617888 -0400 @@ -198,9 +198,11 @@ #endif "pushl %eax\n\t" "pushl %edx\n\t" - "pushl %ecx\n\t" + "pushl %ecx\n\t" // Register to save + "pushl %ecx\n\t" // Passed parameter "call __down\n\t" - "popl %ecx\n\t" + "leal 0x04(%esp), %esp\t\n" // Bypass corrupted parameter + "popl %ecx\n\t" // Restore original "popl %edx\n\t" "popl %eax\n\t" #if defined(CONFIG_FRAME_POINTER) @@ -220,9 +222,11 @@ "movl %esp,%ebp\n\t" #endif "pushl %edx\n\t" - "pushl %ecx\n\t" + "pushl %ecx\n\t" // Save register + "pushl %ecx\n\t" // Passed parameter "call __down_interruptible\n\t" - "popl %ecx\n\t" + "leal 0x04(%esp), %esp\n\t" // Bypass corrupted parameter + "popl %ecx\n\t" // Restore register "popl %edx\n\t" #if defined(CONFIG_FRAME_POINTER) "movl %ebp,%esp\n\t" @@ -241,9 +245,11 @@ "movl %esp,%ebp\n\t" #endif "pushl %edx\n\t" - "pushl %ecx\n\t" + "pushl %ecx\n\t" // Save register + "pushl %ecx\n\t" // Passed parameter "call __down_trylock\n\t" - "popl %ecx\n\t" + "leal 0x04(%esp), %esp\n\t" // Bypass corrupted parameter + "popl %ecx\n\t" // Restore register "popl %edx\n\t" #if defined(CONFIG_FRAME_POINTER) "movl %ebp,%esp\n\t" @@ -259,9 +265,11 @@ "__up_wakeup:\n\t" "pushl %eax\n\t" "pushl %edx\n\t" - "pushl %ecx\n\t" + "pushl %ecx\n\t" // Save register + "pushl %ecx\n\t" // Passed parameter "call __up\n\t" - "popl %ecx\n\t" + "leal 0x04(%esp), %esp\n\t" // Bypass corrupted parameter + "popl %ecx\n\t" // Restore register "popl %edx\n\t" "popl %eax\n\t" "ret"
Cheers, Dick Johnson Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips). Notice : All mail here is now cached for review by John Ashcroft. 98.36% of all statistics are fiction. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |