[patch 2.6.28-rc6] when to BUG(), and when to not BUG()

[patch 2.6.28-rc6] when to BUG(), and when to not BUG()

[David Brownell]

From: David Brownell <dbrownell@users.sourceforge.net>

Provide some basic advice about when to use BUG()/BUG_ON():
never, unless there's really no better option.

This matches my understanding of the standard policy ... which
seems not to be written down so far, outside of LKML messages
that I haven't bookmarked.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
---
This arguably belongs in patch checklists too... but I figure we
should start by flushing out any contrary opinions.

 include/asm-generic/bug.h |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -20,6 +20,17 @@ struct bug_entry {
 #define BUGFLAG_WARNING	(1<<0)
 #endif	/* CONFIG_GENERIC_BUG */
 
+/*
+ * Don't use BUG() or BUG_ON() unless there's really no way out; one
+ * example might be detecting data structure corruption in the middle
+ * of an operation that can't be backed out of.  If the (sub)system
+ * can somehow continue operating, perhaps with reduced functionality,
+ * it's probably not BUG-worthy.
+ *
+ * If you're tempted to BUG(), think again:  is completely giving up
+ * really the *only* solution?  There are usually better options, where
+ * users don't need to reboot ASAP and can mostly shut down cleanly.
+ */
 #ifndef HAVE_ARCH_BUG
 #define BUG() do { \
 	printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
@@ -31,6 +42,12 @@ struct bug_entry {
 #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
+ * appear at runtime.  Use the versions with printk format strings
+ * to provide better diagnostics.
+ */
 #ifndef __WARN
 #ifndef __ASSEMBLY__
 extern void warn_on_slowpath(const char *file, const int line);

Re: [patch 2.6.28-rc6] when to BUG(), and when to not BUG()

[Andrew Morton]

On Sat, 29 Nov 2008 19:21:30 -0800 David Brownell <david-b@pacbell.net> wrote:

> From: David Brownell <dbrownell@users.sourceforge.net>
> 
> Provide some basic advice about when to use BUG()/BUG_ON():
> never, unless there's really no better option.
> 
> This matches my understanding of the standard policy ... which
> seems not to be written down so far, outside of LKML messages
> that I haven't bookmarked.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> ---
> This arguably belongs in patch checklists too... but I figure we
> should start by flushing out any contrary opinions.
> 
>  include/asm-generic/bug.h |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -20,6 +20,17 @@ struct bug_entry {
>  #define BUGFLAG_WARNING	(1<<0)
>  #endif	/* CONFIG_GENERIC_BUG */
>  
> +/*
> + * Don't use BUG() or BUG_ON() unless there's really no way out; one
> + * example might be detecting data structure corruption in the middle
> + * of an operation that can't be backed out of.  If the (sub)system
> + * can somehow continue operating, perhaps with reduced functionality,
> + * it's probably not BUG-worthy.
> + *
> + * If you're tempted to BUG(), think again:  is completely giving up
> + * really the *only* solution?  There are usually better options, where
> + * users don't need to reboot ASAP and can mostly shut down cleanly.
> + */
>  #ifndef HAVE_ARCH_BUG
>  #define BUG() do { \
>  	printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
> @@ -31,6 +42,12 @@ struct bug_entry {
>  #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
> + * appear at runtime.  Use the versions with printk format strings
> + * to provide better diagnostics.
> + */
>  #ifndef __WARN
>  #ifndef __ASSEMBLY__
>  extern void warn_on_slowpath(const char *file, const int line);

I don't disagree.

One reason for trying to keep going rather than going BUG is that it
increases the chances that the information about the bug gets into the
logs, gets sent across netconsole, etc.