[patch] add loglevel to printk in fs/afs/cmservice.c

[patch] add loglevel to printk in fs/afs/cmservice.c

[Jesper Juhl]

Hi, 

below is a small patch that adds loglevel to a printk in 
fs/afs/cmservice.c

If considered OK please consider applying.

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

diff -up linux-2.6.10-bk2-orig/fs/afs/cmservice.c linux-2.6.10-bk2/fs/afs/cmservice.c
--- linux-2.6.10-bk2-orig/fs/afs/cmservice.c	2004-12-24 22:34:44.000000000 +0100
+++ linux-2.6.10-bk2/fs/afs/cmservice.c	2004-12-31 02:59:13.000000000 +0100
@@ -118,7 +118,7 @@ static int kafscmd(void *arg)
 	_SRXAFSCM_xxxx_t func;
 	int die;
 
-	printk("kAFS: Started kafscmd %d\n", current->pid);
+	printk(KERN_INFO "kAFS: Started kafscmd %d\n", current->pid);
 
 	daemonize("kafscmd");
 

printk loglevel policy?

[Coywolf Qi Hunt]

Hi all, 

Recently, I've seen a lot of add loglevel to printk patches. 
grep 'printk("' -r | wc shows me 2433. There are probably 2433 printk
need to patch, is it?  What's this printk loglevel policy, all these
printk calls need loglevel adjusted?  The default loglevel is
KERN_WARNING.


--coywolf



On Fri, 31 Dec 2004 03:02:57 +0100 (CET), Jesper Juhl <juhl-lkml@dif.dk> wrote:
> 
> Hi,
> 
> below is a small patch that adds loglevel to a printk in
> fs/afs/cmservice.c
> 
> If considered OK please consider applying.
> 
> Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
> 
> diff -up linux-2.6.10-bk2-orig/fs/afs/cmservice.c linux-2.6.10-bk2/fs/afs/cmservice.c
> --- linux-2.6.10-bk2-orig/fs/afs/cmservice.c    2004-12-24 22:34:44.000000000 +0100
> +++ linux-2.6.10-bk2/fs/afs/cmservice.c 2004-12-31 02:59:13.000000000 +0100
> @@ -118,7 +118,7 @@ static int kafscmd(void *arg)
>         _SRXAFSCM_xxxx_t func;
>         int die;
> 
> -       printk("kAFS: Started kafscmd %d\n", current->pid);
> +       printk(KERN_INFO "kAFS: Started kafscmd %d\n", current->pid);
> 
>         daemonize("kafscmd");
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


-- 
Coywolf Qi Hunt
Homepage http://sosdg.org/~coywolf/

Re: printk loglevel policy?

[Jim Nelson]

Coywolf Qi Hunt wrote:
> Hi all, 
> 
> Recently, I've seen a lot of add loglevel to printk patches. 
> grep 'printk("' -r | wc shows me 2433. There are probably 2433 printk
> need to patch, is it?  What's this printk loglevel policy, all these
> printk calls need loglevel adjusted?  The default loglevel is
> KERN_WARNING.
> 
> 
> --coywolf

Not every printk() needs a loglevel.  For example:

	printk (KERN_WARN "blah...");

	some stuff...

	printk ("bleh...");

	more stuff...

	printk ("done\n");

is used in a lot of areas.

You'll also see:
#ifdef VERBOSE_DEBUG
#define FOO_DEBUG(a, b) printk ("%s: %s\n", a, b)
#else
#define FOO_DEBUG(a, b)
#endif

which is normally only used for debug builds.

The logging levels are, for the most part, common sense.  KERN_ERR for error 
conditions, KERN_INFO for informational (i. e. "driver just loaded", "new disk 
detected"), KERN_CRIT if your computer just caught on fire (!), and KERN_DEBUG for 
any kind of verbose printing.

Re: printk loglevel policy?

[Jesper Juhl]

On Thu, 30 Dec 2004, Jim Nelson wrote:

> Coywolf Qi Hunt wrote:
> > Hi all, 
> > Recently, I've seen a lot of add loglevel to printk patches. grep 'printk("'
> > -r | wc shows me 2433. There are probably 2433 printk
> > need to patch, is it?  What's this printk loglevel policy, all these
> > printk calls need loglevel adjusted?  The default loglevel is
> > KERN_WARNING.
> > 
[...]
> 
> The logging levels are, for the most part, common sense.  KERN_ERR for error
> conditions, KERN_INFO for informational (i. e. "driver just loaded", "new disk
> detected"), KERN_CRIT if your computer just caught on fire (!), and KERN_DEBUG
> for any kind of verbose printing.
> 
And the patches I've posted just try to set the loglevel to something 
sensible where what is sensible seems to be obvious. If I'm right or wrong 
I'll leave to the maintainers to decide.


-- 
Jesper Juhl

Re: printk loglevel policy?

[Alan Cox]

On Gwe, 2004-12-31 at 02:20, Coywolf Qi Hunt wrote:
> Hi all, 
> 
> Recently, I've seen a lot of add loglevel to printk patches. 
> grep 'printk("' -r | wc shows me 2433. There are probably 2433 printk
> need to patch, is it?  What's this printk loglevel policy, all these

You would need to work out which were at the start of a newline - most
of them are probably just fine and valid

Re: printk loglevel policy?

[Jim Nelson]

Alan Cox wrote:
> On Gwe, 2004-12-31 at 02:20, Coywolf Qi Hunt wrote:
> 
>>Hi all, 
>>
>>Recently, I've seen a lot of add loglevel to printk patches. 
>>grep 'printk("' -r | wc shows me 2433. There are probably 2433 printk
>>need to patch, is it?  What's this printk loglevel policy, all these
> 
> 
> You would need to work out which were at the start of a newline - most
> of them are probably just fine and valid
> 

That reminds me of a question I've had inthe back of my head.  When you have a SMP 
system wouldn't it be possible to have:

CPU 1 (running func1)	CPU 2 (running func2)
  |			 |
  printk ("foo...");	 |
  |			printk ("bleh\n");
  printk ("finished\n);	 |
			printk ("readout from bleh\n";

Is that possible?  Especially if the process on CPU 1 slept on a semaphore or 
something similar?

Or does printk() do some tracking that I didn't see as to where in the kernel the 
strings are coming from?

Re: printk loglevel policy?

[Randy.Dunlap]

Jim Nelson wrote:
> Alan Cox wrote:
> 
>> On Gwe, 2004-12-31 at 02:20, Coywolf Qi Hunt wrote:
>>
>>> Hi all,
>>> Recently, I've seen a lot of add loglevel to printk patches. grep 
>>> 'printk("' -r | wc shows me 2433. There are probably 2433 printk
>>> need to patch, is it?  What's this printk loglevel policy, all these
>>
>>
>>
>> You would need to work out which were at the start of a newline - most
>> of them are probably just fine and valid
>>
> 
> That reminds me of a question I've had inthe back of my head.  When you 
> have a SMP system wouldn't it be possible to have:
> 
> CPU 1 (running func1)    CPU 2 (running func2)
>  |             |
>  printk ("foo...");     |
>  |            printk ("bleh\n");
>  printk ("finished\n);     |
>             printk ("readout from bleh\n";
> 
> Is that possible?  Especially if the process on CPU 1 slept on a 
> semaphore or something similar?
> 
> Or does printk() do some tracking that I didn't see as to where in the 
> kernel the strings are coming from?

That kind of garbled output has been known to happen, but
the <console_sem> is supposed to prevent that (along with
zap_locks() in kernel/printk.c).

If it still happens, it needs to be fixed.
David Howells (RH) has posted patches that fix it.

-- 
~Randy

Re: printk loglevel policy?

[Keith Owens]

On Sun, 02 Jan 2005 13:41:34 -0800, 
"Randy.Dunlap" <rddunlap@osdl.org> wrote:
>Jim Nelson wrote:
>> Or does printk() do some tracking that I didn't see as to where in the 
>> kernel the strings are coming from?
>
>That kind of garbled output has been known to happen, but
>the <console_sem> is supposed to prevent that (along with
>zap_locks() in kernel/printk.c).

Using multiple calls to printk to print a single line has always been
subject to the possibility of interleaving on SMP.  We just live with
the risk.  Printing a complete line in a single call to printk is
protected by various locks.  Print a line in multiple calls is not
protected.  If it bothers you that much, build up the line in a local
buffer then call printk once.

Re: printk loglevel policy?

[Randy.Dunlap]

Keith Owens wrote:
> On Sun, 02 Jan 2005 13:41:34 -0800, 
> "Randy.Dunlap" <rddunlap@osdl.org> wrote:
> 
>>Jim Nelson wrote:
>>
>>>Or does printk() do some tracking that I didn't see as to where in the 
>>>kernel the strings are coming from?
>>
>>That kind of garbled output has been known to happen, but
>>the <console_sem> is supposed to prevent that (along with
>>zap_locks() in kernel/printk.c).
> 
> 
> Using multiple calls to printk to print a single line has always been
> subject to the possibility of interleaving on SMP.  We just live with
> the risk.  Printing a complete line in a single call to printk is
> protected by various locks.  Print a line in multiple calls is not
> protected.  If it bothers you that much, build up the line in a local
> buffer then call printk once.

True, I was thinking about the single line case, which I
have seen garbled/mixed in the past (on SMP).  Hopefully
that one is fixed.

-- 
~Randy

Re: printk loglevel policy?

[Jim Nelson]

Randy.Dunlap wrote:
> Keith Owens wrote:
> 
>> On Sun, 02 Jan 2005 13:41:34 -0800, "Randy.Dunlap" <rddunlap@osdl.org> 
>> wrote:
>>
>>> Jim Nelson wrote:
>>>
>>>> Or does printk() do some tracking that I didn't see as to where in 
>>>> the kernel the strings are coming from?
>>>
>>>
>>> That kind of garbled output has been known to happen, but
>>> the <console_sem> is supposed to prevent that (along with
>>> zap_locks() in kernel/printk.c).
>>
>>
>>
>> Using multiple calls to printk to print a single line has always been
>> subject to the possibility of interleaving on SMP.  We just live with
>> the risk.  Printing a complete line in a single call to printk is
>> protected by various locks.  Print a line in multiple calls is not
>> protected.  If it bothers you that much, build up the line in a local
>> buffer then call printk once.
> 
> 
> True, I was thinking about the single line case, which I
> have seen garbled/mixed in the past (on SMP).  Hopefully
> that one is fixed.
> 

Okay, that answered my question.  Is is frowned upon to use strncat/strcat in the 
kernel?  I have yet to see any use of them outside of the definition in 
lib/string.c.  It would seem to be faster (less potential contention for the 
printk locks).

Re: printk loglevel policy?

[David Howells]

Keith Owens <kaos@ocs.com.au> wrote:

> >That kind of garbled output has been known to happen, but
> >the <console_sem> is supposed to prevent that (along with
> >zap_locks() in kernel/printk.c).
> 
> Using multiple calls to printk to print a single line has always been
> subject to the possibility of interleaving on SMP.  We just live with the
> risk. Printing a complete line in a single call to printk is protected by
> various locks.  Print a line in multiple calls is not protected.  If it
> bothers you that much, build up the line in a local buffer then call printk
> once.

The oops writer breaks the locks. It's _really_ annoying when oopses happen
simultaneously on separate CPUs - the oops reports end up interleaved
char-by-char.

My patch serialised oops writing.

David