From patchwork Wed Mar 13 12:40:21 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Perches X-Patchwork-Id: 2263301 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 015ACDF215 for ; Wed, 13 Mar 2013 12:40:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755986Ab3CMMkX (ORCPT ); Wed, 13 Mar 2013 08:40:23 -0400 Received: from perches-mx.perches.com ([206.117.179.246]:49990 "EHLO labridge.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755526Ab3CMMkW (ORCPT ); Wed, 13 Mar 2013 08:40:22 -0400 Received: from [173.51.221.202] (account joe@perches.com HELO [192.168.1.152]) by labridge.com (CommuniGate Pro SMTP 5.0.14) with ESMTPA id 20789566; Wed, 13 Mar 2013 05:40:22 -0700 Message-ID: <1363178421.2031.29.camel@joe-AO722> Subject: Re: [PATCH] cifs: Rename cERROR and cifserror to cifs_vfs_err From: Joe Perches To: Jeff Layton Cc: Steve French , linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org Date: Wed, 13 Mar 2013 05:40:21 -0700 In-Reply-To: <20130313075155.7464a34a@tlielax.poochiereds.net> References: <1363128287.23052.21.camel@joe-AO722> <1363174614.2031.9.camel@joe-AO722> <20130313075155.7464a34a@tlielax.poochiereds.net> X-Mailer: Evolution 3.6.2-0ubuntu0.1 Mime-Version: 1.0 Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org On Wed, 2013-03-13 at 07:51 -0400, Jeff Layton wrote: > On Wed, 13 Mar 2013 04:36:54 -0700 > Joe Perches wrote: > > Perhaps a better idea than this patch is to > > change both the cERROR and cFYI macros to > > a new use of cifs_dbg(type, fmt, ...) > > > > cERROR(set, fmt, ...) -> cifs_dbg(VFS, fmt, ...) > > cFYI(set, fmt, ...) -> cifs_dbg(FYI, fmt, ...) > > > > This conversion would mark both these macros > > as debug stataments as they are only enabled > > with CONFIG_CIFS_DEBUG. [] > > This would also enable an easier conversion to > > dynamic debugging of these debug macros. > > > > I'd prefer to move the newline from the macro > > to the format as that is more consistent with > > the rest of the kernel. [] > I like this change overall, but the size of the patch is pretty > daunting. I'd characterize it as more mechanically dull than daunting, but it is awfully large (~300 KB). > If you could change the code that underlies cERROR() and > cFYI() without needing to touch all of their call sites, it might be > a simpler initial step. I think that would not help. > OTOH, I would also prefer to move the newline into the format and > that's impossible without touching most of these call sites. Well, all but the ones that already have a defective newline. I think there are 4 or 5. The .ko size increases a few hundred bytes when the newlines are moved to the format, though it's still about a 1% overall size reduction. There would be: 264 uses of cifs_dbg(VFS, ...) 622 uses of cifs_dbg(FYI, ...) 15 uses of cifs_dbg(NOISY, ...) to verify. Using strings on the .ko simplifies that. $ size fs/cifs/cifs.ko* text data bss dec hex filename 265891 2525 132 268548 41904 fs/cifs/cifs.ko.new 268359 2525 132 271016 422a8 fs/cifs/cifs.ko.old The core of it is to cifs_debug.h Acked-by: Jeff Layton --- fs/cifs/cifs_debug.h | 70 +++++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 47 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h index 69ae3d3..c99b40f 100644 --- a/fs/cifs/cifs_debug.h +++ b/fs/cifs/cifs_debug.h @@ -25,18 +25,20 @@ void cifs_dump_mem(char *label, void *data, int length); void cifs_dump_detail(void *); void cifs_dump_mids(struct TCP_Server_Info *); -#ifdef CONFIG_CIFS_DEBUG2 -#define DBG2 2 -#else -#define DBG2 0 -#endif extern int traceSMB; /* flag which enables the function below */ void dump_smb(void *, int); #define CIFS_INFO 0x01 #define CIFS_RC 0x02 #define CIFS_TIMER 0x04 +#define VFS 1 +#define FYI 2 extern int cifsFYI; +#ifdef CONFIG_CIFS_DEBUG2 +#define NOISY 4 +#else +#define NOISY 0 +#endif /* * debug ON @@ -44,31 +46,21 @@ extern int cifsFYI; */ #ifdef CONFIG_CIFS_DEBUG -/* information message: e.g., configuration, major event */ -#define cifsfyi(fmt, ...) \ -do { \ - if (cifsFYI & CIFS_INFO) \ - printk(KERN_DEBUG "%s: " fmt "\n", \ - __FILE__, ##__VA_ARGS__); \ -} while (0) - -#define cFYI(set, fmt, ...) \ -do { \ - if (set) \ - cifsfyi(fmt, ##__VA_ARGS__); \ -} while (0) +__printf(1, 2) void cifs_vfs_err(const char *fmt, ...); -#define cifswarn(fmt, ...) \ - printk(KERN_WARNING fmt "\n", ##__VA_ARGS__) - -/* error event message: e.g., i/o error */ -#define cifserror(fmt, ...) \ - printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__); \ - -#define cERROR(set, fmt, ...) \ +/* information message: e.g., configuration, major event */ +#define cifs_dbg(type, fmt, ...) \ do { \ - if (set) \ - cifserror(fmt, ##__VA_ARGS__); \ + if (type == FYI) { \ + if (cifsFYI & CIFS_INFO) { \ + printk(KERN_DEBUG "%s: " fmt, \ + __FILE__, ##__VA_ARGS__); \ + } \ + } else if (type == VFS) { \ + cifs_vfs_err(fmt, ##__VA_ARGS__); \ + } else if (type == NOISY && type != 0) { \ + printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ + } \ } while (0) /* @@ -76,27 +68,11 @@ do { \ * --------- */ #else /* _CIFS_DEBUG */ -#define cifsfyi(fmt, ...) \ +#define cifs_dbg(type, fmt, ...) \ do { \ if (0) \ - printk(KERN_DEBUG "%s: " fmt "\n", \ - __FILE__, ##__VA_ARGS__); \ + printk(KERN_DEBUG fmt, ##__VA_ARGS__); \ } while (0) -#define cFYI(set, fmt, ...) \ -do { \ - if (0 && set) \ - cifsfyi(fmt, ##__VA_ARGS__); \ -} while (0) -#define cifserror(fmt, ...) \ -do { \ - if (0) \ - printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__); \ -} while (0) -#define cERROR(set, fmt, ...) \ -do { \ - if (0 && set) \ - cifserror(fmt, ##__VA_ARGS__); \ -} while (0) -#endif /* _CIFS_DEBUG */ +#endif #endif /* _H_CIFS_DEBUG */