From: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> To: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Cc: Steve French <sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>, linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH] cifs: Rename cERROR and cifserror to cifs_vfs_err Date: Wed, 13 Mar 2013 05:40:21 -0700 [thread overview] Message-ID: <1363178421.2031.29.camel@joe-AO722> (raw) In-Reply-To: <20130313075155.7464a34a-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org> On Wed, 2013-03-13 at 07:51 -0400, Jeff Layton wrote: > On Wed, 13 Mar 2013 04:36:54 -0700 > Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> 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 --- fs/cifs/cifs_debug.h | 70 +++++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 47 deletions(-) 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 */
WARNING: multiple messages have this Message-ID (diff)
From: Joe Perches <joe@perches.com> To: Jeff Layton <jlayton@redhat.com> Cc: Steve French <sfrench@samba.org>, linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] cifs: Rename cERROR and cifserror to cifs_vfs_err Date: Wed, 13 Mar 2013 05:40:21 -0700 [thread overview] Message-ID: <1363178421.2031.29.camel@joe-AO722> (raw) In-Reply-To: <20130313075155.7464a34a@tlielax.poochiereds.net> On Wed, 2013-03-13 at 07:51 -0400, Jeff Layton wrote: > On Wed, 13 Mar 2013 04:36:54 -0700 > Joe Perches <joe@perches.com> 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 --- fs/cifs/cifs_debug.h | 70 +++++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 47 deletions(-) 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 */
next prev parent reply other threads:[~2013-03-13 12:40 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-03-12 22:44 [PATCH] cifs: Rename cERROR and cifserror to cifs_vfs_err Joe Perches 2013-03-13 11:36 ` Joe Perches 2013-03-13 11:36 ` Joe Perches 2013-03-13 11:51 ` Jeff Layton 2013-03-13 11:51 ` Jeff Layton [not found] ` <20130313075155.7464a34a-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org> 2013-03-13 12:40 ` Joe Perches [this message] 2013-03-13 12:40 ` Joe Perches 2013-03-14 19:24 ` [PATCH] cifs: Rename cERROR and cFYI to cifs_dbg Joe Perches 2013-03-15 18:56 ` Jeff Layton 2013-03-15 19:01 ` Jeff Layton 2013-03-15 19:01 ` Jeff Layton [not found] ` <CAH2r5mvW29FV+stH5CsjY5S2FdaHn48TfiSEk_beC=NzHC7-MQ@mail.gmail.com> [not found] ` <CAH2r5mvW29FV+stH5CsjY5S2FdaHn48TfiSEk_beC=NzHC7-MQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-03-15 20:43 ` Joe Perches 2013-03-15 20:43 ` Joe Perches 2013-03-25 4:44 ` Steve French 2013-03-25 4:44 ` Steve French 2013-03-15 21:46 ` Fwd: " Steve French 2013-03-15 21:46 ` Steve French
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1363178421.2031.29.camel@joe-AO722 \ --to=joe-6d6dil74uinbdgjk7y7tuq@public.gmane.org \ --cc=jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \ --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.