linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* The SMP alternatives code breaks exception fixup?
@ 2008-01-21 20:47 Chuck Ebbert
  2008-01-21 21:25 ` Chuck Ebbert
  2008-01-22  5:26 ` Andi Kleen
  0 siblings, 2 replies; 11+ messages in thread
From: Chuck Ebbert @ 2008-01-21 20:47 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Gerd Hoffmann

Looking at the oops report from this bug:

[bugzilla]  https://bugzilla.redhat.com/show_bug.cgi?id=429412
[oops]      https://bugzilla.redhat.com/attachment.cgi?id=292260

It was an unhandled page fault (protection violation.)

I tracked it down to the cmpxchg in this code:

include/asm-x86/futex_32.h::futex_atomic_cmpxchg_inatomic()
         __asm__ __volatile__(
                 "1:     " LOCK_PREFIX "cmpxchgl %3, %1          \n"
                 "2:     .section .fixup, \"ax\"                 \n"
                 "3:     mov     %2, %0                          \n"
                 "       jmp     2b                              \n"
                 "       .previous                               \n"
                 "       .section __ex_table, \"a\"              \n"
                 "       .align  8                               \n"
                 "       .long   1b,3b                           \n"
                 "       .previous                               \n"

There is a fixup, so this should never happen. But the lock instruction
was replaced with a nop by the altinstruction code, and that makes the fixup
address wrong. AFAICT we don't fix up the exception table when we replace
a lock with a nop, which makes the fixup table point to the nop instead
of the cmpxchg instruction and causes us to miss the fixup.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: The SMP alternatives code breaks exception fixup?
  2008-01-21 20:47 The SMP alternatives code breaks exception fixup? Chuck Ebbert
@ 2008-01-21 21:25 ` Chuck Ebbert
  2008-01-22  5:26 ` Andi Kleen
  1 sibling, 0 replies; 11+ messages in thread
From: Chuck Ebbert @ 2008-01-21 21:25 UTC (permalink / raw)
  To: Chuck Ebbert; +Cc: linux-kernel, Ingo Molnar, Gerd Hoffmann

On 01/21/2008 03:47 PM, Chuck Ebbert wrote:
> Looking at the oops report from this bug:
> 
> [bugzilla]  https://bugzilla.redhat.com/show_bug.cgi?id=429412
> [oops]      https://bugzilla.redhat.com/attachment.cgi?id=292260
> 
> It was an unhandled page fault (protection violation.)
> 
> I tracked it down to the cmpxchg in this code:
> 
> include/asm-x86/futex_32.h::futex_atomic_cmpxchg_inatomic()
>          __asm__ __volatile__(
>                  "1:     " LOCK_PREFIX "cmpxchgl %3, %1          \n"
>                  "2:     .section .fixup, \"ax\"                 \n"
>                  "3:     mov     %2, %0                          \n"
>                  "       jmp     2b                              \n"
>                  "       .previous                               \n"
>                  "       .section __ex_table, \"a\"              \n"
>                  "       .align  8                               \n"
>                  "       .long   1b,3b                           \n"
>                  "       .previous                               \n"
> 
> There is a fixup, so this should never happen. But the lock instruction
> was replaced with a nop by the altinstruction code, and that makes the fixup
> address wrong. AFAICT we don't fix up the exception table when we replace
> a lock with a nop, which makes the fixup table point to the nop instead
> of the cmpxchg instruction and causes us to miss the fixup.
> 

The bug reporter has confirmed that booting with "noreplace-smp" prevents
the kernel oops.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: The SMP alternatives code breaks exception fixup?
  2008-01-21 20:47 The SMP alternatives code breaks exception fixup? Chuck Ebbert
  2008-01-21 21:25 ` Chuck Ebbert
@ 2008-01-22  5:26 ` Andi Kleen
  2008-01-22 12:35   ` Thomas Gleixner
  1 sibling, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2008-01-22  5:26 UTC (permalink / raw)
  To: Chuck Ebbert; +Cc: linux-kernel, Ingo Molnar, Gerd Hoffmann

Chuck Ebbert <cebbert@redhat.com> writes:
> 
> There is a fixup, so this should never happen. But the lock instruction
> was replaced with a nop by the altinstruction code, and that makes the fixup
> address wrong. AFAICT we don't fix up the exception table when we replace
> a lock with a nop, which makes the fixup table point to the nop instead
> of the cmpxchg instruction and causes us to miss the fixup.

Indeed.  Nasty issue.

A quick fix would be to add another fixup to handle both cases
I checked the other LOCK_PREFIX users and they look ok.

Does this fix it?

-Andi

(untested) 

---

Add exception handlers for both the LOCK and no LOCK prefix
case in futex.

Hopefully fixes https://bugzilla.redhat.com/show_bug.cgi?id=429412

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/include/asm-x86/futex.h
===================================================================
--- linux.orig/include/asm-x86/futex.h
+++ linux/include/asm-x86/futex.h
@@ -30,7 +30,7 @@
 "1:	movl	%2, %0\n					\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n			\
+"2:	" LOCK_PREFIX "\n5: cmpxchgl %3, %2\n			\
 	jnz	1b\n						\
 3:	.section .fixup,\"ax\"\n				\
 4:	mov	%5, %1\n					\
@@ -38,7 +38,7 @@
 	.previous\n						\
 	.section __ex_table,\"a\"\n				\
 	.align	8\n"						\
-	_ASM_PTR "1b,4b,2b,4b\n					\
+	_ASM_PTR "1b,4b,2b,4b,5b,4b\n				\
 	.previous"						\
 	: "=&a" (oldval), "=&r" (ret), "+m" (*uaddr),		\
 	  "=&r" (tem)						\

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: The SMP alternatives code breaks exception fixup?
  2008-01-22  5:26 ` Andi Kleen
@ 2008-01-22 12:35   ` Thomas Gleixner
  2008-01-22 20:19     ` Chuck Ebbert
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2008-01-22 12:35 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Andrew Morton, Linus Torvalds, Stable Team, Chuck Ebbert,
	linux-kernel, Ingo Molnar, Gerd Hoffmann

On Tue, 22 Jan 2008, Andi Kleen wrote:
> Chuck Ebbert <cebbert@redhat.com> writes:
> > 
> > There is a fixup, so this should never happen. But the lock instruction
> > was replaced with a nop by the altinstruction code, and that makes the fixup
> > address wrong. AFAICT we don't fix up the exception table when we replace
> > a lock with a nop, which makes the fixup table point to the nop instead
> > of the cmpxchg instruction and causes us to miss the fixup.
> 
> Indeed.  Nasty issue.
> 
> A quick fix would be to add another fixup to handle both cases

Agreed.

> I checked the other LOCK_PREFIX users and they look ok.

Really ?

> Does this fix it?

No it does not. Chuck tracked it down to 

include/asm-x86/futex_32.h::futex_atomic_cmpxchg_inatomic()

And your patch is changing: __futex_atomic_op2(). That's fine, because
we need the fixup for both places.

Also your patch is against x86.git and not against mainline. Corrected
version below. x86.git needs a different fixup once this hits
mainline, as it unifies the 32/64bit versions.

That's a long standing bug in both the PI futex and the standard futex
code. Needs to go to stable as well.

Thanks,
	tglx

------------->
Subject: x86: fix missing exception entry for SMP alternatives in futex macros
From: Thomas Gleixner <tglx@linutronix.de>

The exception fixup for the futex macros __futex_atomic_op2 and
futex_atomic_cmpxchg_inatomic() is missing an entry when the lock
prefix is replaced by a NOP via SMP alternatives.

Chuck Ebert tracked this down from the information provided in:
https://bugzilla.redhat.com/show_bug.cgi?id=429412

The solution is to add another fixup after the LOCK_PREFIX, so both
the LOCK and NOP case have their own entry in the exception table.

The solution was pointed out by Andi Kleen.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@elte.hu>

---
 include/asm-x86/futex_32.h |    8 ++++----
 include/asm-x86/futex_64.h |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

Index: linux-2.6/include/asm-x86/futex_32.h
===================================================================
--- linux-2.6.orig/include/asm-x86/futex_32.h	2008-01-22 13:13:10.000000000 +0100
+++ linux-2.6/include/asm-x86/futex_32.h	2008-01-22 13:13:49.000000000 +0100
@@ -28,7 +28,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	" LOCK_PREFIX "\n 5: cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -36,7 +36,7 @@
 	.previous\n\
 	.section __ex_table,\"a\"\n\
 	.align	8\n\
-	.long	1b,4b,2b,4b\n\
+	.long	1b,4b,2b,4b,5b,4b\n\
 	.previous"						\
 	: "=&a" (oldval), "=&r" (ret), "+m" (*uaddr),		\
 	  "=&r" (tem)						\
@@ -111,7 +111,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	" LOCK_PREFIX "\n 4: cmpxchgl %3, %1	\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
@@ -120,7 +120,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 
 		"	.section __ex_table, \"a\"		\n"
 		"	.align  8				\n"
-		"	.long   1b,3b				\n"
+		"	.long   1b,3b,4b,3b			\n"
 		"	.previous				\n"
 
 		: "=a" (oldval), "+m" (*uaddr)
Index: linux-2.6/include/asm-x86/futex_64.h
===================================================================
--- linux-2.6.orig/include/asm-x86/futex_64.h	2008-01-22 13:13:10.000000000 +0100
+++ linux-2.6/include/asm-x86/futex_64.h	2008-01-22 13:13:49.000000000 +0100
@@ -27,7 +27,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	" LOCK_PREFIX "\n 5: cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -35,7 +35,7 @@
 	.previous\n\
 	.section __ex_table,\"a\"\n\
 	.align	8\n\
-	.quad	1b,4b,2b,4b\n\
+	.quad	1b,4b,2b,4b,5b,4b\n\
 	.previous"						\
 	: "=&a" (oldval), "=&r" (ret), "=m" (*uaddr),		\
 	  "=&r" (tem)						\
@@ -101,7 +101,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	" LOCK_PREFIX "\n 4: cmpxchgl %3, %1	\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
@@ -110,7 +110,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 
 		"	.section __ex_table, \"a\"		\n"
 		"	.align  8				\n"
-		"	.quad   1b,3b				\n"
+		"	.quad   1b,3b,4b,3b			\n"
 		"	.previous				\n"
 
 		: "=a" (oldval), "=m" (*uaddr)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: The SMP alternatives code breaks exception fixup?
  2008-01-22 12:35   ` Thomas Gleixner
@ 2008-01-22 20:19     ` Chuck Ebbert
  2008-02-06 23:10       ` [stable] " Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Chuck Ebbert @ 2008-01-22 20:19 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andi Kleen, Andrew Morton, Linus Torvalds, Stable Team,
	linux-kernel, Ingo Molnar, Gerd Hoffmann

On 01/22/2008 07:35 AM, Thomas Gleixner wrote:
> 
> That's a long standing bug in both the PI futex and the standard futex
> code. Needs to go to stable as well.
> 

Here's the 2.6.23 version:


Subject: x86: fix missing exception entry for SMP alternatives in futex macros
From: Thomas Gleixner <tglx@linutronix.de>

The exception fixup for the futex macros __futex_atomic_op2 and
futex_atomic_cmpxchg_inatomic() is missing an entry when the lock
prefix is replaced by a NOP via SMP alternatives.

Chuck Ebert tracked this down from the information provided in:
https://bugzilla.redhat.com/show_bug.cgi?id=429412

The solution is to add another fixup after the LOCK_PREFIX, so both
the LOCK and NOP case have their own entry in the exception table.

The solution was pointed out by Andi Kleen.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@elte.hu>

---
 include/asm-i386/futex.h |    8 ++++----
 include/asm-x86_64/futex.h |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

Index: linux-2.6/include/asm-i386/futex.h
===================================================================
--- linux-2.6.orig/include/asm-i386/futex.h	2008-01-22 13:13:10.000000000 +0100
+++ linux-2.6/include/asm-i386/futex.h	2008-01-22 13:13:49.000000000 +0100
@@ -28,7 +28,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	" LOCK_PREFIX "\n 5: cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -36,7 +36,7 @@
 	.previous\n\
 	.section __ex_table,\"a\"\n\
 	.align	8\n\
-	.long	1b,4b,2b,4b\n\
+	.long	1b,4b,2b,4b,5b,4b\n\
 	.previous"						\
 	: "=&a" (oldval), "=&r" (ret), "+m" (*uaddr),		\
 	  "=&r" (tem)						\
@@ -111,7 +111,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	" LOCK_PREFIX "\n 4: cmpxchgl %3, %1	\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
@@ -120,7 +120,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 
 		"	.section __ex_table, \"a\"		\n"
 		"	.align  8				\n"
-		"	.long   1b,3b				\n"
+		"	.long   1b,3b,4b,3b			\n"
 		"	.previous				\n"
 
 		: "=a" (oldval), "+m" (*uaddr)
Index: linux-2.6/include/asm-x86_64/futex.h
===================================================================
--- linux-2.6.orig/include/asm-x86_64/futex.h	2008-01-22 13:13:10.000000000 +0100
+++ linux-2.6/include/asm-x86_64/futex.h	2008-01-22 13:13:49.000000000 +0100
@@ -27,7 +27,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	" LOCK_PREFIX "\n 5: cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -35,7 +35,7 @@
 	.previous\n\
 	.section __ex_table,\"a\"\n\
 	.align	8\n\
-	.quad	1b,4b,2b,4b\n\
+	.quad	1b,4b,2b,4b,5b,4b\n\
 	.previous"						\
 	: "=&a" (oldval), "=&r" (ret), "=m" (*uaddr),		\
 	  "=&r" (tem)						\
@@ -101,7 +101,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	" LOCK_PREFIX "\n 4: cmpxchgl %3, %1	\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
@@ -110,7 +110,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 
 		"	.section __ex_table, \"a\"		\n"
 		"	.align  8				\n"
-		"	.quad   1b,3b				\n"
+		"	.quad   1b,3b,4b,3b			\n"
 		"	.previous				\n"
 
 		: "=a" (oldval), "=m" (*uaddr)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [stable] The SMP alternatives code breaks exception fixup?
  2008-01-22 20:19     ` Chuck Ebbert
@ 2008-02-06 23:10       ` Greg KH
  2008-02-06 23:36         ` Linus Torvalds
  2008-02-07 12:16         ` Thomas Gleixner
  0 siblings, 2 replies; 11+ messages in thread
From: Greg KH @ 2008-02-06 23:10 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Thomas Gleixner, Ingo Molnar, linux-kernel, Andi Kleen,
	Gerd Hoffmann, Andrew Morton, Linus Torvalds, Stable Team

On Tue, Jan 22, 2008 at 03:19:33PM -0500, Chuck Ebbert wrote:
> On 01/22/2008 07:35 AM, Thomas Gleixner wrote:
> > 
> > That's a long standing bug in both the PI futex and the standard futex
> > code. Needs to go to stable as well.
> > 
> 
> Here's the 2.6.23 version:
> 
> 
> Subject: x86: fix missing exception entry for SMP alternatives in futex macros
> From: Thomas Gleixner <tglx@linutronix.de>

I don't see this in Linus's tree, am I just missing it?  Do you have a
git commit id?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [stable] The SMP alternatives code breaks exception fixup?
  2008-02-06 23:10       ` [stable] " Greg KH
@ 2008-02-06 23:36         ` Linus Torvalds
  2008-02-06 23:43           ` Ingo Molnar
  2008-02-07 12:16         ` Thomas Gleixner
  1 sibling, 1 reply; 11+ messages in thread
From: Linus Torvalds @ 2008-02-06 23:36 UTC (permalink / raw)
  To: Greg KH
  Cc: Chuck Ebbert, Thomas Gleixner, Ingo Molnar, linux-kernel,
	Andi Kleen, Gerd Hoffmann, Andrew Morton, Stable Team



On Wed, 6 Feb 2008, Greg KH wrote:
> 
> I don't see this in Linus's tree, am I just missing it?  Do you have a
> git commit id?

Isn't this 9d55b9923a1b7ea8193b8875c57ec940dc2ff027 (possibly with 
2532ec6d178abc55681d049097d3dc577eaa266c on top)?

		Linus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [stable] The SMP alternatives code breaks exception fixup?
  2008-02-06 23:36         ` Linus Torvalds
@ 2008-02-06 23:43           ` Ingo Molnar
  2008-02-07  0:00             ` Greg KH
  2008-02-07  6:55             ` Greg KH
  0 siblings, 2 replies; 11+ messages in thread
From: Ingo Molnar @ 2008-02-06 23:43 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Greg KH, Chuck Ebbert, Thomas Gleixner, linux-kernel, Andi Kleen,
	Gerd Hoffmann, Andrew Morton, Stable Team


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> > I don't see this in Linus's tree, am I just missing it?  Do you have 
> > a git commit id?
> 
> Isn't this 9d55b9923a1b7ea8193b8875c57ec940dc2ff027 (possibly with 
> 2532ec6d178abc55681d049097d3dc577eaa266c on top)?

yeah.

Greg: note that this is against post-unification asm-x86/futex.h so it 
wont apply to .24.

I've backported the fix to .24 - find it below. (untested but obvious)

	Ingo

----------------------->
Subject: x86: replace LOCK_PREFIX in futex.h
From: Thomas Gleixner <tglx@linutronix.de>

The exception fixup for the futex macros __futex_atomic_op1/2 and
futex_atomic_cmpxchg_inatomic() is missing an entry when the lock
prefix is replaced by a NOP via SMP alternatives.

Chuck Ebert tracked this down from the information provided in:
https://bugzilla.redhat.com/show_bug.cgi?id=429412

A possible solution would be to add another fixup after the
LOCK_PREFIX, so both the LOCK and NOP case have their own entry in the
exception table, but it's not really worth the trouble.

Simply replace LOCK_PREFIX with lock and keep those untouched by SMP
alternatives.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/asm-x86/futex_32.h |    6 +++---
 include/asm-x86/futex_64.h |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

Index: linux-2.6.24/include/asm-x86/futex_32.h
===================================================================
--- linux-2.6.24.orig/include/asm-x86/futex_32.h
+++ linux-2.6.24/include/asm-x86/futex_32.h
@@ -28,7 +28,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	lock; cmpxchgl %3, %2\n					\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -68,7 +68,7 @@ futex_atomic_op_inuser (int encoded_op, 
 #endif
 		switch (op) {
 		case FUTEX_OP_ADD:
-			__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret,
+			__futex_atomic_op1("lock; xaddl %0, %2", ret, oldval,
 					   oldval, uaddr, oparg);
 			break;
 		case FUTEX_OP_OR:
@@ -111,8 +111,8 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
 
+		"1:	lock; cmpxchgl %3, %1			\n"
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
 		"	jmp     2b				\n"
Index: linux-2.6.24/include/asm-x86/futex_64.h
===================================================================
--- linux-2.6.24.orig/include/asm-x86/futex_64.h
+++ linux-2.6.24/include/asm-x86/futex_64.h
@@ -27,7 +27,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	lock; cmpxchgl %3, %2\n					\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -62,7 +62,7 @@ futex_atomic_op_inuser (int encoded_op, 
 		__futex_atomic_op1("xchgl %0, %2", ret, oldval, uaddr, oparg);
 		break;
 	case FUTEX_OP_ADD:
-		__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret, oldval,
+		__futex_atomic_op1("lock; xaddl %0, %2", ret, oldval,
 				   uaddr, oparg);
 		break;
 	case FUTEX_OP_OR:
@@ -101,8 +101,8 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
 
+		"1:	lock; cmpxchgl %3, %1			\n"
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
 		"	jmp     2b				\n"

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [stable] The SMP alternatives code breaks exception fixup?
  2008-02-06 23:43           ` Ingo Molnar
@ 2008-02-07  0:00             ` Greg KH
  2008-02-07  6:55             ` Greg KH
  1 sibling, 0 replies; 11+ messages in thread
From: Greg KH @ 2008-02-07  0:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, Chuck Ebbert, Thomas Gleixner, linux-kernel,
	Andi Kleen, Gerd Hoffmann, Andrew Morton, Stable Team

On Thu, Feb 07, 2008 at 12:43:09AM +0100, Ingo Molnar wrote:
> 
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > > I don't see this in Linus's tree, am I just missing it?  Do you have 
> > > a git commit id?
> > 
> > Isn't this 9d55b9923a1b7ea8193b8875c57ec940dc2ff027 (possibly with 
> > 2532ec6d178abc55681d049097d3dc577eaa266c on top)?
> 
> yeah.
> 
> Greg: note that this is against post-unification asm-x86/futex.h so it 
> wont apply to .24.
> 
> I've backported the fix to .24 - find it below. (untested but obvious)

Thanks for the fix, I'm guessing that this should only go into
.24-stable and nothing prior?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [stable] The SMP alternatives code breaks exception fixup?
  2008-02-06 23:43           ` Ingo Molnar
  2008-02-07  0:00             ` Greg KH
@ 2008-02-07  6:55             ` Greg KH
  1 sibling, 0 replies; 11+ messages in thread
From: Greg KH @ 2008-02-07  6:55 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linus Torvalds, Chuck Ebbert, Thomas Gleixner, linux-kernel,
	Andi Kleen, Gerd Hoffmann, Andrew Morton, Stable Team

On Thu, Feb 07, 2008 at 12:43:09AM +0100, Ingo Molnar wrote:
> 
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > > I don't see this in Linus's tree, am I just missing it?  Do you have 
> > > a git commit id?
> > 
> > Isn't this 9d55b9923a1b7ea8193b8875c57ec940dc2ff027 (possibly with 
> > 2532ec6d178abc55681d049097d3dc577eaa266c on top)?
> 
> yeah.
> 
> Greg: note that this is against post-unification asm-x86/futex.h so it 
> wont apply to .24.
> 
> I've backported the fix to .24 - find it below. (untested but obvious)

unfortunatly it doesn't even compile:

distcc[30740] ERROR: compile (null) on localhost failed
In file included from include/asm/futex.h:2,
                 from kernel/futex.c:59:
include/asm/futex_32.h:72:29: error: macro "__futex_atomic_op1" passed 6 arguments, but takes just 5
In file included from include/asm/futex.h:2,
                 from kernel/futex.c:59:
include/asm/futex_32.h: In function 'futex_atomic_op_inuser':
include/asm/futex_32.h:71: error: '__futex_atomic_op1' undeclared (first use in this function)
include/asm/futex_32.h:71: error: (Each undeclared identifier is reported only once
include/asm/futex_32.h:71: error: for each function it appears in.)
distcc[30739] ERROR: compile kernel/futex.c on localhost failed
make[1]: *** [kernel/futex.o] Error 1


Care to try again?  :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [stable] The SMP alternatives code breaks exception fixup?
  2008-02-06 23:10       ` [stable] " Greg KH
  2008-02-06 23:36         ` Linus Torvalds
@ 2008-02-07 12:16         ` Thomas Gleixner
  1 sibling, 0 replies; 11+ messages in thread
From: Thomas Gleixner @ 2008-02-07 12:16 UTC (permalink / raw)
  To: Greg KH
  Cc: Chuck Ebbert, Ingo Molnar, linux-kernel, Andi Kleen,
	Gerd Hoffmann, Andrew Morton, Linus Torvalds, Stable Team

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1017 bytes --]

On Wed, 6 Feb 2008, Greg KH wrote:

> On Tue, Jan 22, 2008 at 03:19:33PM -0500, Chuck Ebbert wrote:
> > On 01/22/2008 07:35 AM, Thomas Gleixner wrote:
> > > 
> > > That's a long standing bug in both the PI futex and the standard futex
> > > code. Needs to go to stable as well.
> > > 
> > 
> > Here's the 2.6.23 version:
> > 
> > 
> > Subject: x86: fix missing exception entry for SMP alternatives in futex macros
> > From: Thomas Gleixner <tglx@linutronix.de>
> 
> I don't see this in Linus's tree, am I just missing it?  Do you have a
> git commit id?

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9d55b9923a1b7ea8193b8875c57ec940dc2ff027
 
I did a simpler version by just replacing LOCK_PREFIX with lock. The
reason is, that we would need a separate implementation for
__futex_atomic_op1() as well, due to:

case FUTEX_OP_ADD:
	__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret, oldval,

And it's not really worth the trouble.

Backport to 2.6.24 and 2.6.23 attached.

Thanks,
	tglx

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2973 bytes --]

Subject: x86: replace LOCK_PREFIX in futex.h
From: Thomas Gleixner <tglx@linutronix.de>
Date: Thu, 07 Feb 2008 13:05:19 +0100

The exception fixup for the futex macros __futex_atomic_op1/2 and
futex_atomic_cmpxchg_inatomic() is missing an entry when the lock
prefix is replaced by a NOP via SMP alternatives.

Chuck Ebert tracked this down from the information provided in:
https://bugzilla.redhat.com/show_bug.cgi?id=429412

A possible solution would be to add another fixup after the
LOCK_PREFIX, so both the LOCK and NOP case have their own entry in the
exception table, but it's not really worth the trouble.

Simply replace LOCK_PREFIX with lock and keep those untouched by SMP
alternatives.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
                                                                
Index: linux-2.6.23/include/asm-i386/futex.h
===================================================================
--- linux-2.6.23.orig/include/asm-i386/futex.h	2008-02-07 13:08:03.000000000 +0100
+++ linux-2.6.23/include/asm-i386/futex.h	2008-02-07 13:09:39.000000000 +0100
@@ -28,7 +28,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	lock cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -68,7 +68,7 @@ futex_atomic_op_inuser (int encoded_op, 
 #endif
 		switch (op) {
 		case FUTEX_OP_ADD:
-			__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret,
+			__futex_atomic_op1("lock xaddl %0, %2", ret,
 					   oldval, uaddr, oparg);
 			break;
 		case FUTEX_OP_OR:
@@ -111,7 +111,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	lock cmpxchgl %3, %1			\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
Index: linux-2.6.23/include/asm-x86_64/futex.h
===================================================================
--- linux-2.6.23.orig/include/asm-x86_64/futex.h	2008-02-07 13:08:03.000000000 +0100
+++ linux-2.6.23/include/asm-x86_64/futex.h	2008-02-07 13:09:39.000000000 +0100
@@ -27,7 +27,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	"lock cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -62,7 +62,7 @@ futex_atomic_op_inuser (int encoded_op, 
 		__futex_atomic_op1("xchgl %0, %2", ret, oldval, uaddr, oparg);
 		break;
 	case FUTEX_OP_ADD:
-		__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret, oldval,
+		__futex_atomic_op1("lock xaddl %0, %2", ret, oldval,
 				   uaddr, oparg);
 		break;
 	case FUTEX_OP_OR:
@@ -101,7 +101,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	lock cmpxchgl %3, %1			\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"

[-- Attachment #3: Type: TEXT/PLAIN, Size: 2913 bytes --]

Subject: x86: replace LOCK_PREFIX in futex.h
From: Thomas Gleixner <tglx@linutronix.de>
Date: Thu, 07 Feb 2008 13:06:02 +0100

The exception fixup for the futex macros __futex_atomic_op1/2 and
futex_atomic_cmpxchg_inatomic() is missing an entry when the lock
prefix is replaced by a NOP via SMP alternatives.

Chuck Ebert tracked this down from the information provided in:
https://bugzilla.redhat.com/show_bug.cgi?id=429412

A possible solution would be to add another fixup after the
LOCK_PREFIX, so both the LOCK and NOP case have their own entry in the
exception table, but it's not really worth the trouble.

Simply replace LOCK_PREFIX with lock and keep those untouched by SMP
alternatives.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Index: linux-2.6.24/include/asm-x86/futex_32.h
===================================================================
--- linux-2.6.24.orig/include/asm-x86/futex_32.h	2008-02-07 13:05:39.000000000 +0100
+++ linux-2.6.24/include/asm-x86/futex_32.h	2008-02-07 13:07:20.000000000 +0100
@@ -28,7 +28,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	lock cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -68,7 +68,7 @@ futex_atomic_op_inuser (int encoded_op, 
 #endif
 		switch (op) {
 		case FUTEX_OP_ADD:
-			__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret,
+			__futex_atomic_op1("lock xaddl %0, %2", ret,
 					   oldval, uaddr, oparg);
 			break;
 		case FUTEX_OP_OR:
@@ -111,7 +111,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	lock cmpxchgl %3, %1			\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"
Index: linux-2.6.24/include/asm-x86/futex_64.h
===================================================================
--- linux-2.6.24.orig/include/asm-x86/futex_64.h	2008-02-07 13:05:39.000000000 +0100
+++ linux-2.6.24/include/asm-x86/futex_64.h	2008-02-07 13:06:45.000000000 +0100
@@ -27,7 +27,7 @@
 "1:	movl	%2, %0\n\
 	movl	%0, %3\n"					\
 	insn "\n"						\
-"2:	" LOCK_PREFIX "cmpxchgl %3, %2\n\
+"2:	"lock cmpxchgl %3, %2\n\
 	jnz	1b\n\
 3:	.section .fixup,\"ax\"\n\
 4:	mov	%5, %1\n\
@@ -62,7 +62,7 @@ futex_atomic_op_inuser (int encoded_op, 
 		__futex_atomic_op1("xchgl %0, %2", ret, oldval, uaddr, oparg);
 		break;
 	case FUTEX_OP_ADD:
-		__futex_atomic_op1(LOCK_PREFIX "xaddl %0, %2", ret, oldval,
+		__futex_atomic_op1("lock xaddl %0, %2", ret, oldval,
 				   uaddr, oparg);
 		break;
 	case FUTEX_OP_OR:
@@ -101,7 +101,7 @@ futex_atomic_cmpxchg_inatomic(int __user
 		return -EFAULT;
 
 	__asm__ __volatile__(
-		"1:	" LOCK_PREFIX "cmpxchgl %3, %1		\n"
+		"1:	lock cmpxchgl %3, %1			\n"
 
 		"2:	.section .fixup, \"ax\"			\n"
 		"3:	mov     %2, %0				\n"

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-02-07 12:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-21 20:47 The SMP alternatives code breaks exception fixup? Chuck Ebbert
2008-01-21 21:25 ` Chuck Ebbert
2008-01-22  5:26 ` Andi Kleen
2008-01-22 12:35   ` Thomas Gleixner
2008-01-22 20:19     ` Chuck Ebbert
2008-02-06 23:10       ` [stable] " Greg KH
2008-02-06 23:36         ` Linus Torvalds
2008-02-06 23:43           ` Ingo Molnar
2008-02-07  0:00             ` Greg KH
2008-02-07  6:55             ` Greg KH
2008-02-07 12:16         ` Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).