| Message ID | 1512358624-6309-4-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove assert() | expand |
On 3 December 2017 at 20:37, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > BUG() and BUG_ON() are generally used to test a condition that should > never happen. If it does, it is a bug. > > Linux always enables them, but doing so in U-Boot causes image size > problems on some platforms. Introduce CONFIG_ENABLE_BUG_CHECKS to > make them no-op by default. Platforms without image size constraint > are free to enable this option to catch bugs easily. > > Likewise, silence WARN_ON() unless this option is enabled. > > Suggested-by: Tom Rini <trini@konsulko.com> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Changes in v2: > - Newly added > > include/linux/bug.h | 9 ++++++++- > lib/Kconfig | 7 +++++++ > 2 files changed, 15 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org>
Hi, On Mon, 4 Dec 2017 12:37:02 +0900 Masahiro Yamada wrote: > BUG() and BUG_ON() are generally used to test a condition that should > never happen. If it does, it is a bug. > > Linux always enables them, but doing so in U-Boot causes image size > problems on some platforms. Introduce CONFIG_ENABLE_BUG_CHECKS to > make them no-op by default. Platforms without image size constraint > are free to enable this option to catch bugs easily. > > Likewise, silence WARN_ON() unless this option is enabled. > > Suggested-by: Tom Rini <trini@konsulko.com> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Changes in v2: > - Newly added > > include/linux/bug.h | 9 ++++++++- > lib/Kconfig | 7 +++++++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/include/linux/bug.h b/include/linux/bug.h > index f07bb71..ac1c7de 100644 > --- a/include/linux/bug.h > +++ b/include/linux/bug.h > @@ -6,17 +6,24 @@ > #include <linux/compiler.h> > #include <linux/printk.h> > > +#ifdef CONFIG_ENABLE_BUG_CHECKS > #define BUG() do { \ > printk("BUG at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ > panic("BUG!"); \ > } while (0) > +#define __WARN() \ > + printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__) > +#else > +#define BUG() > +#define __WARN() > +#endif > > #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0) > > #define WARN_ON(condition) ({ \ > int __ret_warn_on = !!(condition); \ > if (unlikely(__ret_warn_on)) \ > - printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ > + __WARN(); \ > unlikely(__ret_warn_on); \ > }) > > diff --git a/lib/Kconfig b/lib/Kconfig > index 00ac650..36b1b3b 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -45,6 +45,13 @@ config USE_TINY_PRINTF > > The supported format specifiers are %c, %s, %u/%d and %x. > > +config ENABLE_BUG_CHECKS > + bool "Enable BUG/BUG_ON() checks and WARN_ON() logs" > + help This should be 'default y' IMO to keep the current behaviour for all existing platforms. Lothar Waßmann
On Tue, Dec 12, 2017 at 10:06:19AM +0100, Lothar Waßmann wrote: > Hi, > > On Mon, 4 Dec 2017 12:37:02 +0900 Masahiro Yamada wrote: > > BUG() and BUG_ON() are generally used to test a condition that should > > never happen. If it does, it is a bug. > > > > Linux always enables them, but doing so in U-Boot causes image size > > problems on some platforms. Introduce CONFIG_ENABLE_BUG_CHECKS to > > make them no-op by default. Platforms without image size constraint > > are free to enable this option to catch bugs easily. > > > > Likewise, silence WARN_ON() unless this option is enabled. > > > > Suggested-by: Tom Rini <trini@konsulko.com> > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > --- > > > > Changes in v2: > > - Newly added > > > > include/linux/bug.h | 9 ++++++++- > > lib/Kconfig | 7 +++++++ > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/bug.h b/include/linux/bug.h > > index f07bb71..ac1c7de 100644 > > --- a/include/linux/bug.h > > +++ b/include/linux/bug.h > > @@ -6,17 +6,24 @@ > > #include <linux/compiler.h> > > #include <linux/printk.h> > > > > +#ifdef CONFIG_ENABLE_BUG_CHECKS > > #define BUG() do { \ > > printk("BUG at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ > > panic("BUG!"); \ > > } while (0) > > +#define __WARN() \ > > + printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__) > > +#else > > +#define BUG() > > +#define __WARN() > > +#endif > > > > #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0) > > > > #define WARN_ON(condition) ({ \ > > int __ret_warn_on = !!(condition); \ > > if (unlikely(__ret_warn_on)) \ > > - printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ > > + __WARN(); \ > > unlikely(__ret_warn_on); \ > > }) > > > > diff --git a/lib/Kconfig b/lib/Kconfig > > index 00ac650..36b1b3b 100644 > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -45,6 +45,13 @@ config USE_TINY_PRINTF > > > > The supported format specifiers are %c, %s, %u/%d and %x. > > > > +config ENABLE_BUG_CHECKS > > + bool "Enable BUG/BUG_ON() checks and WARN_ON() logs" > > + help > This should be 'default y' IMO to keep the current behaviour for all > existing platforms. I brought this up to Masahiro privately as I had been testing the series, and with ENABLE_BUG_CHECKS=n we get a warning over in the USB code, which in turn got me thinking harder. We do want to be able to disable this, for space reasons, when needed, but it should default to enabled (even if this increases the overall size). -- Tom
2017-12-12 22:47 GMT+09:00 Tom Rini <trini@konsulko.com>: > On Tue, Dec 12, 2017 at 10:06:19AM +0100, Lothar Waßmann wrote: >> Hi, >> >> On Mon, 4 Dec 2017 12:37:02 +0900 Masahiro Yamada wrote: >> > BUG() and BUG_ON() are generally used to test a condition that should >> > never happen. If it does, it is a bug. >> > >> > Linux always enables them, but doing so in U-Boot causes image size >> > problems on some platforms. Introduce CONFIG_ENABLE_BUG_CHECKS to >> > make them no-op by default. Platforms without image size constraint >> > are free to enable this option to catch bugs easily. >> > >> > Likewise, silence WARN_ON() unless this option is enabled. >> > >> > Suggested-by: Tom Rini <trini@konsulko.com> >> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> > --- >> > >> > Changes in v2: >> > - Newly added >> > >> > include/linux/bug.h | 9 ++++++++- >> > lib/Kconfig | 7 +++++++ >> > 2 files changed, 15 insertions(+), 1 deletion(-) >> > >> > diff --git a/include/linux/bug.h b/include/linux/bug.h >> > index f07bb71..ac1c7de 100644 >> > --- a/include/linux/bug.h >> > +++ b/include/linux/bug.h >> > @@ -6,17 +6,24 @@ >> > #include <linux/compiler.h> >> > #include <linux/printk.h> >> > >> > +#ifdef CONFIG_ENABLE_BUG_CHECKS >> > #define BUG() do { \ >> > printk("BUG at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ >> > panic("BUG!"); \ >> > } while (0) >> > +#define __WARN() \ >> > + printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__) >> > +#else >> > +#define BUG() >> > +#define __WARN() >> > +#endif >> > >> > #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0) >> > >> > #define WARN_ON(condition) ({ \ >> > int __ret_warn_on = !!(condition); \ >> > if (unlikely(__ret_warn_on)) \ >> > - printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ >> > + __WARN(); \ >> > unlikely(__ret_warn_on); \ >> > }) >> > >> > diff --git a/lib/Kconfig b/lib/Kconfig >> > index 00ac650..36b1b3b 100644 >> > --- a/lib/Kconfig >> > +++ b/lib/Kconfig >> > @@ -45,6 +45,13 @@ config USE_TINY_PRINTF >> > >> > The supported format specifiers are %c, %s, %u/%d and %x. >> > >> > +config ENABLE_BUG_CHECKS >> > + bool "Enable BUG/BUG_ON() checks and WARN_ON() logs" >> > + help >> This should be 'default y' IMO to keep the current behaviour for all >> existing platforms. > > I brought this up to Masahiro privately as I had been testing the > series, and with ENABLE_BUG_CHECKS=n we get a warning over in the USB > code, which in turn got me thinking harder. We do want to be able to > disable this, for space reasons, when needed, but it should default to > enabled (even if this increases the overall size). > > -- > Tom I will flip the default in v3. Tom privately mentioned this series would be postponed by the next MW. So, I will do v3 when he is ready to receive it.
diff --git a/include/linux/bug.h b/include/linux/bug.h index f07bb71..ac1c7de 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -6,17 +6,24 @@ #include <linux/compiler.h> #include <linux/printk.h> +#ifdef CONFIG_ENABLE_BUG_CHECKS #define BUG() do { \ printk("BUG at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ panic("BUG!"); \ } while (0) +#define __WARN() \ + printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__) +#else +#define BUG() +#define __WARN() +#endif #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0) #define WARN_ON(condition) ({ \ int __ret_warn_on = !!(condition); \ if (unlikely(__ret_warn_on)) \ - printk("WARNING at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ + __WARN(); \ unlikely(__ret_warn_on); \ }) diff --git a/lib/Kconfig b/lib/Kconfig index 00ac650..36b1b3b 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -45,6 +45,13 @@ config USE_TINY_PRINTF The supported format specifiers are %c, %s, %u/%d and %x. +config ENABLE_BUG_CHECKS + bool "Enable BUG/BUG_ON() checks and WARN_ON() logs" + help + BUG() and BUG_ON() are no-op by default. This option enables + them to print noisy messages, then reboot or halt the system. + It also enables WARN_ON() messages. + config PANIC_HANG bool "Do not reset the system on fatal error" help
BUG() and BUG_ON() are generally used to test a condition that should never happen. If it does, it is a bug. Linux always enables them, but doing so in U-Boot causes image size problems on some platforms. Introduce CONFIG_ENABLE_BUG_CHECKS to make them no-op by default. Platforms without image size constraint are free to enable this option to catch bugs easily. Likewise, silence WARN_ON() unless this option is enabled. Suggested-by: Tom Rini <trini@konsulko.com> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v2: - Newly added include/linux/bug.h | 9 ++++++++- lib/Kconfig | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-)