* [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN
@ 2007-10-11 17:12 Olof Johansson
2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Olof Johansson @ 2007-10-11 17:12 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, paulus, lethal, kyle, grundler
HAVE_ARCH_WARN is used to determine if an arch already has a __WARN()
macro, or if a generic one is needed.
With this, some of the arch-specific WARN_ON() implementations can be
made common instead (see follow-up patch for powerpc).
Signed-off-by: Olof Johansson <olof@lixom.net>
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index d56fedb..c6b8386 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -31,14 +31,19 @@ struct bug_entry {
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
#endif
+#ifndef HAVE_ARCH_WARN
+#define __WARN() do { \
+ printk("WARNING: at %s:%d %s()\n", __FILE__, \
+ __LINE__, __FUNCTION__); \
+ dump_stack(); \
+} while (0)
+#endif
+
#ifndef HAVE_ARCH_WARN_ON
#define WARN_ON(condition) ({ \
int __ret_warn_on = !!(condition); \
- if (unlikely(__ret_warn_on)) { \
- printk("WARNING: at %s:%d %s()\n", __FILE__, \
- __LINE__, __FUNCTION__); \
- dump_stack(); \
- } \
+ if (unlikely(__ret_warn_on)) \
+ __WARN(); \
unlikely(__ret_warn_on); \
})
#endif
diff --git a/include/asm-parisc/bug.h b/include/asm-parisc/bug.h
index 8cfc553..3f68100 100644
--- a/include/asm-parisc/bug.h
+++ b/include/asm-parisc/bug.h
@@ -8,6 +8,7 @@
#ifdef CONFIG_BUG
#define HAVE_ARCH_BUG
+#define HAVE_ARCH_WARN
#define HAVE_ARCH_WARN_ON
/* the break instruction is used as BUG() marker. */
diff --git a/include/asm-powerpc/bug.h b/include/asm-powerpc/bug.h
index e55d1f6..02e171c 100644
--- a/include/asm-powerpc/bug.h
+++ b/include/asm-powerpc/bug.h
@@ -110,6 +110,7 @@
})
#define HAVE_ARCH_BUG
+#define HAVE_ARCH_WARN
#define HAVE_ARCH_BUG_ON
#define HAVE_ARCH_WARN_ON
#endif /* __ASSEMBLY __ */
diff --git a/include/asm-sh/bug.h b/include/asm-sh/bug.h
index a78d482..4ea415b 100644
--- a/include/asm-sh/bug.h
+++ b/include/asm-sh/bug.h
@@ -5,6 +5,7 @@
#ifdef CONFIG_BUG
#define HAVE_ARCH_BUG
+#define HAVE_ARCH_WARN
#define HAVE_ARCH_WARN_ON
/**
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-11 17:12 [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Olof Johansson @ 2007-10-11 17:14 ` Olof Johansson 2007-10-11 19:55 ` Scott Wood ` (2 more replies) 2007-10-11 17:20 ` [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Kyle McMartin ` (3 subsequent siblings) 4 siblings, 3 replies; 13+ messages in thread From: Olof Johansson @ 2007-10-11 17:14 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, paulus, lethal, kyle, grundler, linuxppc-dev 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 <olof@lixom.net> diff --git a/include/asm-powerpc/bug.h b/include/asm-powerpc/bug.h index 02e171c..966de9b 100644 --- a/include/asm-powerpc/bug.h +++ b/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,27 +72,8 @@ "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); \ -}) - #define HAVE_ARCH_BUG #define HAVE_ARCH_WARN -#define HAVE_ARCH_BUG_ON -#define HAVE_ARCH_WARN_ON #endif /* __ASSEMBLY __ */ #endif /* CONFIG_BUG */ ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson @ 2007-10-11 19:55 ` Scott Wood 2007-10-12 1:23 ` Paul Mackerras 2007-10-19 2:03 ` [PATCH v2] [2/2] " Olof Johansson 2 siblings, 0 replies; 13+ messages in thread From: Scott Wood @ 2007-10-11 19:55 UTC (permalink / raw) To: Olof Johansson; +Cc: linux-kernel, grundler, kyle, linuxppc-dev, lethal, akpm On Thu, Oct 11, 2007 at 12:14:13PM -0500, Olof Johansson wrote: > 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. It'd be nice if we could get GCC to generate bug table entries for __builtin_trap(); that way we could use GCC's ability to put arbitrary conditions in the trap instruction. -Scott ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson 2007-10-11 19:55 ` Scott Wood @ 2007-10-12 1:23 ` Paul Mackerras 2007-10-12 2:04 ` Kyle McMartin 2007-10-12 2:40 ` Olof Johansson 2007-10-19 2:03 ` [PATCH v2] [2/2] " Olof Johansson 2 siblings, 2 replies; 13+ messages in thread From: Paul Mackerras @ 2007-10-12 1:23 UTC (permalink / raw) To: Olof Johansson; +Cc: linux-kernel, akpm, lethal, kyle, grundler, linuxppc-dev Olof Johansson writes: > 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. It might be more instructions but it takes fewer cycles, I would expect. Do you have the actual instruction sequences to compare? Paul. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-12 1:23 ` Paul Mackerras @ 2007-10-12 2:04 ` Kyle McMartin 2007-10-12 2:41 ` Olof Johansson 2007-10-12 2:40 ` Olof Johansson 1 sibling, 1 reply; 13+ messages in thread From: Kyle McMartin @ 2007-10-12 2:04 UTC (permalink / raw) To: Paul Mackerras Cc: Olof Johansson, linux-kernel, akpm, lethal, kyle, grundler, linuxppc-dev On Fri, Oct 12, 2007 at 11:23:39AM +1000, Paul Mackerras wrote: > Olof Johansson writes: > > > 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. > > It might be more instructions but it takes fewer cycles, I would > expect. Do you have the actual instruction sequences to compare? > I really hope WARN_ON/BUG_ON aren't hotpaths on powerpc. ;-) Cheers, Kyle ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-12 2:04 ` Kyle McMartin @ 2007-10-12 2:41 ` Olof Johansson 0 siblings, 0 replies; 13+ messages in thread From: Olof Johansson @ 2007-10-12 2:41 UTC (permalink / raw) To: Kyle McMartin Cc: Paul Mackerras, linux-kernel, akpm, lethal, kyle, grundler, linuxppc-dev On Thu, Oct 11, 2007 at 10:04:19PM -0400, Kyle McMartin wrote: > On Fri, Oct 12, 2007 at 11:23:39AM +1000, Paul Mackerras wrote: > > Olof Johansson writes: > > > > > 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. > > > > It might be more instructions but it takes fewer cycles, I would > > expect. Do you have the actual instruction sequences to compare? > > > > I really hope WARN_ON/BUG_ON aren't hotpaths on powerpc. ;-) Not the taken branch of them, no. :) But making it past them as fast as possible when they're not tripping is always good. -Olof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-12 1:23 ` Paul Mackerras 2007-10-12 2:04 ` Kyle McMartin @ 2007-10-12 2:40 ` Olof Johansson 1 sibling, 0 replies; 13+ messages in thread From: Olof Johansson @ 2007-10-12 2:40 UTC (permalink / raw) To: Paul Mackerras; +Cc: grundler, linux-kernel, kyle, linuxppc-dev, lethal, akpm On Fri, Oct 12, 2007 at 11:23:39AM +1000, Paul Mackerras wrote: > Olof Johansson writes: > > > 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. > > It might be more instructions but it takes fewer cycles, I would > expect. Do you have the actual instruction sequences to compare? Sure. Just looking at a couple of cases: On range comparisons such as (a < CONST): * without this patch, you end up with a comparison, then a cr-to-cr op + mfcr + mask operation + tdnei. * with the patch, you get a comparison and a conditional branch with twi out of line. On multiple comparisons like WARN_ON( a || b), it seems that there's an added li to make the tdnei always hit for the first case. There it uses a regular conditional branch, so no real difference in generated code besides that. Code examples. This was with a ppc64_defconfig, so CONFIG_POWER4_ONLY wasn't enabled. It uses mfocrf when it is, which is alot cheaper. Still, the rest of the code sequence is the same: void irq_free_virt(unsigned int virq, unsigned int count) { unsigned long flags; unsigned int i; WARN_ON (virq < NUM_ISA_INTERRUPTS); WARN_ON (count == 0 || (virq + count) > irq_virq_count); spin_lock_irqsave(&irq_big_lock, flags); Without the patch: c00000000000b33c <.irq_free_virt>: c00000000000b33c: 7c 08 02 a6 mflr r0 c00000000000b340: 2b 83 00 0f cmplwi cr7,r3,15 c00000000000b344: fb c1 ff f0 std r30,-16(r1) c00000000000b348: fb e1 ff f8 std r31,-8(r1) c00000000000b34c: 7c 9e 23 78 mr r30,r4 c00000000000b350: 7c 7f 1b 78 mr r31,r3 c00000000000b354: 4f dd e8 42 crnot 4*cr7+eq,4*cr7+gt c00000000000b358: f8 01 00 10 std r0,16(r1) c00000000000b35c: f8 21 ff 81 stdu r1,-128(r1) c00000000000b360: 7c 00 00 26 mfcr r0 c00000000000b364: 54 00 ff fe rlwinm r0,r0,31,31,31 c00000000000b368: 0b 00 00 00 tdnei r0,0 c00000000000b36c: 2f a4 00 00 cmpdi cr7,r4,0 c00000000000b370: 38 00 00 01 li r0,1 c00000000000b374: 41 9e 00 1c beq cr7,c00000000000b390 <.irq_free_virt+0x54> c00000000000b378: e9 22 80 d8 ld r9,-32552(r2) c00000000000b37c: 7d 63 22 14 add r11,r3,r4 c00000000000b380: 80 09 00 00 lwz r0,0(r9) c00000000000b384: 7f 8b 00 40 cmplw cr7,r11,r0 c00000000000b388: 7c 00 00 26 mfcr r0 c00000000000b38c: 54 00 f7 fe rlwinm r0,r0,30,31,31 c00000000000b390: 0b 00 00 00 tdnei r0,0 c00000000000b394: e8 62 80 f8 ld r3,-32520(r2) c00000000000b398: 48 54 4c b1 bl c000000000550048 <._spin_lock_irqsave> c00000000000b39c: 60 00 00 00 nop With the patch: c00000000000b0b0 <.irq_free_virt>: c00000000000b0b0: 7c 08 02 a6 mflr r0 c00000000000b0b4: 2b 83 00 0f cmplwi cr7,r3,15 c00000000000b0b8: fb c1 ff f0 std r30,-16(r1) c00000000000b0bc: fb e1 ff f8 std r31,-8(r1) c00000000000b0c0: 7c 9e 23 78 mr r30,r4 c00000000000b0c4: 7c 7f 1b 78 mr r31,r3 c00000000000b0c8: f8 01 00 10 std r0,16(r1) c00000000000b0cc: f8 21 ff 81 stdu r1,-128(r1) c00000000000b0d0: 41 bd 00 08 bgt cr7,c00000000000b0d8 <.irq_free_virt+0x28> c00000000000b0d4: 0f e0 00 00 twi 31,r0,0 c00000000000b0d8: 2f be 00 00 cmpdi cr7,r30,0 c00000000000b0dc: 41 9e 00 18 beq cr7,c00000000000b0f4 <.irq_free_virt+0x44> c00000000000b0e0: e9 22 80 d8 ld r9,-32552(r2) c00000000000b0e4: 7d 7f f2 14 add r11,r31,r30 c00000000000b0e8: 80 09 00 00 lwz r0,0(r9) c00000000000b0ec: 7f 8b 00 40 cmplw cr7,r11,r0 c00000000000b0f0: 40 bd 00 08 ble cr7,c00000000000b0f8 <.irq_free_virt+0x48> c00000000000b0f4: 0f e0 00 00 twi 31,r0,0 c00000000000b0f8: e8 62 80 f8 ld r3,-32520(r2) c00000000000b0fc: 48 54 3f 75 bl c00000000054f070 <._spin_lock_irqsave> c00000000000b100: 60 00 00 00 nop ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] [2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() 2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson 2007-10-11 19:55 ` Scott Wood 2007-10-12 1:23 ` Paul Mackerras @ 2007-10-19 2:03 ` Olof Johansson 2 siblings, 0 replies; 13+ messages in thread From: Olof Johansson @ 2007-10-19 2:03 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, paulus, lethal, kyle, grundler, linuxppc-dev [POWERPC] Switch to generic WARN_ON()/BUG_ON() 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 <olof@lixom.net> Index: k.org/include/asm-powerpc/bug.h =================================================================== --- k.org.orig/include/asm-powerpc/bug.h +++ k.org/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 */ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN 2007-10-11 17:12 [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Olof Johansson 2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson @ 2007-10-11 17:20 ` Kyle McMartin 2007-10-12 2:53 ` Paul Mundt ` (2 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: Kyle McMartin @ 2007-10-11 17:20 UTC (permalink / raw) To: Olof Johansson; +Cc: linux-kernel, akpm, paulus, lethal, kyle, grundler On Thu, Oct 11, 2007 at 12:12:11PM -0500, Olof Johansson wrote: > HAVE_ARCH_WARN is used to determine if an arch already has a __WARN() > macro, or if a generic one is needed. > > With this, some of the arch-specific WARN_ON() implementations can be > made common instead (see follow-up patch for powerpc). > > Signed-off-by: Olof Johansson <olof@lixom.net> > ack parisc hunk (obviously :) cheers, kyle ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN 2007-10-11 17:12 [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Olof Johansson 2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson 2007-10-11 17:20 ` [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Kyle McMartin @ 2007-10-12 2:53 ` Paul Mundt 2007-10-18 22:17 ` Andrew Morton 2007-10-19 2:03 ` [PATCH v2] [1/2] bug.h: Remove HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* Olof Johansson 4 siblings, 0 replies; 13+ messages in thread From: Paul Mundt @ 2007-10-12 2:53 UTC (permalink / raw) To: Olof Johansson; +Cc: linux-kernel, akpm, paulus, kyle, grundler On Thu, Oct 11, 2007 at 12:12:11PM -0500, Olof Johansson wrote: > HAVE_ARCH_WARN is used to determine if an arch already has a __WARN() > macro, or if a generic one is needed. > > With this, some of the arch-specific WARN_ON() implementations can be > made common instead (see follow-up patch for powerpc). > > Signed-off-by: Olof Johansson <olof@lixom.net> > sh bits are fine. Acked-by: Paul Mundt <lethal@linux-sh.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN 2007-10-11 17:12 [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Olof Johansson ` (2 preceding siblings ...) 2007-10-12 2:53 ` Paul Mundt @ 2007-10-18 22:17 ` Andrew Morton 2007-10-18 22:34 ` Olof Johansson 2007-10-19 2:03 ` [PATCH v2] [1/2] bug.h: Remove HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* Olof Johansson 4 siblings, 1 reply; 13+ messages in thread From: Andrew Morton @ 2007-10-18 22:17 UTC (permalink / raw) To: Olof Johansson; +Cc: linux-kernel, paulus, lethal, kyle, grundler On Thu, 11 Oct 2007 12:12:11 -0500 Olof Johansson <olof@lixom.net> wrote: > HAVE_ARCH_WARN is used to determine if an arch already has a __WARN() > macro, or if a generic one is needed. > > With this, some of the arch-specific WARN_ON() implementations can be > made common instead (see follow-up patch for powerpc). > Those HAVE_ARCH_foo things are unpleasant. > > diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h > index d56fedb..c6b8386 100644 > --- a/include/asm-generic/bug.h > +++ b/include/asm-generic/bug.h > @@ -31,14 +31,19 @@ struct bug_entry { > #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0) > #endif > > +#ifndef HAVE_ARCH_WARN > +#define __WARN() do { \ > + printk("WARNING: at %s:%d %s()\n", __FILE__, \ > + __LINE__, __FUNCTION__); \ > + dump_stack(); \ > +} while (0) > +#endif Can't we just do #ifndef __WARN? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN 2007-10-18 22:17 ` Andrew Morton @ 2007-10-18 22:34 ` Olof Johansson 0 siblings, 0 replies; 13+ messages in thread From: Olof Johansson @ 2007-10-18 22:34 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, paulus, lethal, kyle, grundler On Thu, Oct 18, 2007 at 03:17:52PM -0700, Andrew Morton wrote: > On Thu, 11 Oct 2007 12:12:11 -0500 > Olof Johansson <olof@lixom.net> wrote: > > > HAVE_ARCH_WARN is used to determine if an arch already has a __WARN() > > macro, or if a generic one is needed. > > > > With this, some of the arch-specific WARN_ON() implementations can be > > made common instead (see follow-up patch for powerpc). > > > > Those HAVE_ARCH_foo things are unpleasant. [...] > Can't we just do #ifndef __WARN? Yep, good idea. I'll respin. -Olof ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] [1/2] bug.h: Remove HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* 2007-10-11 17:12 [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Olof Johansson ` (3 preceding siblings ...) 2007-10-18 22:17 ` Andrew Morton @ 2007-10-19 2:03 ` Olof Johansson 4 siblings, 0 replies; 13+ messages in thread From: Olof Johansson @ 2007-10-19 2:03 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, paulus, lethal, kyle, grundler, linux-arch No need to have the HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* defines, when the generic implementation can just use #ifndef on the macros themselves. Also, introduce __WARN() in the generic case, so the generic WARN_ON() can use arch-specific code for when the condition is true. Built on powerpc, i386, sh and sparc64. Signed-off-by: Olof Johansson <olof@lixom.net> Index: k.org/include/asm-generic/bug.h =================================================================== --- k.org.orig/include/asm-generic/bug.h +++ k.org/include/asm-generic/bug.h @@ -20,39 +20,44 @@ 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 -#ifndef HAVE_ARCH_WARN_ON +#ifndef __WARN +#define __WARN() do { \ + printk("WARNING: at %s:%d %s()\n", __FILE__, \ + __LINE__, __FUNCTION__); \ + dump_stack(); \ +} while (0) +#endif + +#ifndef WARN_ON #define WARN_ON(condition) ({ \ int __ret_warn_on = !!(condition); \ - if (unlikely(__ret_warn_on)) { \ - printk("WARNING: at %s:%d %s()\n", __FILE__, \ - __LINE__, __FUNCTION__); \ - dump_stack(); \ - } \ + if (unlikely(__ret_warn_on)) \ + __WARN(); \ unlikely(__ret_warn_on); \ }) #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); \ Index: k.org/include/asm-parisc/bug.h =================================================================== --- k.org.orig/include/asm-parisc/bug.h +++ k.org/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" Index: k.org/include/asm-powerpc/bug.h =================================================================== --- k.org.orig/include/asm-powerpc/bug.h +++ k.org/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 */ Index: k.org/include/asm-sh/bug.h =================================================================== --- k.org.orig/include/asm-sh/bug.h +++ k.org/include/asm-sh/bug.h @@ -4,8 +4,6 @@ #define TRAPA_BUG_OPCODE 0xc33e /* trapa #0x3e */ #ifdef CONFIG_BUG -#define HAVE_ARCH_BUG -#define HAVE_ARCH_WARN_ON /** * _EMIT_BUG_ENTRY Index: k.org/include/asm-alpha/bug.h =================================================================== --- k.org.orig/include/asm-alpha/bug.h +++ k.org/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> Index: k.org/include/asm-arm/bug.h =================================================================== --- k.org.orig/include/asm-arm/bug.h +++ k.org/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> Index: k.org/include/asm-avr32/bug.h =================================================================== --- k.org.orig/include/asm-avr32/bug.h +++ k.org/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> Index: k.org/include/asm-frv/bug.h =================================================================== --- k.org.orig/include/asm-frv/bug.h +++ k.org/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(); \ Index: k.org/include/asm-ia64/bug.h =================================================================== --- k.org.orig/include/asm-ia64/bug.h +++ k.org/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> Index: k.org/include/asm-m68k/bug.h =================================================================== --- k.org.orig/include/asm-m68k/bug.h +++ k.org/include/asm-m68k/bug.h @@ -21,7 +21,6 @@ } while (0) #endif -#define HAVE_ARCH_BUG #endif #include <asm-generic/bug.h> Index: k.org/include/asm-mips/bug.h =================================================================== --- k.org.orig/include/asm-mips/bug.h +++ k.org/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 Index: k.org/include/asm-s390/bug.h =================================================================== --- k.org.orig/include/asm-s390/bug.h +++ k.org/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> Index: k.org/include/asm-sh64/bug.h =================================================================== --- k.org.orig/include/asm-sh64/bug.h +++ k.org/include/asm-sh64/bug.h @@ -11,7 +11,6 @@ *(volatile int *)0 = 0; \ } while (0) -#define HAVE_ARCH_BUG #endif #include <asm-generic/bug.h> Index: k.org/include/asm-sparc/bug.h =================================================================== --- k.org.orig/include/asm-sparc/bug.h +++ k.org/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> Index: k.org/include/asm-sparc64/bug.h =================================================================== --- k.org.orig/include/asm-sparc64/bug.h +++ k.org/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> Index: k.org/include/asm-v850/bug.h =================================================================== --- k.org.orig/include/asm-v850/bug.h +++ k.org/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> Index: k.org/include/asm-x86/bug.h =================================================================== --- k.org.orig/include/asm-x86/bug.h +++ k.org/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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2007-10-19 1:57 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-11 17:12 [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Olof Johansson 2007-10-11 17:14 ` [PATCH 2/2] powerpc: Switch to generic WARN_ON()/BUG_ON() Olof Johansson 2007-10-11 19:55 ` Scott Wood 2007-10-12 1:23 ` Paul Mackerras 2007-10-12 2:04 ` Kyle McMartin 2007-10-12 2:41 ` Olof Johansson 2007-10-12 2:40 ` Olof Johansson 2007-10-19 2:03 ` [PATCH v2] [2/2] " Olof Johansson 2007-10-11 17:20 ` [PATCH 1/2] bug.h: Introduce HAVE_ARCH_WARN Kyle McMartin 2007-10-12 2:53 ` Paul Mundt 2007-10-18 22:17 ` Andrew Morton 2007-10-18 22:34 ` Olof Johansson 2007-10-19 2:03 ` [PATCH v2] [1/2] bug.h: Remove HAVE_ARCH_BUG.* / HAVE_ARCH_WARN.* Olof Johansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox