Messages in this thread Patch in this message | | | Date | Fri, 29 Oct 2004 12:20:58 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: Semaphore assembly-code bug |
| |
Here's a totally untested patch to make the semaphores use "fastcall" instead of "asmlinkage", and thus pass the argument in %eax instead of on the stack. Does it work? I have no idea. If it does, it should fix the particular bug that started this thread..
Linus
--- ===== arch/i386/kernel/semaphore.c 1.10 vs edited ===== --- 1.10/arch/i386/kernel/semaphore.c 2004-04-12 10:53:59 -07:00 +++ edited/arch/i386/kernel/semaphore.c 2004-10-29 12:19:22 -07:00 @@ -49,12 +49,12 @@ * we cannot lose wakeup events. */ -asmlinkage void __up(struct semaphore *sem) +fastcall void __up(struct semaphore *sem) { wake_up(&sem->wait); } -asmlinkage void __sched __down(struct semaphore * sem) +fastcall void __sched __down(struct semaphore * sem) { struct task_struct *tsk = current; DECLARE_WAITQUEUE(wait, tsk); @@ -91,7 +91,7 @@ tsk->state = TASK_RUNNING; } -asmlinkage int __sched __down_interruptible(struct semaphore * sem) +fastcall int __sched __down_interruptible(struct semaphore * sem) { int retval = 0; struct task_struct *tsk = current; @@ -154,7 +154,7 @@ * single "cmpxchg" without failure cases, * but then it wouldn't work on a 386. */ -asmlinkage int __down_trylock(struct semaphore * sem) +fastcall int __down_trylock(struct semaphore * sem) { int sleepers; unsigned long flags; @@ -183,9 +183,9 @@ * need to convert that sequence back into the C sequence when * there is contention on the semaphore. * - * %ecx contains the semaphore pointer on entry. Save the C-clobbered - * registers (%eax, %edx and %ecx) except %eax when used as a return - * value.. + * %eax contains the semaphore pointer on entry. Save the C-clobbered + * registers (%eax, %edx and %ecx) except %eax whish is either a return + * value or just clobbered.. */ asm( ".section .sched.text\n" @@ -196,13 +196,11 @@ "pushl %ebp\n\t" "movl %esp,%ebp\n\t" #endif - "pushl %eax\n\t" "pushl %edx\n\t" "pushl %ecx\n\t" "call __down\n\t" "popl %ecx\n\t" "popl %edx\n\t" - "popl %eax\n\t" #if defined(CONFIG_FRAME_POINTER) "movl %ebp,%esp\n\t" "popl %ebp\n\t" @@ -257,13 +255,11 @@ ".align 4\n" ".globl __up_wakeup\n" "__up_wakeup:\n\t" - "pushl %eax\n\t" "pushl %edx\n\t" "pushl %ecx\n\t" "call __up\n\t" "popl %ecx\n\t" "popl %edx\n\t" - "popl %eax\n\t" "ret" ); ===== include/asm-i386/linkage.h 1.4 vs edited ===== --- 1.4/include/asm-i386/linkage.h 2004-10-16 18:24:37 -07:00 +++ edited/include/asm-i386/linkage.h 2004-10-29 11:32:18 -07:00 @@ -1,7 +1,7 @@ #ifndef __ASM_LINKAGE_H #define __ASM_LINKAGE_H -#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0))) +#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(3))) #define FASTCALL(x) x __attribute__((regparm(3))) #define fastcall __attribute__((regparm(3))) ===== include/asm-i386/semaphore.h 1.9 vs edited ===== --- 1.9/include/asm-i386/semaphore.h 2004-08-27 00:02:38 -07:00 +++ edited/include/asm-i386/semaphore.h 2004-10-29 12:06:48 -07:00 @@ -87,15 +87,15 @@ sema_init(sem, 0); } -asmlinkage void __down_failed(void /* special register calling convention */); -asmlinkage int __down_failed_interruptible(void /* params in registers */); -asmlinkage int __down_failed_trylock(void /* params in registers */); -asmlinkage void __up_wakeup(void /* special register calling convention */); - -asmlinkage void __down(struct semaphore * sem); -asmlinkage int __down_interruptible(struct semaphore * sem); -asmlinkage int __down_trylock(struct semaphore * sem); -asmlinkage void __up(struct semaphore * sem); +fastcall void __down_failed(void /* special register calling convention */); +fastcall int __down_failed_interruptible(void /* params in registers */); +fastcall int __down_failed_trylock(void /* params in registers */); +fastcall void __up_wakeup(void /* special register calling convention */); + +fastcall void __down(struct semaphore * sem); +fastcall int __down_interruptible(struct semaphore * sem); +fastcall int __down_trylock(struct semaphore * sem); +fastcall void __up(struct semaphore * sem); /* * This is ugly, but we want the default case to fall through. @@ -111,12 +111,13 @@ "js 2f\n" "1:\n" LOCK_SECTION_START("") - "2:\tcall __down_failed\n\t" + "2:\tlea %0,%%eax\n\t" + "call __down_failed\n\t" "jmp 1b\n" LOCK_SECTION_END :"=m" (sem->count) - :"c" (sem) - :"memory"); + : + :"memory","ax"); } /* @@ -135,11 +136,12 @@ "xorl %0,%0\n" "1:\n" LOCK_SECTION_START("") - "2:\tcall __down_failed_interruptible\n\t" + "2:\tlea %1,%%eax\n\t" + "call __down_failed_interruptible\n\t" "jmp 1b\n" LOCK_SECTION_END :"=a" (result), "=m" (sem->count) - :"c" (sem) + : :"memory"); return result; } @@ -159,11 +161,12 @@ "xorl %0,%0\n" "1:\n" LOCK_SECTION_START("") - "2:\tcall __down_failed_trylock\n\t" + "2:\tlea %1,%%eax\n\t" + "call __down_failed_trylock\n\t" "jmp 1b\n" LOCK_SECTION_END :"=a" (result), "=m" (sem->count) - :"c" (sem) + : :"memory"); return result; } @@ -182,13 +185,14 @@ "jle 2f\n" "1:\n" LOCK_SECTION_START("") - "2:\tcall __up_wakeup\n\t" + "2:\tlea %0,%%eax\n\t" + "call __up_wakeup\n\t" "jmp 1b\n" LOCK_SECTION_END ".subsection 0\n" :"=m" (sem->count) - :"c" (sem) - :"memory"); + : + :"memory","ax"); } #endif ===== include/linux/spinlock.h 1.32 vs edited ===== --- 1.32/include/linux/spinlock.h 2004-10-24 16:24:20 -07:00 +++ edited/include/linux/spinlock.h 2004-10-29 12:08:14 -07:00 @@ -27,7 +27,7 @@ extra \ ".ifndef " LOCK_SECTION_NAME "\n\t" \ LOCK_SECTION_NAME ":\n\t" \ - ".endif\n\t" + ".endif\n" #define LOCK_SECTION_END \ ".previous\n\t" - 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/
| |