lkml.org 
[lkml]   [2004]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: 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/

\
 
 \ /
  Last update: 2005-03-22 14:07    [W:0.182 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site