[PATCH] Adding KERN_INFO to some printks #2

[PATCH] Adding KERN_INFO to some printks #2

[vda]

Primary purpose of this patch is to make KERN_WARNING and
KERN_INFO log levels closer to their original meaning.
Today they are quite far from what was intended.
Just look what kernel writes at the WARNING level
each time you boot your box!

When I was making this patch I couldn't resist and fixed
messed up tabs around affected printks, wrapped some
lines longer than 80 columns, fixed some typos.
My formatting preferences:
* log entries are started with capital letters except for
function/modules names in lowercase or acronyms (IDE etc)
* Dot before \n is a waste of space
* colon style: "Foo: blah blan" (not "Foo : blah" or "Foo: Blah")
But I'm not a religious fanatic: it ok to disagree with me :-)
You can see in the patch that I wasn't overly distracted
by this decorative work.

I'm doing my best trying not to break working code.
However, if you feel paranoid today you may remove
any hunk of this patch you may deem suspicious
and apply the rest - all these changes are independent.

If you like this patch but have more interesting things to play with,
you may do the following:
* clear your logs
* reconfigure syslogd to spew warnings to /var/log/syslog.warnings
* reboot
* mail boot time "warnings" which you think are not warnings but
info only ("104-key keyboard detected"-type msgs) to me -
I'll add fixes for those to this patch.

Patch can be found at
http://port.imtp.ilyichevsk.odessa.ua/linux/vda/KERN_INFO-2.4.13.diff

or emailed on request (our www server isn't exactly powerful, you may
have difficulty downloading the patch)
--
vda

Re: [PATCH] Adding KERN_INFO to some printks #2

[Robert Love]

On Fri, 2001-11-09 at 08:47, vda wrote:
> Primary purpose of this patch is to make KERN_WARNING and
> KERN_INFO log levels closer to their original meaning.
> Today they are quite far from what was intended.
> Just look what kernel writes at the WARNING level
> each time you boot your box!

This is an _excellent_ patch and you should proffer it to Linus and Alan
when you are done.  I would recommend diffing off 2.4.14 instead of
2.4.13, to this end.

I haven't gone over the actual loglevel warnings, but I plan to.  A
quick glimpse shows you are changing what needs to be changed.  Good
job.

> Patch can be found at
> http://port.imtp.ilyichevsk.odessa.ua/linux/vda/KERN_INFO-2.4.13.diff
> 
> or emailed on request (our www server isn't exactly powerful, you may
> have difficulty downloading the patch)

Yah it was slow but it worked :)

	Robert Love

Re: [PATCH] Adding KERN_INFO to some printks #2

[Alan Cox]

> Well... thanks man.
> I hope patch will be noticed by our tribal leaders :-)
> (Linus? Alan?)

Its not at the top of my list right now. I want to get the merge with Linus
done before attacking anything newer

Re: [PATCH] Adding KERN_INFO to some printks #2

[Robert Love]

On Fri, 2001-11-09 at 18:20, vda wrote:
> Well... thanks man.
> I hope patch will be noticed by our tribal leaders :-)
> (Linus? Alan?)

Alan is really busy stuffing patches off to Linus, and thus he is more
concerned with getting Linus's 2.4.15 up to sync with him right now. 
Linus is probably busy with that, too.  If you don't see this in a Linus
-pre, 2.5.0 is also right around the tree.

I think the most important thing you are doing is adding loglevel values
to printk statements that have none -- that is important not just to
clarify and make sure the value is right, but because the default
loglevel can and will change (it has before).

I went over the patch and found a few things...

printk(KERN_INFO "No local APIC present or hardware disabled\n");

 I'd make this a KERN_WARNING.  Consider the case where I compile my own
kernel and I add APIC support.  If the driver is failing to find my APIC
then either (a) my BIOS is broken or (b) I should remove the driver. 
Either way I would want to know.

printk (KERN_WARNING "mtrr: your CPUs had inconsistent fixed MTRR 
printk (KERN_WARNING "mtrr: your CPUs had inconsistent variable MTRR
printk (KERN_WARNING "mtrr: your CPUs had inconsistent MTRRdefType
printk (KERN_WARNING "mtrr: probably your BIOS does not setup all 

 These can actually be KERN_INFO, because it is not a problem and the
mtrr driver fixes the issue.

There are a _lot_ of printk statements in your patch where you didn't
add a loglevel.  You modified them for some reason (in many cases to
change printk("%s" ...) to printk(pf: ...).  You can easily find them
via a search on `printk("' ... that same search can be a grep to find
on-specified printks in the whole tree, too :)

Good work.

	Robert Love

Re: [PATCH] Adding KERN_INFO to some printks #2

[Robert Love]

On Fri, 2001-11-09 at 18:31, vda wrote:
> What amazes me to no end is how Alan can be so active.
> Alan, you are something. Really. :-)

Didn't you hear?  I think Linus broke the news awhile back: Alan has the
uncanny ability to fork() himself infinitely many times.  And he has no
resource contention, so he scales O(1).

	Robert Love

Re: [PATCH] Adding KERN_INFO to some printks #2

[George Greer]

On 9 Nov 2001, Robert Love wrote:

>On Fri, 2001-11-09 at 18:31, vda wrote:
>> What amazes me to no end is how Alan can be so active.
>> Alan, you are something. Really. :-)
>
>Didn't you hear?  I think Linus broke the news awhile back: Alan has the
>uncanny ability to fork() himself infinitely many times.  And he has no
>resource contention, so he scales O(1).

That was the "thousand gnomes" message:

http://www.uwsg.iu.edu/hypermail/linux/kernel/0005.0/0274.html

Suspiciously, there was no denial. :)

-George Greer

Re: [PATCH] Adding KERN_INFO to some printks #2

[vda]

On Friday 09 November 2001 15:56, Robert Love wrote:
> On Fri, 2001-11-09 at 08:47, vda wrote:
> > Primary purpose of this patch is to make KERN_WARNING and...
>
> This is an _excellent_ patch and you should proffer it to Linus and Alan
> when you are done.  I would recommend diffing off 2.4.14 instead of
> 2.4.13, to this end.
>
> I haven't gone over the actual loglevel warnings, but I plan to.  A
> quick glimpse shows you are changing what needs to be changed.  Good
> job.

Well... thanks man.
I hope patch will be noticed by our tribal leaders :-)
(Linus? Alan?)

Rediffed patch against 2.4.15-pre1 can be found at 
http://port.imtp.ilyichevsk.odessa.ua/linux/vda/KERN_INFO-2.4.15-pre1.diff

or emailed on request (our www server isn't exactly powerful, you may
have difficulty downloading the patch)
--------
Primary purpose of this patch is to make KERN_WARNING and
KERN_INFO log levels closer to their original meaning.
Today they are quite far from what was intended.
Just look what kernel writes at the WARNING level
each time you boot your box!

When I was making this patch I couldn't resist and fixed
messed up tabs around affected printks, wrapped some
lines longer than 80 columns, fixed some typos.
My formatting preferences:
* log entries are started with capital letters except for
function/modules names in lowercase or acronyms (IDE etc)
* Dot before \n is a waste of space
* colon style: "Foo: blah blan" (not "Foo : blah" or "Foo: Blah")
But I'm not a religious fanatic: it ok to disagree with me :-)
You can see in the patch that I wasn't overly distracted
by this decorative work.

I'm doing my best trying not to break working code.
However, if you feel paranoid today you may remove
any hunk of this patch you may deem suspicious
and apply the rest - all these changes are independent
of each other, you may even just ignore rejects
if you are patching newer/older kernel!

If you like this patch but have more interesting things to play with,
you may do the following:
* clear your logs
* reconfigure syslogd to spew warnings to /var/log/syslog.warnings
* reboot
* mail boot time "warnings" which you think are not warnings but
info only ("104-key keyboard detected"-type msgs) to me -
I'll add fixes for those msgs to this patch
--
vda

Re: [PATCH] Adding KERN_INFO to some printks #2

[vda]

On Friday 09 November 2001 21:32, Alan Cox wrote:
> > Well... thanks man.
> > I hope patch will be noticed by our tribal leaders :-)
> > (Linus? Alan?)
>
> Its not at the top of my list right now. I want to get the merge with Linus
> done before attacking anything newer

What amazes me to no end is how Alan can be so active.
Alan, you are something. Really. :-)

I will keep rediffing patches against newer kernels and announcing diffs.
Take them when you are ready. 
--
vda

Re: [PATCH] Adding KERN_INFO to some printks #2

[vda]

On Friday 09 November 2001 21:45, Robert Love wrote:
> I went over the patch and found a few things...
>
> printk(KERN_INFO "No local APIC present or hardware disabled\n");
>
>  I'd make this a KERN_WARNING.  Consider the case where I compile my own
> kernel and I add APIC support.  If the driver is failing to find my APIC
> then either (a) my BIOS is broken or (b) I should remove the driver.
> Either way I would want to know.

Ok I'll do

> printk (KERN_WARNING "mtrr: your CPUs had inconsistent fixed MTRR
> printk (KERN_WARNING "mtrr: your CPUs had inconsistent variable MTRR
> printk (KERN_WARNING "mtrr: your CPUs had inconsistent MTRRdefType
> printk (KERN_WARNING "mtrr: probably your BIOS does not setup all
>
>  These can actually be KERN_INFO, because it is not a problem and the
> mtrr driver fixes the issue.

I'd rather not overdo my patch. Better leave some KERN_WARNINGs where they 
are now than hide something important. I am concentrated on killing
_obviously_ informative msgs.

> There are a _lot_ of printk statements in your patch where you didn't
> add a loglevel.  You modified them for some reason (in many cases to
> change printk("%s" ...) to printk(pf: ...).  You can easily find them
> via a search on `printk("' ... that same search can be a grep to find
> on-specified printks in the whole tree, too :)

I modified printks which were hard to find due to lack of
any greppable [ :-) ] string. Next poor soul will be more lucky :-)

I don't think adding log levels massively is good: I'd like to see
real world bogus warning log files and fix only those ('don't overdo it' 
policy)
--
vda

Re: [PATCH] Adding KERN_INFO to some printks #2

[John Levon]

On Fri, Nov 09, 2001 at 04:45:48PM -0500, Robert Love wrote:

> I went over the patch and found a few things...
> 
> printk(KERN_INFO "No local APIC present or hardware disabled\n");
> 
>  I'd make this a KERN_WARNING.  Consider the case where I compile my own
> kernel and I add APIC support.  If the driver is failing to find my APIC
> then either (a) my BIOS is broken or (b) I should remove the driver. 
> Either way I would want to know.

This isn't what's happening - check apic.c. It is re-enabled if possible,
or a local APIC really doesn't exist. Either way I don't see the point
in a warning.

regards
john

-- 
"I know I believe in nothing but it is my nothing"
	- Manic Street Preachers