Remaining bits of WARN_ON series

PATCH In review Olof Johansson 3 participants 6 messages Jan 17, 2008

Subsystems (2)

Library code Generic include/asm header files

lore

Share & tools

lore

Olof Johansson

author

2008-01-17 00:51 UTC #0 lore raw

cover Remaining bits of WARN_ON series

Arjan posted a series of WARN_ON enhancements, parts of those were from two previous patches from me.

Ingo picked up 3 of the 5 patches. Here are the rest-reposted again, plus another patch bringing the generic report_bug() code up to the same format as the x86-specific one.

These have to be applied on top of x86.git, since they depend on changes in there. Ingo didn't want to take them directly since they touch code in other architectures.

-Olof

Olof Johansson

author

2008-01-17 00:58 UTC #1 lore raw

PATCH 1/3 bug.h: Remove HAVE_ARCH_BUG and HAVE_ARCH_WARN

No need to have the HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* defines, when the generic implementation can just use #ifndef on the macros themselves.

Signed-off-by Olof Johansson <[email protected]>

Cc <[email protected]>

---

 include/asm-alpha/bug.h   |    1 -
 include/asm-arm/bug.h     |    1 -
 include/asm-avr32/bug.h   |    3 ---
 include/asm-frv/bug.h     |    1 -
 include/asm-generic/bug.h |   10 +++++-----
 include/asm-ia64/bug.h    |    1 -
 include/asm-m68k/bug.h    |    1 -
 include/asm-mips/bug.h    |    4 ----
 include/asm-parisc/bug.h  |    2 --
 include/asm-powerpc/bug.h |    3 ---
 include/asm-s390/bug.h    |    2 --
 include/asm-sparc/bug.h   |    1 -
 include/asm-sparc64/bug.h |    1 -
 include/asm-v850/bug.h    |    1 -
 include/asm-x86/bug.h     |    1 -
 15 files changed, 5 insertions(+), 28 deletions(-)

include/asm-alpha/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-alpha/bug.h
===================================================================
@@ -10,7 +10,6 @@
   __asm__ __volatile__("call_pal %0  # bugchk\n\t"".long %1\n\t.8byte %2" \
 		       : : "i" (PAL_bugchk), "i"(__LINE__), "i"(__FILE__))
 
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-arm/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-arm/bug.h
===================================================================
@@ -16,7 +16,6 @@ extern void __bug(const char *file, int 
 
 #endif
 
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-avr32/bug.h +0 -3

Index: linux-2.6.24-rc6/include/asm-avr32/bug.h
===================================================================
@@ -63,9 +63,6 @@
 		unlikely(__ret_warn_on);				\
 	})
 
-#define HAVE_ARCH_BUG
-#define HAVE_ARCH_WARN_ON
-
 #endif /* CONFIG_BUG */
 
 #include <asm-generic/bug.h>

include/asm-frv/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-frv/bug.h
===================================================================
@@ -32,7 +32,6 @@ do {						\
 	asm volatile("nop");			\
 } while(0)
 
-#define HAVE_ARCH_BUG
 #define BUG()					\
 do {						\
 	_debug_bug_printk();			\

include/asm-generic/bug.h +5 -5

Index: linux-2.6.24-rc6/include/asm-generic/bug.h
===================================================================
@@ -20,14 +20,14 @@ struct bug_entry {
 #define BUGFLAG_WARNING	(1<<0)
 #endif	/* CONFIG_GENERIC_BUG */
 
-#ifndef HAVE_ARCH_BUG
+#ifndef BUG
 #define BUG() do { \
 	printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __FUNCTION__); \
 	panic("BUG!"); \
 } while (0)
 #endif
 
-#ifndef HAVE_ARCH_BUG_ON
+#ifndef BUG_ON
 #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
 #endif

@@ -49,15 +49,15 @@ extern void warn_on_slowpath(const char 
 #endif
 
 #else /* !CONFIG_BUG */
-#ifndef HAVE_ARCH_BUG
+#ifndef BUG
 #define BUG()
 #endif
 
-#ifndef HAVE_ARCH_BUG_ON
+#ifndef BUG_ON
 #define BUG_ON(condition) do { if (condition) ; } while(0)
 #endif
 
-#ifndef HAVE_ARCH_WARN_ON
+#ifndef WARN_ON
 #define WARN_ON(condition) ({						\
 	int __ret_warn_on = !!(condition);				\
 	unlikely(__ret_warn_on);					\

include/asm-ia64/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-ia64/bug.h
===================================================================
@@ -6,7 +6,6 @@
 #define BUG() do { printk("kernel BUG at %s:%d!\n", __FILE__, __LINE__); ia64_abort(); } while (0)
 
 /* should this BUG be made generic? */
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-m68k/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-m68k/bug.h
===================================================================
@@ -21,7 +21,6 @@
 } while (0)
 #endif
 
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-mips/bug.h +0 -4

Index: linux-2.6.24-rc6/include/asm-mips/bug.h
===================================================================
@@ -12,8 +12,6 @@ do {									\
 	__asm__ __volatile__("break %0" : : "i" (BRK_BUG));		\
 } while (0)
 
-#define HAVE_ARCH_BUG
-
 #if (_MIPS_ISA > _MIPS_ISA_MIPS1)
 
 #define BUG_ON(condition)						\

@@ -22,8 +20,6 @@ do {									\
 			     : : "r" (condition), "i" (BRK_BUG));	\
 } while (0)
 
-#define HAVE_ARCH_BUG_ON
-
 #endif /* _MIPS_ISA > _MIPS_ISA_MIPS1 */
 
 #endif

include/asm-parisc/bug.h +0 -2

Index: linux-2.6.24-rc6/include/asm-parisc/bug.h
===================================================================
@@ -7,8 +7,6 @@
  */
 
 #ifdef CONFIG_BUG
-#define HAVE_ARCH_BUG
-#define HAVE_ARCH_WARN_ON
 
 /* the break instruction is used as BUG() marker.  */
 #define	PARISC_BUG_BREAK_ASM	"break 0x1f, 0x1fff"

include/asm-powerpc/bug.h +0 -3

Index: linux-2.6.24-rc6/include/asm-powerpc/bug.h
===================================================================
@@ -109,9 +109,6 @@
 	unlikely(__ret_warn_on);				\
 })
 
-#define HAVE_ARCH_BUG
-#define HAVE_ARCH_BUG_ON
-#define HAVE_ARCH_WARN_ON
 #endif /* __ASSEMBLY __ */
 #endif /* CONFIG_BUG */

include/asm-s390/bug.h +0 -2

Index: linux-2.6.24-rc6/include/asm-s390/bug.h
===================================================================
@@ -61,8 +61,6 @@
 	unlikely(__ret_warn_on);			\
 })
 
-#define HAVE_ARCH_BUG
-#define HAVE_ARCH_WARN_ON
 #endif /* CONFIG_BUG */
 
 #include <asm-generic/bug.h>

include/asm-sparc/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-sparc/bug.h
===================================================================
@@ -26,7 +26,6 @@ extern void do_BUG(const char *file, int
 #define BUG()		__bug_trap()
 #endif
 
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-sparc64/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-sparc64/bug.h
===================================================================
@@ -14,7 +14,6 @@ extern void do_BUG(const char *file, int
 #define BUG()		__builtin_trap()
 #endif
 
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-v850/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-v850/bug.h
===================================================================
@@ -17,7 +17,6 @@
 #ifdef CONFIG_BUG
 extern void __bug (void) __attribute__ ((noreturn));
 #define BUG()		__bug()
-#define HAVE_ARCH_BUG
 #endif
 
 #include <asm-generic/bug.h>

include/asm-x86/bug.h +0 -1

Index: linux-2.6.24-rc6/include/asm-x86/bug.h
===================================================================
@@ -2,7 +2,6 @@
 #define _ASM_X86_BUG_H
 
 #ifdef CONFIG_BUG
-#define HAVE_ARCH_BUG
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE

Russell King

Acked-by on v1 1/3

2008-01-17 08:37 UTC #2 lore raw

Re: [patch 1/3] bug.h: Remove HAVE_ARCH_BUG and HAVE_ARCH_WARN

Quoted 5 lines · Olof Johansson

No need to have the HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* defines, when the generic implementation can just use #ifndef on the macros themselves.
Signed-off-by Olof Johansson <[email protected]>
Cc <[email protected]>

For the ARM bit:

Acked-by Russell King <[email protected]>

Signature · 3 lines

Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Olof Johansson

author

2008-01-17 00:59 UTC #3 lore raw

PATCH 2/3 switch to generic WARN_ON / BUG_ON

In reply to Olof Johansson

Not using the ppc-specific WARN_ON/BUG_ON constructs actually saves about 4K text on a ppc64_defconfig. The main reason seems to be that prepping the arguments to the conditional trap instructions is more work than just doing a compare and branch.

Signed-off-by Olof Johansson <[email protected]>

Cc <[email protected]>

Cc Paul Mackerras <[email protected]>

---

 include/asm-powerpc/bug.h |   37 -------------------------------------
 1 file changed, 37 deletions(-)

include/asm-powerpc/bug.h +0 ~~-37~~

Index: linux-2.6.24-rc6/include/asm-powerpc/bug.h
===================================================================
@@ -54,12 +54,6 @@
 	".previous\n"
 #endif
 
-/*
- * BUG_ON() and WARN_ON() do their best to cooperate with compile-time
- * optimisations. However depending on the complexity of the condition
- * some compiler versions may not produce optimal results.
- */
-
 #define BUG() do {						\
 	__asm__ __volatile__(					\
 		"1:	twi 31,0,0\n"				\

@@ -69,20 +63,6 @@
 	for(;;) ;						\
 } while (0)
 
-#define BUG_ON(x) do {						\
-	if (__builtin_constant_p(x)) {				\
-		if (x)						\
-			BUG();					\
-	} else {						\
-		__asm__ __volatile__(				\
-		"1:	"PPC_TLNEI"	%4,0\n"			\
-		_EMIT_BUG_ENTRY					\
-		: : "i" (__FILE__), "i" (__LINE__), "i" (0),	\
-		  "i" (sizeof(struct bug_entry)),		\
-		  "r" ((__force long)(x)));			\
-	}							\
-} while (0)
-
 #define __WARN() do {						\
 	__asm__ __volatile__(					\
 		"1:	twi 31,0,0\n"				\

@@ -92,23 +72,6 @@
 		  "i" (sizeof(struct bug_entry)));		\
 } while (0)
 
-#define WARN_ON(x) ({						\
-	int __ret_warn_on = !!(x);				\
-	if (__builtin_constant_p(__ret_warn_on)) {		\
-		if (__ret_warn_on)				\
-			__WARN();				\
-	} else {						\
-		__asm__ __volatile__(				\
-		"1:	"PPC_TLNEI"	%4,0\n"			\
-		_EMIT_BUG_ENTRY					\
-		: : "i" (__FILE__), "i" (__LINE__),		\
-		  "i" (BUGFLAG_WARNING),			\
-		  "i" (sizeof(struct bug_entry)),		\
-		  "r" (__ret_warn_on));				\
-	}							\
-	unlikely(__ret_warn_on);				\
-})
-
 #endif /* __ASSEMBLY __ */
 #endif /* CONFIG_BUG */

Olof Johansson

author

2008-01-17 00:59 UTC #4 lore raw

PATCH 3/3 Add bug/warn marker to generic report_bug()

In reply to Olof Johansson

Powerpc uses the generic report_bug() from lib/bug.c to report warnings, and I'm guessing other arches do as well.

Add the module list as well as the end-of-trace marker to the output. This required making print_oops_end_marker() nonstatic.

Signed-off-by Olof Johansson <[email protected]>

include/linux/kernel.h +1 -0

index 94bc996..88d1aa3 100644
@@ -133,6 +133,7 @@ NORET_TYPE void panic(const char * fmt, ...)
 extern void oops_enter(void);
 extern void oops_exit(void);
 extern int oops_may_print(void);
+extern void print_oops_end_marker(void);
 fastcall NORET_TYPE void do_exit(long error_code)
 	ATTRIB_NORET;
 NORET_TYPE void complete_and_exit(struct completion *, long)

kernel/panic.c +1 -1

index d9e90cf..0269a7f 100644
@@ -281,7 +281,7 @@ static int init_oops_id(void)
 }
 late_initcall(init_oops_id);
 
-static void print_oops_end_marker(void)
+void print_oops_end_marker(void)
 {
 	init_oops_id();
 	printk(KERN_WARNING "---[ end trace %016llx ]---\n",

lib/bug.c +2 -0

index 530f38f..3aa60a5 100644
@@ -148,7 +148,9 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 			       "[verbose debug info unavailable]\n",
 			       (void *)bugaddr);
 
+		print_modules();
 		show_regs(regs);
+		print_oops_end_marker();
 		return BUG_TRAP_TYPE_WARN;
 	}

Arjan van de Ven

Acked-by on v1 3/3

2008-01-17 01:18 UTC #5 lore raw

Re: [patch 3/3] Add bug/warn marker to generic report_bug()

Quoted 10 lines · On Wed, 16 Jan 2008 19:10:00 -0600 Olof Johansson <[email protected]> wrote:

Powerpc uses the generic report_bug() from lib/bug.c to report warnings, and I'm guessing other arches do as well.
Add the module list as well as the end-of-trace marker to the output. This required making print_oops_end_marker() nonstatic.
Signed-off-by Olof Johansson <[email protected]>

All three are

Acked-by Arjan van de Ven <[email protected]>