Re: [PM-SR][PATCH 01/12] omap3: voltage: cleanup pr_xxxx

[Nishanth Menon <nm@ti.com>]

Mark Brown had written, on 08/06/2010 07:18 AM, the following:
> On Fri, Aug 06, 2010 at 06:08:03AM -0500, Nishanth Menon wrote:
> 
>> Well that is my motivation here. if driver reports a warning to kernel  
>> syslog, well, I expect the log to contain the function name for me to  
>> maintain faster.
> 
> That's really not the expectation for Linux log messages - the general
> approach to find the source of a message is generally to just grep for
> the message text which is usually very effective.

taking a small sample set of pr_xyz(); (pr which spans a single line):

$ git grep pr_ drivers/|grep ")"|grep __func__|wc -l
589
$ git grep pr_ drivers/|grep ")"|grep -v __func__|wc -l
5373
$ git grep pr_fmt drivers/|wc -l
138

Reading Documentation/dynamic-debug-howto.txt, I see that you are in a 
one way right. I can get fine grained control over the log by enabling 
CONFIG_DYNAMIC_DEBUG.

At the same time, I have wondered on the usage of pr_fmt() in many of 
the drivers. in a way, if i wanted to be that verbose in a driver, I 
could in theory do:
#define pr_fmt(fmt) "%s:" fmt, __func__ and get the benefit throughout 
the file..

but overall, I still disagree that we dont need to have the function 
name in log is a speed booster for maintenance folks.
a) when the strings are split up when they go multiple lines:
E.g.:
	"abcd "
	"efg"
print comes out abcd efg
i) you do a git grep "abcd efg" will return nada
ii) if you do a git grep of "abcd", you will probably see half a dozen 
prints there, then open each file to see where the "real" message is, 
and you find the file searching a bit


b) when there are couple more usage in cases of commonly used error 
message- (e.g. invalid parameters),  you end up getting multiple hits, 
and you are left guessing where it came from

in this example: lets see: (on l-o pm branch):
git grep "DEBUG option not enabled" .
arch/arm/mach-omap2/smartreflex.c:              pr_notice("DEBUG option 
not enabled!\n  \
arch/arm/mach-omap2/voltage.c:          pr_notice("DEBUG option not 
enabled!\n  \

now open up both the files to find exactly what you are looking for.

c) time required:
$ time git grep "DEBUG option not enabled" .
arch/arm/mach-omap2/smartreflex.c:              pr_notice("DEBUG option 
not enabled!\n  \
arch/arm/mach-omap2/voltage.c:          pr_notice("DEBUG option not 
enabled!\n  \

real	1m34.722s
user	0m0.440s
sys	0m1.820s

Vs cscope or ctags where it is rather instantaneous if you know the 
function name..
-- 
Regards,
Nishanth Menon