All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Fengguang Wu <fengguang.wu@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH, -v3] compiler/gcc4: Add quirk for 'asm goto' miscompilation bug
Date: Thu, 10 Oct 2013 17:12:19 +0200	[thread overview]
Message-ID: <20131010151219.GC5167@gmail.com> (raw)
In-Reply-To: <20131010142703.GI30970@tucnak.zalov.cz>


* Jakub Jelinek <jakub@redhat.com> wrote:

> On Thu, Oct 10, 2013 at 07:04:18AM -0700, Richard Henderson wrote:
> > On 10/10/2013 01:31 AM, Jakub Jelinek wrote:
> > > Also, for the bitops patch, you probably want an asm_volatile_goto variant.
> > 
> > Why?  Asm without output (which asm goto must be) are automatically volatile.
> 
> You're right.

Indeed. Something like the updated version below?

[ I kept the asm_volatile_goto() naming to make sure the implicit volatile 
  is visible. ]

Thanks,

	Ingo

--------------------->
>From 1ab592e8d55576880b90c131529111f51c188284 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@kernel.org>
Date: Thu, 10 Oct 2013 10:16:30 +0200
Subject: [PATCH] compiler/gcc4: Add quirk for 'asm goto' miscompilation bug

Fengguang Wu, Oleg Nesterov and Peter Zijlstra tracked down
a kernel crash to a GCC bug: GCC miscompiles certain 'asm goto'
constructs, as outlined here:

  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670

Implement a workaround suggested by Jakub Jelinek.

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Reported-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Suggested-by: Jakub Jelinek <jakub@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm/include/asm/jump_label.h     |  2 +-
 arch/mips/include/asm/jump_label.h    |  2 +-
 arch/powerpc/include/asm/jump_label.h |  2 +-
 arch/s390/include/asm/jump_label.h    |  2 +-
 arch/sparc/include/asm/jump_label.h   |  2 +-
 arch/x86/include/asm/cpufeature.h     |  6 +++---
 arch/x86/include/asm/jump_label.h     |  2 +-
 arch/x86/include/asm/mutex_64.h       |  4 ++--
 arch/x86/include/asm/rmwcc.h          |  2 +-
 include/linux/compiler-gcc4.h         | 15 +++++++++++++++
 10 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h
index bfc198c..863c892 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -16,7 +16,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-	asm goto("1:\n\t"
+	asm_volatile_goto("1:\n\t"
 		 JUMP_LABEL_NOP "\n\t"
 		 ".pushsection __jump_table,  \"aw\"\n\t"
 		 ".word 1b, %l[l_yes], %c0\n\t"
diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
index 4d6d77e..e194f95 100644
--- a/arch/mips/include/asm/jump_label.h
+++ b/arch/mips/include/asm/jump_label.h
@@ -22,7 +22,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-	asm goto("1:\tnop\n\t"
+	asm_volatile_goto("1:\tnop\n\t"
 		"nop\n\t"
 		".pushsection __jump_table,  \"aw\"\n\t"
 		WORD_INSN " 1b, %l[l_yes], %0\n\t"
diff --git a/arch/powerpc/include/asm/jump_label.h b/arch/powerpc/include/asm/jump_label.h
index ae098c4..f016bb6 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -19,7 +19,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-	asm goto("1:\n\t"
+	asm_volatile_goto("1:\n\t"
 		 "nop\n\t"
 		 ".pushsection __jump_table,  \"aw\"\n\t"
 		 JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
index 6c32190..346b1c8 100644
--- a/arch/s390/include/asm/jump_label.h
+++ b/arch/s390/include/asm/jump_label.h
@@ -15,7 +15,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-	asm goto("0:	brcl 0,0\n"
+	asm_volatile_goto("0:	brcl 0,0\n"
 		".pushsection __jump_table, \"aw\"\n"
 		ASM_ALIGN "\n"
 		ASM_PTR " 0b, %l[label], %0\n"
diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
index 5080d16..ec2e2e2 100644
--- a/arch/sparc/include/asm/jump_label.h
+++ b/arch/sparc/include/asm/jump_label.h
@@ -9,7 +9,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-		asm goto("1:\n\t"
+		asm_volatile_goto("1:\n\t"
 			 "nop\n\t"
 			 "nop\n\t"
 			 ".pushsection __jump_table,  \"aw\"\n\t"
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index d3f5c63..89270b4 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -374,7 +374,7 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
 		 * Catch too early usage of this before alternatives
 		 * have run.
 		 */
-		asm goto("1: jmp %l[t_warn]\n"
+		asm_volatile_goto("1: jmp %l[t_warn]\n"
 			 "2:\n"
 			 ".section .altinstructions,\"a\"\n"
 			 " .long 1b - .\n"
@@ -388,7 +388,7 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
 
 #endif
 
-		asm goto("1: jmp %l[t_no]\n"
+		asm_volatile_goto("1: jmp %l[t_no]\n"
 			 "2:\n"
 			 ".section .altinstructions,\"a\"\n"
 			 " .long 1b - .\n"
@@ -453,7 +453,7 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
  * have. Thus, we force the jump to the widest, 4-byte, signed relative
  * offset even though the last would often fit in less bytes.
  */
-		asm goto("1: .byte 0xe9\n .long %l[t_dynamic] - 2f\n"
+		asm_volatile_goto("1: .byte 0xe9\n .long %l[t_dynamic] - 2f\n"
 			 "2:\n"
 			 ".section .altinstructions,\"a\"\n"
 			 " .long 1b - .\n"		/* src offset */
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 64507f3..6a2cefb 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -18,7 +18,7 @@
 
 static __always_inline bool arch_static_branch(struct static_key *key)
 {
-	asm goto("1:"
+	asm_volatile_goto("1:"
 		".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
 		".pushsection __jump_table,  \"aw\" \n\t"
 		_ASM_ALIGN "\n\t"
diff --git a/arch/x86/include/asm/mutex_64.h b/arch/x86/include/asm/mutex_64.h
index e7e6751..07537a4 100644
--- a/arch/x86/include/asm/mutex_64.h
+++ b/arch/x86/include/asm/mutex_64.h
@@ -20,7 +20,7 @@
 static inline void __mutex_fastpath_lock(atomic_t *v,
 					 void (*fail_fn)(atomic_t *))
 {
-	asm volatile goto(LOCK_PREFIX "   decl %0\n"
+	asm_volatile_goto(LOCK_PREFIX "   decl %0\n"
 			  "   jns %l[exit]\n"
 			  : : "m" (v->counter)
 			  : "memory", "cc"
@@ -75,7 +75,7 @@ static inline int __mutex_fastpath_lock_retval(atomic_t *count)
 static inline void __mutex_fastpath_unlock(atomic_t *v,
 					   void (*fail_fn)(atomic_t *))
 {
-	asm volatile goto(LOCK_PREFIX "   incl %0\n"
+	asm_volatile_goto(LOCK_PREFIX "   incl %0\n"
 			  "   jg %l[exit]\n"
 			  : : "m" (v->counter)
 			  : "memory", "cc"
diff --git a/arch/x86/include/asm/rmwcc.h b/arch/x86/include/asm/rmwcc.h
index 735f184..1ff990f 100644
--- a/arch/x86/include/asm/rmwcc.h
+++ b/arch/x86/include/asm/rmwcc.h
@@ -5,7 +5,7 @@
 
 #define __GEN_RMWcc(fullop, var, cc, ...)				\
 do {									\
-	asm volatile goto (fullop "; j" cc " %l[cc_label]"		\
+	asm_volatile_goto (fullop "; j" cc " %l[cc_label]"		\
 			: : "m" (var), ## __VA_ARGS__ 			\
 			: "memory" : cc_label);				\
 	return 0;							\
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 842de22..ded4299 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -65,6 +65,21 @@
 #define __visible __attribute__((externally_visible))
 #endif
 
+/*
+ * GCC 'asm goto' miscompiles certain code sequences:
+ *
+ *   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
+ *
+ * Work it around via a compiler barrier quirk suggested by Jakub Jelinek.
+ * Fixed in GCC 4.8.2 and later versions.
+ *
+ * (asm goto is automatically volatile - the naming reflects this.)
+ */
+#if GCC_VERSION <= 40801
+# define asm_volatile_goto(x...)	do { asm goto(x); asm (""); } while (0)
+#else
+# define asm_volatile_goto(x...)	do { asm goto(x); } while (0)
+#endif
 
 #ifdef CONFIG_ARCH_USE_BUILTIN_BSWAP
 #if GCC_VERSION >= 40400

  reply	other threads:[~2013-10-10 15:12 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-05 23:44 [x86] BUG: unable to handle kernel paging request at 00740060 Fengguang Wu
2013-10-05 23:47 ` [x86] BUG: unable to handle kernel paging request at 08000060 Fengguang Wu
2013-10-06  7:27   ` Mike Galbraith
2013-10-06  7:36     ` Fengguang Wu
2013-10-07  8:49   ` Peter Zijlstra
2013-10-07  9:17     ` Fengguang Wu
2013-10-07  9:36       ` Peter Zijlstra
2013-10-07  9:46         ` Fengguang Wu
2013-10-07  8:55 ` [x86] BUG: unable to handle kernel paging request at 00740060 Peter Zijlstra
2013-10-07  9:08   ` Peter Zijlstra
2013-10-07 11:32     ` Fengguang Wu
2013-10-07  9:27   ` Fengguang Wu
2013-10-07 18:47 ` Linus Torvalds
2013-10-08  7:51   ` Fengguang Wu
2013-10-08 16:21     ` Linus Torvalds
2013-10-08 17:15       ` [x86] BUG: unable to handle kernel NULL pointer dereference at (null) Fengguang Wu
2013-10-08 17:31         ` Linus Torvalds
2013-10-09  1:09           ` Fengguang Wu
2013-10-09  1:33             ` Linus Torvalds
2013-10-08 18:51       ` [x86] BUG: unable to handle kernel paging request at 00740060 Oleg Nesterov
2013-10-08 19:05         ` Jakub Jelinek
2013-10-08 19:20           ` Linus Torvalds
2013-10-08 19:34             ` Linus Torvalds
2013-10-08 19:35           ` Oleg Nesterov
2013-10-08 19:49             ` Linus Torvalds
2013-10-09  1:43           ` Mike Galbraith
2013-10-08 19:05         ` Linus Torvalds
2013-10-08 16:46     ` Oleg Nesterov
2013-10-08 14:34   ` Oleg Nesterov
2013-10-09  8:04     ` Fengguang Wu
2013-10-09 12:19       ` Fengguang Wu
2013-10-09 12:21         ` Fengguang Wu
2013-10-09 12:27         ` Peter Zijlstra
2013-10-09 12:52           ` Ingo Molnar
2013-10-09 17:18             ` Ingo Molnar
2013-10-10  2:15               ` Mike Galbraith
2013-10-09 12:56           ` Fengguang Wu
2013-10-09 12:43       ` Oleg Nesterov
2013-10-09 14:07         ` Peter Zijlstra
2013-10-09 14:17           ` Oleg Nesterov
2013-10-09 14:32           ` Ingo Molnar
2013-10-09 14:33           ` Peter Zijlstra
2013-10-09 14:46             ` Peter Zijlstra
2013-10-09 18:16               ` Jakub Jelinek
2013-10-09 18:54                 ` Linus Torvalds
2013-10-09 19:02                 ` Peter Zijlstra
2013-10-09 19:08                   ` Jakub Jelinek
2013-10-10  6:22                     ` Ingo Molnar
2013-10-10  6:51                       ` Jakub Jelinek
2013-10-10  8:04                         ` Jakub Jelinek
2013-10-10  8:24                           ` [PATCH] gcc4: Add 'asm goto' miscompilation quirk Ingo Molnar
2013-10-10  8:31                             ` Jakub Jelinek
2013-10-10  8:45                               ` Ingo Molnar
2013-10-10  8:55                                 ` [PATCH, -v2] compiler/gcc4: Add quirk for 'asm goto' miscompilation bug Ingo Molnar
2013-10-10 11:56                                   ` Peter Zijlstra
2013-10-10 12:32                                     ` Jakub Jelinek
2013-10-10 13:10                                       ` Peter Zijlstra
2013-10-10 15:04                                         ` Ingo Molnar
2013-10-10 14:04                               ` [PATCH] gcc4: Add 'asm goto' miscompilation quirk Richard Henderson
2013-10-10 14:27                                 ` Jakub Jelinek
2013-10-10 15:12                                   ` Ingo Molnar [this message]
2013-10-10 16:15                                     ` [PATCH, -v3] compiler/gcc4: Add quirk for 'asm goto' miscompilation bug Richard Henderson
2013-10-10 16:49                                       ` Ingo Molnar
2013-10-11  4:35                                     ` Fengguang Wu
2013-10-11  5:46                                       ` Ingo Molnar
2013-10-11  6:51                                         ` Fengguang Wu
2013-10-11  9:30                                           ` Fengguang Wu
2013-10-12 17:03                                             ` Ingo Molnar
2013-10-10  8:34                             ` [PATCH] gcc4: Add 'asm goto' miscompilation quirk Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131010151219.GC5167@gmail.com \
    --to=mingo@kernel.org \
    --cc=fengguang.wu@intel.com \
    --cc=jakub@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rth@twiddle.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.