From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932216Ab1I2PYt (ORCPT ); Thu, 29 Sep 2011 11:24:49 -0400 Received: from perches-mx.perches.com ([206.117.179.246]:44576 "EHLO labridge.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753285Ab1I2PYs (ORCPT ); Thu, 29 Sep 2011 11:24:48 -0400 Message-ID: <1317309888.1854.18.camel@Joe-Laptop> Subject: Re: [PATCH 25/26] dynamic_debug: add pr_fmt_dbg() for dynamic_pr_debug From: Joe Perches To: Jim Cromie Cc: jbaron@redhat.com, bart.vanassche@gmail.com, greg@kroah.com, linux-kernel@vger.kernel.org Date: Thu, 29 Sep 2011 08:24:48 -0700 In-Reply-To: References: <1316642115-20029-1-git-send-email-jim.cromie@gmail.com> <1316642115-20029-26-git-send-email-jim.cromie@gmail.com> <1316725066.29447.16.camel@Joe-Laptop> <1317166589.19340.14.camel@Joe-Laptop> <1317185489.19340.22.camel@Joe-Laptop> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.1.92- Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2011-09-28 at 11:27 -0600, Jim Cromie wrote: > On Tue, Sep 27, 2011 at 10:51 PM, Joe Perches 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_ 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_ 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_ 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