Re: [PATCH] printk.caller

Previous thread: atl2 network driver status by Greg KH on Thursday, October 2, 2008 - 4:02 pm. (7 messages)

Next thread: [PATCH] ftrace: move pc counter in irqstrace by Steven Rostedt on Thursday, October 2, 2008 - 4:23 pm. (2 messages)
From: Roland McGrath
Date: Thursday, October 2, 2008 - 4:21 pm

This adds the printk.caller=[0|1] boot parameter, default setting
controlled by CONFIG_PRINTK_CALLER.  (This is modelled on printk.time
and CONFIG_PRINTK_TIME.)

When this is set, each printk line is automagically prefixed with
"{0x123abc} " giving the PC address of that printk call (actually
the PC address just after the call).

As a kernel hacker, I always hate having to grep for some fragment
of a message to find the code that generated it.  But I always have
my -g vmlinux handy, so:
	(gdb) info line *(0x123abc - 1)
is real handy (it pops the source up in an Emacs buffer).

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 Documentation/kernel-parameters.txt |    3 ++
 include/linux/kernel.h              |    4 ++-
 kernel/printk.c                     |   38 +++++++++++++++++++++++++++++-----
 lib/Kconfig.debug                   |    9 ++++++++
 4 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 1150444..9f1e1b9 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1689,6 +1689,9 @@ and is between 256 and 4096 characters. It is defined in the file
 	printk.time=	Show timing data prefixed to each printk message line
 			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
 
+	printk.caller=	Show caller PC prefixed to each printk message line
+			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
+
 	profile=	[KNL] Enable kernel profiling via /proc/profile
 			Format: [schedule,]<number>
 			Param: "schedule" - profile schedule points.
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2651f80..e03b475 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -191,7 +191,9 @@ struct pid;
 extern struct pid *session_of_pgrp(struct pid *pgrp);
 
 #ifdef CONFIG_PRINTK
-asmlinkage int vprintk(const char *fmt, va_list args)
+# define vprintk(fmt, args) \
+	vprintk_caller(fmt, args, ...
From: Andrew Morton
Date: Thursday, October 2, 2008 - 4:31 pm

On Thu,  2 Oct 2008 16:21:15 -0700 (PDT)


This is quite misleading.  The config help implies that
CONFIG_PRINTK_CALLER will enable the feature at compile time.  But it
doesn't - it just sets the boot-time enable/disable default.

If you do this:

#ifdef CONFIG_PRINTK_CALLER
static int printk_caller = 1;
#else
#define printk_caller 0
#endif

then the implementation would somewhat reflect the config option.


But I'd suggest that this thing is so small that it doesn't need a
config option to enable the presence of the code - just make it
unconditional.

Also, I guess that the boot-time option is useful, but a runtime /proc
knob might also be needed?

--

From: Roland McGrath
Date: Thursday, October 2, 2008 - 4:43 pm

I don't disagree with any of that.  
I followed the printk.time example closely.
All those things are equally true of that option.


Thanks,
Roland
--

From: Peter Zijlstra
Date: Friday, October 3, 2008 - 3:40 am

git grep is usually plenty fast for me, but I guess different people,
different tastes.

Also, I always use addr2line instead of gdb,.. another not-to-the-point
difference ;-)

The only real downside to this patch for me is that it potentially
increases the length of lines which means I;d have to stretch my serial
console window, but I guess others might object to the puny increase in
object size.

Flip a coin.


I thought we did sizeof() in-kernel.

--

Previous thread: atl2 network driver status by Greg KH on Thursday, October 2, 2008 - 4:02 pm. (7 messages)

Next thread: [PATCH] ftrace: move pc counter in irqstrace by Steven Rostedt on Thursday, October 2, 2008 - 4:23 pm. (2 messages)