Re: [PATCH -next] acpi utmisc: use WARN_ON() instead of warn_on_slowpath()

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 02 Jul 2008 22:11:06 +0200
Andi Kleen <andi@firstfloor.org> wrote:

> Here's a patch. Can you use that one instead of Randy's please?
> 

No.

> 
> [bug-fallback  text/plain (593B)]
> Supply warn_on_slow_path() even for the !CONFIG_BUG case
> 
> Fix build problem with ACPI for !CONFIG_BUG. Noted by Randy Dunlap.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 2632328..d0d83b7 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -63,6 +63,9 @@ extern void warn_on_slowpath(const char *file, const int line);
>  	unlikely(__ret_warn_on);					\
>  })
>  #endif
> +
> +static inline void warn_on_slowpath(const char *file, const int line) {}
> +
>  #endif

There's no reason why asm/bug.h has to even include asm-generic/bug.h. 
And there's no reason why an architecture which defined __WARN needs to
define warn_on_slowpath() even if it _does_ include asm-generic/bug.h. 
And I didn't even begin to look at what might disable
WANT_WARN_ON_SLOWPATH.

This is all poking deep into the private internals of one particular
implementation of this interface.

Furthermore even if it _does_ happen to work, you've gone and coupled
the availability of this acpi debbugging feature to CONFIG_BUG, which
seems arbitrary.

If you really want to do it this way (and it sounds reasonable) then
can we please do it in a less-than-totally-hacky-and-broken way?



For example, define a new, always-available helper function in (say)
kernel/panic.c along the lines of

void emit_warning_message(?)(const char *msg, int line)


and then call that from warn_on_slowpath().  Will require that
warn_on_slowpath become inlined so we don't get a useless extra entry
in the backtraces all the time.

Or just use printk and dump_stack.