Am 06.05.2014 09:35, schrieb Ingo Molnar:
>
> * Richard Weinberger <richard.weinber...@gmail.com> wrote:
>
>> On Wed, Apr 30, 2014 at 5:03 PM, Paul Gortmaker
>> <paul.gortma...@windriver.com> wrote:
>>> A long standing problem for us has been the misuse of BUG/BUG_ON.
>>> The typical misuse is someone only thinking of what represents
>>> a bug in their local code, and especially for people relatively
>>> new to Linux, starting out in device drivers, the appeal of using
>>> BUG w/o knowing what it really does is too great.
>>>
>>> So you end up with some trivial non system critical driver bringing
>>> the whole system to a grinding halt just because it detected an
>>> internal inconsistency. That just makes users unhappy and looks bad.
>>>
>>> It is hopeless to think we can reclaim BUG/BUG_ON for their original
>>> intent, given there are currently ~20k instances. To make progress
>>> here, we create BUG_AND_HALT variants, which leave no doubt as to
>>> what they do in name alone.
>>>
>>> Then we can incrementally move the real BUG users (unrecoverable
>>> filesystem corruption, page table mangling, etc) onto BUG_AND_HALT,
>>> and finally at some time in the future we'll simply make the old
>>> BUG/BUG_ON be aliases for WARN/WARN_ON, once we've moved over the
>>> bulk of the instances really needing to halt the system.
>>>
>>> Suggested-by: Ingo Molnar <mi...@kernel.org>
>>> Signed-off-by: Paul Gortmaker <paul.gortma...@windriver.com>
>>> ---
>>>
>>> [This might not be a unique idea; but I'm pretty sure I'd first
>>> heard of it during a discussion with Ingo at RT summit last year.]
>>>
>>> include/asm-generic/bug.h | 16 ++++++++++++++++
>>> scripts/checkpatch.pl | 7 +++++++
>>> 2 files changed, 23 insertions(+)
>>>
>>> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
>>> index 630dd2372238..57b79a394ceb 100644
>>> --- a/include/asm-generic/bug.h
>>> +++ b/include/asm-generic/bug.h
>>> @@ -43,6 +43,14 @@ struct bug_entry {
>>> * 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.
>>> + *
>>> + * Sadly nobody listens to the above, and trying to reclaim BUG/BUG_ON
>>> + * for their original intent is about as hopeful as wishing "selfie"
>>> + * wasn't headed for the OED. So the plan is to avoid BUG/BUG_ON
>>> + * entirely. Either use WARN/WARN_ON or BUG_AND_HALT/BUG_AND_HALT_ON.
>>> + * Once the critical (e.g. fs etc) BUG/BUG_ON users are updated to use
>>> + * the clearly named HALT variants, we can point the old BUG/BUG_ON
>>> + * defines below to be clones of the less drastic WARN variants.
>>> */
>>> #ifndef HAVE_ARCH_BUG
>>> #define BUG() do { \
>>> @@ -51,10 +59,18 @@ struct bug_entry {
>>> } while (0)
>>> #endif
>>>
>>> +#ifndef HAVE_ARCH_BUG_AND_HALT
>>> +#define BUG_AND_HALT BUG
>>> +#endif
>>> +
>>> #ifndef HAVE_ARCH_BUG_ON
>>> #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
>>> #endif
>>>
>>> +#ifndef HAVE_ARCH_BUG_AND_HALT_ON
>>> +#define BUG_AND_HALT_ON BUG_ON
>>> +#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
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 34eb2160489d..3cbf3591cf76 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -2010,6 +2010,13 @@ sub process {
>>> $rpt_cleaners = 1;
>>> }
>>>
>>> +# Dont use BUG/BUG_ON; use WARN/WARN_ON or BUG_AND_HALT/BUG_AND_HALT_ON
>>> + if ($rawline =~ /^\+.*BUG\(/ || $rawline =~
>>> /^\+.*BUG_ON\(/) {
>>> + my $herevet = "$here\n" . cat_vet($rawline) . "\n";
>>> + WARN("BUG/BUG_ON",
>>> + "Use of BUG/BUG_ON is deprecated. Use
>>> WARN/WARN_ON or BUG_AND_HALT/BUG_AND_HALT_ON\n" . $herevet);
>>> + }
>>> +
>>> # Check for FSF mailing addresses.
>>> if ($rawline =~ /\bwrite to the Free/i ||
>>> $rawline =~ /\b59\s+Temple\s+Pl/i ||
>>> --
>>
>> I like the idea but not the name.
>> What about DIE() and DIE_ON()?
>
> CRASH_ON() might be a suggestive name as well, as from the user's
> point of view we are crashing her system.
I fear such users will think "Why should I crash the kernel?". ;-)
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
