Messages in this thread | | | Subject | Re: [PATCH 25/26] dynamic_debug: add pr_fmt_dbg() for dynamic_pr_debug | From | Joe Perches <> | Date | Thu, 29 Sep 2011 08:24:48 -0700 |
| |
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
| |