* [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family
@ 2014-02-26 3:48 Josh Triplett
2014-02-26 3:48 ` [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Josh Triplett @ 2014-02-26 3:48 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, linux-arch, linux-kernel
When !CONFIG_BUG, WARN_ON and family become simple passthroughs of their
condition argument; however, WARN_ON_ONCE and family still have
conditions and a boolean to detect one-time invocation, even though the
warning they'd emit doesn't exist. Make the existing definitions
conditional on CONFIG_BUG, and add definitions for !CONFIG_BUG that map
to the passthrough versions of WARN and WARN_ON.
This saves 4.4k on a minimized configuration (smaller than
allnoconfig), and 20.6k with defconfig plus CONFIG_BUG=n.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v2: Incorporate feedback from Arnd Bergmann: make the WARN_* variants with
format strings and arguments call WARN and pass along those arguments,
rather than calling WARN_ON. Used by the new patch 3, which makes the stub
version of WARN call no_printk.
include/asm-generic/bug.h | 57 +++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7d10f96..7ecd398 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -106,33 +106,6 @@ extern void warn_slowpath_null(const char *file, const int line);
unlikely(__ret_warn_on); \
})
-#else /* !CONFIG_BUG */
-#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
-#endif
-
-#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
-#endif
-
-#ifndef HAVE_ARCH_WARN_ON
-#define WARN_ON(condition) ({ \
- int __ret_warn_on = !!(condition); \
- unlikely(__ret_warn_on); \
-})
-#endif
-
-#ifndef WARN
-#define WARN(condition, format...) ({ \
- int __ret_warn_on = !!(condition); \
- unlikely(__ret_warn_on); \
-})
-#endif
-
-#define WARN_TAINT(condition, taint, format...) WARN_ON(condition)
-
-#endif
-
#define WARN_ON_ONCE(condition) ({ \
static bool __section(.data.unlikely) __warned; \
int __ret_warn_once = !!(condition); \
@@ -163,6 +136,36 @@ extern void warn_slowpath_null(const char *file, const int line);
unlikely(__ret_warn_once); \
})
+#else /* !CONFIG_BUG */
+#ifndef HAVE_ARCH_BUG
+#define BUG() do {} while(0)
+#endif
+
+#ifndef HAVE_ARCH_BUG_ON
+#define BUG_ON(condition) do { if (condition) ; } while(0)
+#endif
+
+#ifndef HAVE_ARCH_WARN_ON
+#define WARN_ON(condition) ({ \
+ int __ret_warn_on = !!(condition); \
+ unlikely(__ret_warn_on); \
+})
+#endif
+
+#ifndef WARN
+#define WARN(condition, format...) ({ \
+ int __ret_warn_on = !!(condition); \
+ unlikely(__ret_warn_on); \
+})
+#endif
+
+#define WARN_ON_ONCE(condition) WARN_ON(condition)
+#define WARN_ONCE(condition, format...) WARN(condition, format)
+#define WARN_TAINT(condition, taint, format...) WARN(condition, format)
+#define WARN_TAINT_ONCE(condition, taint, format...) WARN(condition, format)
+
+#endif
+
/*
* WARN_ON_SMP() is for cases that the warning is either
* meaningless for !SMP or may even cause failures.
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/
2014-02-26 3:48 [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
@ 2014-02-26 3:48 ` Josh Triplett
2014-02-26 13:25 ` Arnd Bergmann
2014-02-26 3:49 ` [PATCH v2 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
` (3 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Josh Triplett @ 2014-02-26 3:48 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, linux-arch, linux-kernel
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
include/asm-generic/bug.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7ecd398..2d54d8d 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -52,7 +52,7 @@ struct bug_entry {
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
+#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
#endif
/*
@@ -138,11 +138,11 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
+#define BUG() do {} while (0)
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) do { if (condition) ; } while (0)
#endif
#ifndef HAVE_ARCH_WARN_ON
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args
2014-02-26 3:48 [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
2014-02-26 3:48 ` [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
@ 2014-02-26 3:49 ` Josh Triplett
2014-02-26 13:25 ` Arnd Bergmann
2014-02-26 3:49 ` [PATCH v2 4/5] bug: Use a common definition of BUG_ON regardless of CONFIG_BUG Josh Triplett
` (2 subsequent siblings)
4 siblings, 1 reply; 21+ messages in thread
From: Josh Triplett @ 2014-02-26 3:49 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, linux-arch, linux-kernel
The stub version of WARN for !CONFIG_BUG completely ignored its format
string and subsequent arguments; make it check them instead, using
no_printk.
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
include/asm-generic/bug.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 2d54d8d..a97fa11 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -155,6 +155,7 @@ extern void warn_slowpath_null(const char *file, const int line);
#ifndef WARN
#define WARN(condition, format...) ({ \
int __ret_warn_on = !!(condition); \
+ no_printk(format); \
unlikely(__ret_warn_on); \
})
#endif
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 4/5] bug: Use a common definition of BUG_ON regardless of CONFIG_BUG
2014-02-26 3:48 [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
2014-02-26 3:48 ` [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
2014-02-26 3:49 ` [PATCH v2 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
@ 2014-02-26 3:49 ` Josh Triplett
2014-02-26 13:26 ` Arnd Bergmann
2014-02-26 3:49 ` [PATCH v2 5/5] bug: Make BUG() call unreachable() Josh Triplett
2014-02-26 13:24 ` [PATCH v2 " Arnd Bergmann
4 siblings, 1 reply; 21+ messages in thread
From: Josh Triplett @ 2014-02-26 3:49 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, linux-arch, linux-kernel
include/asm-generic/bug.h defines BUG_ON to call BUG() if CONFIG_BUG=y,
or as a no-op if !CONFIG_BUG. However, BUG() is already a no-op if
!CONFIG_BUG, making this pointless. Use a common definition that always
calls BUG().
This does not change the compiled code at all.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
include/asm-generic/bug.h | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index a97fa11..653c44a 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -51,10 +51,6 @@ struct bug_entry {
} while (0)
#endif
-#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
-#endif
-
/*
* WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
* significant issues that need prompt attention if they should ever
@@ -141,10 +137,6 @@ extern void warn_slowpath_null(const char *file, const int line);
#define BUG() do {} while (0)
#endif
-#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while (0)
-#endif
-
#ifndef HAVE_ARCH_WARN_ON
#define WARN_ON(condition) ({ \
int __ret_warn_on = !!(condition); \
@@ -167,6 +159,10 @@ extern void warn_slowpath_null(const char *file, const int line);
#endif
+#ifndef HAVE_ARCH_BUG_ON
+#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
+#endif
+
/*
* WARN_ON_SMP() is for cases that the warning is either
* meaningless for !SMP or may even cause failures.
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 5/5] bug: Make BUG() call unreachable()
2014-02-26 3:48 [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
` (2 preceding siblings ...)
2014-02-26 3:49 ` [PATCH v2 4/5] bug: Use a common definition of BUG_ON regardless of CONFIG_BUG Josh Triplett
@ 2014-02-26 3:49 ` Josh Triplett
2014-02-26 13:29 ` Arnd Bergmann
2014-02-26 13:24 ` [PATCH v2 " Arnd Bergmann
4 siblings, 1 reply; 21+ messages in thread
From: Josh Triplett @ 2014-02-26 3:49 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, linux-arch, linux-kernel
This effectively causes BUG() to act like a function with the noreturn
attribute, which prevents GCC from warning about the code that follows
BUG() (for instance, warning about not returning a value in a non-void
function after calling BUG()).
This actually makes the kernel smaller; bloat-o-meter summary:
add/remove: 2/7 grow/shrink: 34/57 up/down: 475/-1233 (-758)
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
include/asm-generic/bug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 653c44a..5f69248 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -134,7 +134,7 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while (0)
+#define BUG() do { unreachable(); } while (0)
#endif
#ifndef HAVE_ARCH_WARN_ON
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family
2014-02-26 3:48 [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
` (3 preceding siblings ...)
2014-02-26 3:49 ` [PATCH v2 5/5] bug: Make BUG() call unreachable() Josh Triplett
@ 2014-02-26 13:24 ` Arnd Bergmann
4 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-26 13:24 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wednesday 26 February 2014, Josh Triplett wrote:
> When !CONFIG_BUG, WARN_ON and family become simple passthroughs of their
> condition argument; however, WARN_ON_ONCE and family still have
> conditions and a boolean to detect one-time invocation, even though the
> warning they'd emit doesn't exist. Make the existing definitions
> conditional on CONFIG_BUG, and add definitions for !CONFIG_BUG that map
> to the passthrough versions of WARN and WARN_ON.
>
> This saves 4.4k on a minimized configuration (smaller than
> allnoconfig), and 20.6k with defconfig plus CONFIG_BUG=n.
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/
2014-02-26 3:48 ` [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
@ 2014-02-26 13:25 ` Arnd Bergmann
0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-26 13:25 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wednesday 26 February 2014, Josh Triplett wrote:
>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args
2014-02-26 3:49 ` [PATCH v2 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
@ 2014-02-26 13:25 ` Arnd Bergmann
0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-26 13:25 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wednesday 26 February 2014, Josh Triplett wrote:
> The stub version of WARN for !CONFIG_BUG completely ignored its format
> string and subsequent arguments; make it check them instead, using
> no_printk.
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/5] bug: Use a common definition of BUG_ON regardless of CONFIG_BUG
2014-02-26 3:49 ` [PATCH v2 4/5] bug: Use a common definition of BUG_ON regardless of CONFIG_BUG Josh Triplett
@ 2014-02-26 13:26 ` Arnd Bergmann
0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-26 13:26 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wednesday 26 February 2014, Josh Triplett wrote:
>
> include/asm-generic/bug.h defines BUG_ON to call BUG() if CONFIG_BUG=y,
> or as a no-op if !CONFIG_BUG. However, BUG() is already a no-op if
> !CONFIG_BUG, making this pointless. Use a common definition that always
> calls BUG().
>
> This does not change the compiled code at all.
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] bug: Make BUG() call unreachable()
2014-02-26 3:49 ` [PATCH v2 5/5] bug: Make BUG() call unreachable() Josh Triplett
@ 2014-02-26 13:29 ` Arnd Bergmann
2014-02-26 14:58 ` Josh Triplett
0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-26 13:29 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wednesday 26 February 2014, Josh Triplett wrote:
> @@ -134,7 +134,7 @@ extern void warn_slowpath_null(const char *file, const int line);
>
> #else /* !CONFIG_BUG */
> #ifndef HAVE_ARCH_BUG
> -#define BUG() do {} while (0)
> +#define BUG() do { unreachable(); } while (0)
> #endif
I disagree with this one. As Alan said, we really want to use an
arch specific BUG() even in the !CONFIG_BUG case.
For the cases where this is not yet possible, I'd suggest using
#define BUG() do { } while (1)
For older gcc versions, this is actually the same as unreachable(), but
it is not the same as __builtin_unreachable, because it causes undefined
behavior.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] bug: Make BUG() call unreachable()
2014-02-26 13:29 ` Arnd Bergmann
@ 2014-02-26 14:58 ` Josh Triplett
2014-02-27 19:19 ` Arnd Bergmann
0 siblings, 1 reply; 21+ messages in thread
From: Josh Triplett @ 2014-02-26 14:58 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wed, Feb 26, 2014 at 02:29:06PM +0100, Arnd Bergmann wrote:
> On Wednesday 26 February 2014, Josh Triplett wrote:
> > @@ -134,7 +134,7 @@ extern void warn_slowpath_null(const char *file, const int line);
> >
> > #else /* !CONFIG_BUG */
> > #ifndef HAVE_ARCH_BUG
> > -#define BUG() do {} while (0)
> > +#define BUG() do { unreachable(); } while (0)
> > #endif
>
> I disagree with this one. As Alan said, we really want to use an
> arch specific BUG() even in the !CONFIG_BUG case.
Possibly, but when doing so on an arch-by-arch basis, we'd need to make
sure that either there's a sensible trap handler for whatever trap it
invokes (for instance, ud2), or that there's some kind of useful
behavior otherwise (for instance, a reboot). Hence why I didn't make
any attempt to add architecture-specific patches in this series.
In any case, I tried a quick test of that on x86 below, along with the
generic equivalent.
> For the cases where this is not yet possible, I'd suggest using
>
> #define BUG() do { } while (1)
I just tested this, and:
$ scripts/bloat-o-meter vmlinux-nobug-base vmlinux-nobug-loop
add/remove: 2/1 grow/shrink: 247/33 up/down: 5461/-604 (4857)
In particular:
$ scripts/bloat-o-meter vmlinux-nobug-unreachable vmlinux-nobug-loop
add/remove: 6/0 grow/shrink: 261/9 up/down: 5679/-64 (5615)
So, some functions do get the optimizations from GCC treating the code
after an infinite loop as unreachable, but overall the infinite loops
themselves (and the conditionals around them in the more common case of
BUG_ON) are non-trivially large.
I tried a quick hack that used:
#define BUG() do { asm("ud2"); unreachable(); } while (0)
in place of the infinite loop, and got:
$ scripts/bloat-o-meter vmlinux-nobug-base vmlinux-nobug-ud2-unreachable
add/remove: 2/1 grow/shrink: 250/37 up/down: 4874/-606 (4268)
So, very little savings there compared to the infinite loop (unsurprising,
since ud2 is two bytes, and so is "1: jmp 1b").
This doesn't seem any different than compiling out assert() at runtime
in a userspace program, given how the kernel uses BUG() and BUG_ON().
I'd argue that adding unreachable() doesn't seem like it makes the
current implementation of BUG() any worse; either way if you reach it
you have a problem.
- Josh Triplett
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] bug: Make BUG() call unreachable()
2014-02-26 14:58 ` Josh Triplett
@ 2014-02-27 19:19 ` Arnd Bergmann
2014-02-28 0:16 ` Josh Triplett
0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-27 19:19 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Wednesday 26 February 2014, Josh Triplett wrote:
> This doesn't seem any different than compiling out assert() at runtime
> in a userspace program, given how the kernel uses BUG() and BUG_ON().
> I'd argue that adding unreachable() doesn't seem like it makes the
> current implementation of BUG() any worse; either way if you reach it
> you have a problem.
I think it's better to get a warning about undefined behavior than
to suppress that warning.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] bug: Make BUG() call unreachable()
2014-02-27 19:19 ` Arnd Bergmann
@ 2014-02-28 0:16 ` Josh Triplett
2014-02-28 8:55 ` Arnd Bergmann
0 siblings, 1 reply; 21+ messages in thread
From: Josh Triplett @ 2014-02-28 0:16 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Andrew Morton, linux-arch, linux-kernel
On Thu, Feb 27, 2014 at 08:19:47PM +0100, Arnd Bergmann wrote:
> On Wednesday 26 February 2014, Josh Triplett wrote:
> > This doesn't seem any different than compiling out assert() at runtime
> > in a userspace program, given how the kernel uses BUG() and BUG_ON().
> > I'd argue that adding unreachable() doesn't seem like it makes the
> > current implementation of BUG() any worse; either way if you reach it
> > you have a problem.
>
> I think it's better to get a warning about undefined behavior than
> to suppress that warning.
Then at this point I'm going to suggest that you go ahead and submit the
patch you want on top of the first four patches of this series. Please
keep in mind the value and code size savings of !CONFIG_BUG, versus
CONFIG_BUG=y and !CONFIG_DEBUG_BUGVERBOSE; those mean two different
things.
Meanwhile: Andrew, could you go ahead and apply the first four patches?
- Josh Triplett
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] bug: Make BUG() call unreachable()
2014-02-28 0:16 ` Josh Triplett
@ 2014-02-28 8:55 ` Arnd Bergmann
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2014-02-28 8:55 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Thursday 27 February 2014 16:16:45 Josh Triplett wrote:
> On Thu, Feb 27, 2014 at 08:19:47PM +0100, Arnd Bergmann wrote:
> > On Wednesday 26 February 2014, Josh Triplett wrote:
> > > This doesn't seem any different than compiling out assert() at runtime
> > > in a userspace program, given how the kernel uses BUG() and BUG_ON().
> > > I'd argue that adding unreachable() doesn't seem like it makes the
> > > current implementation of BUG() any worse; either way if you reach it
> > > you have a problem.
> >
> > I think it's better to get a warning about undefined behavior than
> > to suppress that warning.
>
> Then at this point I'm going to suggest that you go ahead and submit the
> patch you want on top of the first four patches of this series.
Sure, no problem. I'll wait for your patches to show up in linux-next
and then do a patch on top. I'll be traveling for the next week, so
it may get delayed another few days.
> Please keep in mind the value and code size savings of !CONFIG_BUG, versus
> CONFIG_BUG=y and !CONFIG_DEBUG_BUGVERBOSE; those mean two different
> things.
I think I compared all the options before in the patch I cited,
https://lkml.org/lkml/2013/7/5/222 but I agree that the list
is a bit confusing.
> Meanwhile: Andrew, could you go ahead and apply the first four patches?
Yes please.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family
2014-02-28 8:55 ` Arnd Bergmann
@ 2014-03-10 1:00 ` Josh Triplett
2014-03-10 1:01 ` [PATCH v3 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
` (4 more replies)
0 siblings, 5 replies; 21+ messages in thread
From: Josh Triplett @ 2014-03-10 1:00 UTC (permalink / raw)
To: Andrew Morton; +Cc: Arnd Bergmann, linux-arch, linux-kernel
When !CONFIG_BUG, WARN_ON and family become simple passthroughs of their
condition argument; however, WARN_ON_ONCE and family still have
conditions and a boolean to detect one-time invocation, even though the
warning they'd emit doesn't exist. Make the existing definitions
conditional on CONFIG_BUG, and add definitions for !CONFIG_BUG that map
to the passthrough versions of WARN and WARN_ON.
This saves 4.4k on a minimized configuration (smaller than
allnoconfig), and 20.6k with defconfig plus CONFIG_BUG=n.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v3: Patch unchanged from v2.
Andrew, can you please replace the entire v2 series currently in -mm
with this new series?
include/asm-generic/bug.h | 57 +++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7d10f96..7ecd398 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -106,33 +106,6 @@ extern void warn_slowpath_null(const char *file, const int line);
unlikely(__ret_warn_on); \
})
-#else /* !CONFIG_BUG */
-#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
-#endif
-
-#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
-#endif
-
-#ifndef HAVE_ARCH_WARN_ON
-#define WARN_ON(condition) ({ \
- int __ret_warn_on = !!(condition); \
- unlikely(__ret_warn_on); \
-})
-#endif
-
-#ifndef WARN
-#define WARN(condition, format...) ({ \
- int __ret_warn_on = !!(condition); \
- unlikely(__ret_warn_on); \
-})
-#endif
-
-#define WARN_TAINT(condition, taint, format...) WARN_ON(condition)
-
-#endif
-
#define WARN_ON_ONCE(condition) ({ \
static bool __section(.data.unlikely) __warned; \
int __ret_warn_once = !!(condition); \
@@ -163,6 +136,36 @@ extern void warn_slowpath_null(const char *file, const int line);
unlikely(__ret_warn_once); \
})
+#else /* !CONFIG_BUG */
+#ifndef HAVE_ARCH_BUG
+#define BUG() do {} while(0)
+#endif
+
+#ifndef HAVE_ARCH_BUG_ON
+#define BUG_ON(condition) do { if (condition) ; } while(0)
+#endif
+
+#ifndef HAVE_ARCH_WARN_ON
+#define WARN_ON(condition) ({ \
+ int __ret_warn_on = !!(condition); \
+ unlikely(__ret_warn_on); \
+})
+#endif
+
+#ifndef WARN
+#define WARN(condition, format...) ({ \
+ int __ret_warn_on = !!(condition); \
+ unlikely(__ret_warn_on); \
+})
+#endif
+
+#define WARN_ON_ONCE(condition) WARN_ON(condition)
+#define WARN_ONCE(condition, format...) WARN(condition, format)
+#define WARN_TAINT(condition, taint, format...) WARN(condition, format)
+#define WARN_TAINT_ONCE(condition, taint, format...) WARN(condition, format)
+
+#endif
+
/*
* WARN_ON_SMP() is for cases that the warning is either
* meaningless for !SMP or may even cause failures.
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
@ 2014-03-10 1:01 ` Josh Triplett
2014-03-10 1:02 ` [PATCH v3 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
` (3 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Josh Triplett @ 2014-03-10 1:01 UTC (permalink / raw)
To: Andrew Morton; +Cc: Arnd Bergmann, linux-arch, linux-kernel
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v3: Patch unchanged from v2.
include/asm-generic/bug.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 7ecd398..2d54d8d 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -52,7 +52,7 @@ struct bug_entry {
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)
+#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
#endif
/*
@@ -138,11 +138,11 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while(0)
+#define BUG() do {} while (0)
#endif
#ifndef HAVE_ARCH_BUG_ON
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) do { if (condition) ; } while (0)
#endif
#ifndef HAVE_ARCH_WARN_ON
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
2014-03-10 1:01 ` [PATCH v3 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
@ 2014-03-10 1:02 ` Josh Triplett
2014-03-10 1:02 ` [PATCH v3 4/5] bug: Make BUG() always stop the machine Josh Triplett
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Josh Triplett @ 2014-03-10 1:02 UTC (permalink / raw)
To: Andrew Morton; +Cc: Arnd Bergmann, linux-arch, linux-kernel
The stub version of WARN for !CONFIG_BUG completely ignored its format
string and subsequent arguments; make it check them instead, using
no_printk.
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v3: Patch unchanged from v2.
include/asm-generic/bug.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 2d54d8d..a97fa11 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -155,6 +155,7 @@ extern void warn_slowpath_null(const char *file, const int line);
#ifndef WARN
#define WARN(condition, format...) ({ \
int __ret_warn_on = !!(condition); \
+ no_printk(format); \
unlikely(__ret_warn_on); \
})
#endif
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/5] bug: Make BUG() always stop the machine
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
2014-03-10 1:01 ` [PATCH v3 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
2014-03-10 1:02 ` [PATCH v3 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
@ 2014-03-10 1:02 ` Josh Triplett
2014-03-10 1:03 ` [PATCH v3 5/5] x86: Always define BUG() and HAVE_ARCH_BUG, even with !CONFIG_BUG Josh Triplett
2014-03-11 16:40 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Arnd Bergmann
4 siblings, 0 replies; 21+ messages in thread
From: Josh Triplett @ 2014-03-10 1:02 UTC (permalink / raw)
To: Andrew Morton; +Cc: Arnd Bergmann, linux-arch, linux-kernel
When !CONFIG_BUG and !HAVE_ARCH_BUG, define the generic BUG() as an
infinite loop rather than a no-op. This avoids undefined behavior if
execution ever actually reaches BUG(), and avoids warnings about code
after BUG() (such as on non-void functions calling BUG() and then
not returning).
bloat-o-meter results:
add/remove: 0/0 grow/shrink: 43/10 up/down: 235/-98 (137)
function old new delta
umount_collect 119 138 +19
notify_change 306 324 +18
xstate_enable_boot_cpu 252 269 +17
kunmap 54 70 +16
balloon_page_dequeue 112 126 +14
mm_take_all_locks 223 233 +10
list_lru_walk_node 143 152 +9
vma_adjust 1059 1067 +8
pcpu_setup_first_chunk 1130 1138 +8
mm_drop_all_locks 143 151 +8
ns_capable 55 62 +7
anon_transport_class_unregister 8 15 +7
srcu_init_notifier_head 35 41 +6
shrink_dcache_for_umount 174 180 +6
kunmap_high 99 105 +6
end_page_writeback 43 49 +6
do_exit 1339 1345 +6
__kfifo_dma_out_prepare_r 86 92 +6
__kfifo_dma_in_prepare_r 90 96 +6
fixup_user_fault 120 125 +5
repair_env_string 73 77 +4
read_cache_pages_invalidate_page 56 60 +4
isolate_lru_pages.isra 142 146 +4
do_notify_parent_cldstop 255 259 +4
cpu_init 370 374 +4
utimes_common 270 272 +2
tasklet_hi_action 91 93 +2
tasklet_action 91 93 +2
set_pte_vaddr 46 48 +2
find_get_pages_tag 202 204 +2
early_iounmap 185 187 +2
__native_set_fixmap 36 38 +2
__get_user_pages 822 824 +2
__early_ioremap 299 301 +2
yield_task_stop 1 2 +1
tick_resume 37 38 +1
switched_to_stop 1 2 +1
switched_to_idle 1 2 +1
prio_changed_stop 1 2 +1
prio_changed_idle 1 2 +1
pm_qos_power_read 111 112 +1
arch_cpu_idle_dead 1 2 +1
__insert_vmap_area 140 141 +1
sys_renameat 614 612 -2
mm_fault_error 297 295 -2
SyS_renameat 614 612 -2
sys_linkat 416 413 -3
SyS_linkat 416 413 -3
chmod_common 129 122 -7
proc_cap_handler 240 225 -15
__schedule 849 831 -18
sys_madvise 1077 1054 -23
SyS_madvise 1077 1054 -23
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v3: New patch in this series, incorporating Arnd's suggestion to make BUG()
always stop execution. This eliminates the new warnings currently appearing in
allnoconfig due to !CONFIG_BUG; unlike v2, this avoids using unreachable(), and
thus avoids the possibility of undefined behvior if execution actually reaches
a call to BUG().
include/asm-generic/bug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index a97fa11..630dd23 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -138,7 +138,7 @@ extern void warn_slowpath_null(const char *file, const int line);
#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
-#define BUG() do {} while (0)
+#define BUG() do {} while (1)
#endif
#ifndef HAVE_ARCH_BUG_ON
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 5/5] x86: Always define BUG() and HAVE_ARCH_BUG, even with !CONFIG_BUG
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
` (2 preceding siblings ...)
2014-03-10 1:02 ` [PATCH v3 4/5] bug: Make BUG() always stop the machine Josh Triplett
@ 2014-03-10 1:03 ` Josh Triplett
2014-03-11 16:40 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Arnd Bergmann
4 siblings, 0 replies; 21+ messages in thread
From: Josh Triplett @ 2014-03-10 1:03 UTC (permalink / raw)
To: Andrew Morton; +Cc: Arnd Bergmann, linux-arch, linux-kernel
This ensures that BUG() always has a definition that causes a trap (via
an undefined instruction), and that the compiler still recognizes the
code following BUG() as unreachable, avoiding warnings that would
otherwise appear (such as on non-void functions that don't return a
value after BUG()).
In addition to saving a few bytes over the generic infinite-loop
implementation, this implementation traps rather than looping, which
potentially allows for better error-recovery behavior (such as by
rebooting).
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v3: New patch for x86-optimized implementation of a simple bug for
!CONFIG_BUG, based on Arnd Bergmann's proposal.
arch/x86/include/asm/bug.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 2f03ff0..ba38ebb 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -1,7 +1,6 @@
#ifndef _ASM_X86_BUG_H
#define _ASM_X86_BUG_H
-#ifdef CONFIG_BUG
#define HAVE_ARCH_BUG
#ifdef CONFIG_DEBUG_BUGVERBOSE
@@ -33,8 +32,6 @@ do { \
} while (0)
#endif
-#endif /* !CONFIG_BUG */
-
#include <asm-generic/bug.h>
#endif /* _ASM_X86_BUG_H */
--
1.9.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
` (3 preceding siblings ...)
2014-03-10 1:03 ` [PATCH v3 5/5] x86: Always define BUG() and HAVE_ARCH_BUG, even with !CONFIG_BUG Josh Triplett
@ 2014-03-11 16:40 ` Arnd Bergmann
2014-03-11 17:49 ` Josh Triplett
4 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2014-03-11 16:40 UTC (permalink / raw)
To: Josh Triplett; +Cc: Andrew Morton, linux-arch, linux-kernel
On Monday 10 March 2014, Josh Triplett wrote:
>
> When !CONFIG_BUG, WARN_ON and family become simple passthroughs of their
> condition argument; however, WARN_ON_ONCE and family still have
> conditions and a boolean to detect one-time invocation, even though the
> warning they'd emit doesn't exist. Make the existing definitions
> conditional on CONFIG_BUG, and add definitions for !CONFIG_BUG that map
> to the passthrough versions of WARN and WARN_ON.
>
> This saves 4.4k on a minimized configuration (smaller than
> allnoconfig), and 20.6k with defconfig plus CONFIG_BUG=n.
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> v3: Patch unchanged from v2.
>
> Andrew, can you please replace the entire v2 series currently in -mm
> with this new series?
[all patches]
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family
2014-03-11 16:40 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Arnd Bergmann
@ 2014-03-11 17:49 ` Josh Triplett
0 siblings, 0 replies; 21+ messages in thread
From: Josh Triplett @ 2014-03-11 17:49 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Andrew Morton, linux-arch, linux-kernel
On Tue, Mar 11, 2014 at 05:40:25PM +0100, Arnd Bergmann wrote:
> On Monday 10 March 2014, Josh Triplett wrote:
> >
> > When !CONFIG_BUG, WARN_ON and family become simple passthroughs of their
> > condition argument; however, WARN_ON_ONCE and family still have
> > conditions and a boolean to detect one-time invocation, even though the
> > warning they'd emit doesn't exist. Make the existing definitions
> > conditional on CONFIG_BUG, and add definitions for !CONFIG_BUG that map
> > to the passthrough versions of WARN and WARN_ON.
> >
> > This saves 4.4k on a minimized configuration (smaller than
> > allnoconfig), and 20.6k with defconfig plus CONFIG_BUG=n.
> >
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > ---
> > v3: Patch unchanged from v2.
> >
> > Andrew, can you please replace the entire v2 series currently in -mm
> > with this new series?
>
>
> [all patches]
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Thanks!
- Josh Triplett
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2014-03-11 17:49 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-26 3:48 [PATCH v2 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
2014-02-26 3:48 ` [PATCH v2 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
2014-02-26 13:25 ` Arnd Bergmann
2014-02-26 3:49 ` [PATCH v2 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
2014-02-26 13:25 ` Arnd Bergmann
2014-02-26 3:49 ` [PATCH v2 4/5] bug: Use a common definition of BUG_ON regardless of CONFIG_BUG Josh Triplett
2014-02-26 13:26 ` Arnd Bergmann
2014-02-26 3:49 ` [PATCH v2 5/5] bug: Make BUG() call unreachable() Josh Triplett
2014-02-26 13:29 ` Arnd Bergmann
2014-02-26 14:58 ` Josh Triplett
2014-02-27 19:19 ` Arnd Bergmann
2014-02-28 0:16 ` Josh Triplett
2014-02-28 8:55 ` Arnd Bergmann
2014-03-10 1:00 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Josh Triplett
2014-03-10 1:01 ` [PATCH v3 2/5] include/asm-generic/bug.h: Style fix: s/while(0)/while (0)/ Josh Triplett
2014-03-10 1:02 ` [PATCH v3 3/5] bug: When !CONFIG_BUG, make WARN call no_printk to check format and args Josh Triplett
2014-03-10 1:02 ` [PATCH v3 4/5] bug: Make BUG() always stop the machine Josh Triplett
2014-03-10 1:03 ` [PATCH v3 5/5] x86: Always define BUG() and HAVE_ARCH_BUG, even with !CONFIG_BUG Josh Triplett
2014-03-11 16:40 ` [PATCH v3 1/5] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family Arnd Bergmann
2014-03-11 17:49 ` Josh Triplett
2014-02-26 13:24 ` [PATCH v2 " Arnd Bergmann
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).