public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [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 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 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  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

* 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 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

* [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

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