Andrew Morton wrote:
> On Wed, 02 Jul 2008 22:11:06 +0200
> Andi Kleen <andi@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
>>
>> 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.
I checked them all and they all do.
Besides for the ACPI case we only have to care about x86 and ia64.
But all do it anyways.
> 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.
I think warn_on_slow_path() should be a general interface.
Perhaps with a better name though.
>
> This is all poking deep into the private internals of one particular
> implementation of this interface.
But it's in kernel/panic.c. I don't think anyone can compile
without that. So it should be always there (unless you undefine CONFIG_BUG)
> 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.
Well not defining CONFIG_BUG means "I don't care about debugging" doesn't it?
Not having (or having only crippled) ACPI debugging in that case seems reasonable
Doesn't seem that arbitrary to me.
>
> 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)
Ok we can just rename warn_on_slow_path() which is already there to that new name.
Would that satisfy you?
>
> 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.
I think the reason for the warn_on_slowpath() is that it produces
patterns that can be detected by Arjan's kerneloops scripts.
I'm not sure manual printk/dump_stack would have the same effect.
-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
