On Tue, 6 May 2014 08:30:56 -0400
Dan Streetman <ddstr...@ieee.org> wrote:
> On Mon, May 5, 2014 at 4:35 PM, Andrew Morton <a...@linux-foundation.org>
> wrote:
> > On Mon, 5 May 2014 10:43:05 -0400 Dan Streetman <ddstr...@ieee.org> wrote:
> >
> >> Replace pr_debug() in lib/plist.c test function plist_test() with
> >> printk(KERN_DEBUG ...).
> >>
> >> Without DEBUG defined, pr_debug() is complied out, but the entire
> >> plist_test() function is already inside CONFIG_DEBUG_PI_LIST, so
> >> printk should just be used directly.
> >>
> >> --- a/lib/plist.c
> >> +++ b/lib/plist.c
> >> @@ -175,7 +175,7 @@ static int __init plist_test(void)
> >> int nr_expect = 0, i, loop;
> >> unsigned int r = local_clock();
> >>
> >> - pr_debug("start plist test\n");
> >> + printk(KERN_DEBUG "start plist test\n");
> >
> > Now someone will come along and helpfully switch it back to pr_debug()
> > again :(
> >
> > What about adding a #define DEBUG?
> >
> >
> >
> > This aspect of pr_debug() is rather surprising and unfortunate and I
> > guess we screwed it up. pr_debug() should unconditionally do the
> > printk, just like pr_warn, pr_emerg, etc. And there should be a
> > separate pr_debug_cond() which honours the DEBUG setting.
>
> I agree, it's definitely surprising and not obvious. At the least,
> maybe some clearer comments/docs would help; besides actually
> reviewing the printk.h code, the only other indication of this
> behavior is CodingStyle which currently says:
>
> "For messages that aren't associated with a particular device,
> <linux/printk.h> defines pr_debug() and pr_info()."
>
> Listing pr_debug() and pr_info() on the same line with no
> qualifications kind of implies they behave the same. Maybe the
> example should be pr_err() and pr_info(), or really anything besides
> pr_debug(), which is discussed in (very brief) detail in the next
> paragraph...
>
> "Such messages should be compiled out when the DEBUG symbol is not
> defined (that is, by default they are not included). When you use
> dev_dbg() or pr_debug(), that's automatic. Many subsystems have
> Kconfig options to turn on -DDEBUG."
>
> While that does explain that pr_debug() won't actually print anything
> without DEBUG defined, it's hardly in a way that jumps out, clearly
> indicating that pr_debug() is unlike all the other pr_XXX() functions.
>
> I'll try sending a patch to update the docs to make pr_debug()'s
> behavior clearer...
Admitting checkpatch is the authority in that matter and that per subsystem
debug granularity would be kept, we could at least add some specification
like below ?
[PATCH 1/1] scripts/checkpatch.pl: add printk(KERN_DEBUG to pr_debug
specification
Such conversions can't be done as trivially as other printk occurences.
Signed-off-by: Fabian Frederick <f...@skynet.be>
---
scripts/checkpatch.pl | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 34eb216..4e462d7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2863,9 +2863,14 @@ sub process {
my $level = lc($orig);
$level = "warn" if ($level eq "warning");
my $level2 = $level;
- $level2 = "dbg" if ($level eq "debug");
+ my $note = "";
+ if ($level eq "debug"){
+ $level2 = "dbg";
+ $note = "Note that printk(KERN_DEBUG
conversions to pr_debug require local DEBUG definition.\n";
+ }
WARN("PREFER_PR_LEVEL",
- "Prefer [subsystem eg:
netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then
pr_$level(... to printk(KERN_$orig ...\n" . $herecurr);
+ "Prefer [subsystem eg:
netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then
pr_$level(... to printk(KERN_$orig ...\n$note" . $herecurr);
+
}
if ($line =~ /\bpr_warning\s*\(/) {
--
1.8.4.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
