lkml.org 
[lkml]   [2011]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 25/26] dynamic_debug: add pr_fmt_dbg() for dynamic_pr_debug
From
Date
On Wed, 2011-09-28 at 11:27 -0600, Jim Cromie wrote:
> On Tue, Sep 27, 2011 at 10:51 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2011-09-27 at 20:54 -0600, Jim Cromie wrote:
> >> Cleanest way is to drop the original synonym, and just use
> >> warn (its shorter), but that creates some churn (havent grepped to see
> >> how much).
> >> I picked what looked like least effort & fewest corner-cases.
> >> ICBW..
> >
> > #ifndef pr_fmt_warning
> > #define pr_fmt_warning pr_fmt_warn
> > #endif
> >
>
> you've made it conditional, where line 203 is not. why ?
> 201 #define pr_warning(fmt, ...) \
> 202 printk(KERN_WARNING pr_fmt_warn(fmt), ##__VA_ARGS__)
> 203 #define pr_warn pr_warning
> 204 #define pr_notice(fmt, ...) \
>
> Also, though minor, 203 adds pr_warn as the shortcut,
> youve suggested the opposite. I presume this is an oversight.

I think pr_warning should be deprecated.
I think not #defining pr_fmt_warning is just fine.

> >> > What did you think of avoiding all of this and
> >> > having __dynamic_pr_debug move the fmt pointer over
> >> > any initial KBUILD_MODULE ": "
> >> >
> >> > int __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
> >> > {
> >> > [...]
> >> > size_t modsize = strlen(descriptor->modname);
> >> > if (0 == strncmp(fmt, descriptor->modname, modsize) &&
> >> > 0 == strncmp(fmt + modsize, ": ", 2))
> >> > fmt += modsize + 2;
> >> > vprintk(fmt, ...)
> >> > ?
> >>
> >> I was getting to that... ;-)
> >> Im not crazy about it. It feels like too much ..
> >> Its a runtime workaround for what I think is a
> >> problem in users' (or header's) #defines.

> > I think exactly the opposite myself.
> >
> > I think all of the '#define pr_fmt(fmt) KBUILD_MODNAME ": "'
> > are effectivly useless and I will eventually delete them.
> >
> > The printk subsystem should look up the module name and
> > prefix them to the printk akin to how the dynamic_debug
> > system does. Except the module name should be a singleton.
>
> hmm, maybe Ive missed something in your argument.
>
> For non-dynamic-debug builds, it seems you still want
> the MODNAME in the pr_(crit|warn|info|debug) output,
> you just dont like the hundreds of defines throughout drivers/*

Yes. I've added most all of them.

It's because the current printk / pr_<level>
subsystem chose to not prefix by default.
Over the last couple of years, I've helped
slowly converted about half of the kernel to
pr_<level> and pr_fmt.

At some point next year or so, enough of the
kernel should be converted so that marking
the last few uses of pr_debug or other
pr_<level> without a preceding pr_fmt should
be be relatively easy to convert to a more
standard KBUILD_MODNAME or other user specified
prefix.

It takes awhile though.

> IOW, it seems to take care of everything, w/o runtime workarounds.

Nope. Try it with and without DEBUG.

Also, look at the pr_debug uses that include __func__.
These also duplicate output.

I'd prefer a solution that allows deduplication.
Having a dynamic_debug output to a fixed buffer
via vsnprintf, scanning that output for leading
KBUILD_MODNAME/__file__/__func__/__LINE__ prefixes
and then skipping them when enabled would seem
appropriate to me.

cheers, Joe



\
 
 \ /
  Last update: 2011-09-29 17:27    [W:0.037 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site