[PATCH] Fix generic WARN_ON message

[PATCH] Fix generic WARN_ON message

[Jeremy Fitzhardinge]

A warning is a warning, not a BUG.

Signed-off-by: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Andrew Morton <akpm@osdl.org>
Cc: Ingo Molnar <mingo@elte.hu>

diff -r 6c118d4e4240 include/asm-generic/bug.h
--- a/include/asm-generic/bug.h	Tue Oct 17 12:43:22 2006 -0700
+++ b/include/asm-generic/bug.h	Tue Oct 17 18:56:59 2006 -0700
@@ -35,7 +35,7 @@ struct bug_entry {
 #define WARN_ON(condition) ({						\
 	typeof(condition) __ret_warn_on = (condition);			\
 	if (unlikely(__ret_warn_on)) {					\
-		printk("BUG: warning at %s:%d/%s()\n", __FILE__,	\
+		printk("WARNING at %s:%d %s()\n", __FILE__,	\
 			__LINE__, __FUNCTION__);			\
 		dump_stack();						\
 	}								\

Re: [PATCH] Fix generic WARN_ON message

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> A warning is a warning, not a BUG.

> -		printk("BUG: warning at %s:%d/%s()\n", __FILE__,	\
> +		printk("WARNING at %s:%d %s()\n", __FILE__,	\

i'm not really happy about this change.

Firstly, most WARN_ON()s are /bugs/, not warnings ... If it's a real 
warning, a KERN_INFO printk should be done.

Secondly, the reason i changed it to the 'BUG: ...' format is that i 
tried to make it easier for automated tools (and for users) to figure 
out that a kernel bug happened.

	Ingo

Re: [PATCH] Fix generic WARN_ON message

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> Firstly, most WARN_ON()s are /bugs/, not warnings ... If it's a real 
> warning, a KERN_INFO printk should be done.
>   

It seems to me that either the warnings are really bugs, in which case 
they should be using BUG/BUG_ON, or they're not really bugs, in which 
case they should be presented differently.

> Secondly, the reason i changed it to the 'BUG: ...' format is that i 
> tried to make it easier for automated tools (and for users) to figure 
> out that a kernel bug happened.
>   

Well, are they bugs or not?  I think people are more confused by the 
"BUG" prefix and stacktrace than helped by it (even an experienced eye 
will glance-parse a BUG+stack trace as a serious oops-level problem 
rather than a warning).

    J

    J

Re: [PATCH] Fix generic WARN_ON message

[Nick Piggin]

Jeremy Fitzhardinge wrote:
> Ingo Molnar wrote:
> 
>> Firstly, most WARN_ON()s are /bugs/, not warnings ... If it's a real 
>> warning, a KERN_INFO printk should be done.
>>   
> 
> 
> It seems to me that either the warnings are really bugs, in which case 
> they should be using BUG/BUG_ON, or they're not really bugs, in which 
> case they should be presented differently.

No. A BUG() will terminate the current process which, aside from the
loss of userspace data, can tangle up the kernel badly and deadlock
or panic it.

If a bug can be fixed up or otherwise will not result in unstable
behaviour with continued operation, then it should be a WARN.

> 
>> Secondly, the reason i changed it to the 'BUG: ...' format is that i 
>> tried to make it easier for automated tools (and for users) to figure 
>> out that a kernel bug happened.
>>   
> 
> 
> Well, are they bugs or not?  I think people are more confused by the 
> "BUG" prefix and stacktrace than helped by it (even an experienced eye 
> will glance-parse a BUG+stack trace as a serious oops-level problem 
> rather than a warning).

Definitely a bug. If the condition is not a bug then the code calling
WARN is, so it is a bug no matter how you look at it ;)

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [PATCH] Fix generic WARN_ON message

[Pavel Machek]

Hi!

> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> 
> > A warning is a warning, not a BUG.
> 
> > -		printk("BUG: warning at %s:%d/%s()\n", __FILE__,	\
> > +		printk("WARNING at %s:%d %s()\n", __FILE__,	\
> 
> i'm not really happy about this change.
> 
> Firstly, most WARN_ON()s are /bugs/, not warnings ... If it's a real 
> warning, a KERN_INFO printk should be done.
> 
> Secondly, the reason i changed it to the 'BUG: ...' format is that i 
> tried to make it easier for automated tools (and for users) to figure 
> out that a kernel bug happened.

Well... but the message is really bad. It leads to users telling us "I
hit BUG in kernel"...
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Fix generic WARN_ON message

[Steven Rostedt]

On Wed, 2006-10-25 at 12:04 +0200, Pavel Machek wrote:
> Hi!
> 
> > * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> > 
> > > A warning is a warning, not a BUG.
> > 
> > > -		printk("BUG: warning at %s:%d/%s()\n", __FILE__,	\
> > > +		printk("WARNING at %s:%d %s()\n", __FILE__,	\
> > 
> > i'm not really happy about this change.
> > 
> > Firstly, most WARN_ON()s are /bugs/, not warnings ... If it's a real 
> > warning, a KERN_INFO printk should be done.
> > 
> > Secondly, the reason i changed it to the 'BUG: ...' format is that i 
> > tried to make it easier for automated tools (and for users) to figure 
> > out that a kernel bug happened.
> 
> Well... but the message is really bad. It leads to users telling us "I
> hit BUG in kernel"...

But they *did* hit a BUG. It just so happens that the BUG was fixable.
We want this reported because a WARN_ON should *never* be hit unless
there's a bug.  If people start getting "WARNING" messages, they will
more likely not be reporting them.

As Ingo already said, if it is just a "warning" then a normal printk
should be used.

-- Steve

Re: [PATCH] Fix generic WARN_ON message

[Pavel Machek]

On Wed 2006-10-25 16:55:22, Steven Rostedt wrote:
> On Wed, 2006-10-25 at 12:04 +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> > > 
> > > > A warning is a warning, not a BUG.
> > > 
> > > > -		printk("BUG: warning at %s:%d/%s()\n", __FILE__,	\
> > > > +		printk("WARNING at %s:%d %s()\n", __FILE__,	\
> > > 
> > > i'm not really happy about this change.
> > > 
> > > Firstly, most WARN_ON()s are /bugs/, not warnings ... If it's a real 
> > > warning, a KERN_INFO printk should be done.
> > > 
> > > Secondly, the reason i changed it to the 'BUG: ...' format is that i 
> > > tried to make it easier for automated tools (and for users) to figure 
> > > out that a kernel bug happened.
> > 
> > Well... but the message is really bad. It leads to users telling us "I
> > hit BUG in kernel"...
> 
> But they *did* hit a BUG. It just so happens that the BUG was fixable.
> We want this reported because a WARN_ON should *never* be hit unless
> there's a bug.  If people start getting "WARNING" messages, they will
> more likely not be reporting them.
> 
> As Ingo already said, if it is just a "warning" then a normal printk
> should be used.

Fine, then why is the macro called WARN_ON()? That's certainly highly
confusing.

NONFATAL_BUG_ON()?

I hate people reporting BUG (or BUG()) when they hit WARN_ON(), and
current wording certainly makes it easy.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html