cifs: neatening and embedded module size reduction
Subsystems (2)
cover cifs: neatening and embedded module size reduction
Joe Perches (3):
cifs: Remove unused cEVENT macro
cifs: Make CIFS_DEBUG possible to undefine
cifs: Enable Kconfig control of CIFS_DEBUG
fs/cifs/Kconfig | 10 ++++++- fs/cifs/cifs_debug.h | 74 +++++++++++++++++++++++++++++-------------------- 2 files changed, 53 insertions(+), 31 deletions(-)
Signature · 1 line
1.7.8.112.g3fd21
PATCH 1/3 cifs: Remove unused cEVENT macro
It uses an undefined KERN_EVENT and is itself unused.
--- fs/cifs/cifs_debug.h | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
fs/cifs/cifs_debug.h +0 -7
@@ -63,12 +63,6 @@ do { \/* debug event message: */ extern int cifsERROR; -#define cEVENT(fmt, arg...) \ -do { \ - if (cifsERROR) \ - printk(KERN_EVENT "%s: " fmt "\n", __FILE__, ##arg); \ -} while (0) -/* error event message: e.g., i/o error */ #define cifserror(fmt, arg...) \ do { \
@@ -88,7 +82,6 @@ do { \*/ #else /* _CIFS_DEBUG */ #define cERROR(set, fmt, arg...) -#define cEVENT(fmt, arg...) #define cFYI(set, fmt, arg...) #define cifserror(fmt, arg...)#endif /* _CIFS_DEBUG */
Signature · 1 line
1.7.8.112.g3fd21
Re: [PATCH 1/3] cifs: Remove unused cEVENT macro
Quoted 25 lines · On Thu, 29 Nov 2012 11:37:18 -0800 Joe Perches…
It uses an undefined
KERN_EVENTand is itself unused.Signed-off-by Joe Perches <[email protected]>--- fs/cifs/cifs_debug.h | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)fs/cifs/cifs_debug.h +0
-6@@ -63,12 +63,6 @@ do { \/* debug event message: */ extern int cifsERROR; -#define cEVENT(fmt, arg...) \ -do { \ - if (cifsERROR) \ - printk(KERN_EVENT "%s: " fmt "\n", __FILE__, ##arg); \ -} while (0) -/* error event message: e.g., i/o error */ #define cifserror(fmt, arg...) \ do { \
fs/cifs/cifs_debug.h do { \ · line 87
Quoted 7 lines
fs/cifs/cifs_debug.h +0
-1@@ -88,7 +82,6 @@ do { \*/ #else /* _CIFS_DEBUG */ #define cERROR(set, fmt, arg...) -#define cEVENT(fmt, arg...) #define cFYI(set, fmt, arg...) #define cifserror(fmt, arg...)#endif /* _CIFS_DEBUG */
Yay! More cruft removal!
Re: [PATCH 1/3] cifs: Remove unused cEVENT macro
I don't mind removing cEVENT (it is currently unused, and presumably the older file systems that used EVENT logging, have moved to dtrace or other tracing mechanisms).
But ... the need still remains for more tracing for cifs (I run into this a few times a month at least when debugging) - does anyone have any ideas for how to add dynamic trace points to cifs (has anyone even done it for the vfs?) - is there any file system that does a good job defining dynamic trace points?
Quoted 39 lines · Jeff Layton
Quoted 25 lines · On Thu, 29 Nov 2012 11:37:18 -0800 Joe Perches…
It uses an undefined
KERN_EVENTand is itself unused.Signed-off-by Joe Perches <[email protected]>--- fs/cifs/cifs_debug.h | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)fs/cifs/cifs_debug.h +0
-6@@ -63,12 +63,6 @@ do { \/* debug event message: */ extern int cifsERROR; -#define cEVENT(fmt, arg...) \ -do { \ - if (cifsERROR) \ - printk(KERN_EVENT "%s: " fmt "\n", __FILE__, ##arg); \ -} while (0) -/* error event message: e.g., i/o error */ #define cifserror(fmt, arg...) \ do { \fs/cifs/cifs_debug.h do { \ · line 87
Quoted 7 lines
fs/cifs/cifs_debug.h +0
-1@@ -88,7 +82,6 @@ do { \*/ #else /* _CIFS_DEBUG */ #define cERROR(set, fmt, arg...) -#define cEVENT(fmt, arg...) #define cFYI(set, fmt, arg...) #define cifserror(fmt, arg...)#endif /* _CIFS_DEBUG */ Yay! More cruft removal!
Reviewed-by Jeff Layton <[email protected]>
Signature · 3 lines
Thanks, Steve
Re: [PATCH 1/3] cifs: Remove unused cEVENT macro
Quoted 5 lines · Steve French
But ... the need still remains for more tracing for cifs (I run into this a few times a month at least when debugging) - does anyone have any ideas for how to add dynamic trace points to cifs (has anyone even done it for the vfs?) - is there any file system that does a good job defining dynamic trace points?
I believe ext4 is the only filesystem with significant use of tracepoints.
Ted? Any tips you care to give as to best implementation methods?
Re: [PATCH 1/3] cifs: Remove unused cEVENT macro
In reply to Steve French(resending due to bounces, sorry for any inconvenience)
Quoted 5 lines · Steve French
But ... the need still remains for more tracing for cifs (I run into this a few times a month at least when debugging) - does anyone have any ideas for how to add dynamic trace points to cifs (has anyone even done it for the vfs?) - is there any file system that does a good job defining dynamic trace points?
I believe ext4 is the only filesystem with significant use of tracepoints.
Ted? Any tips you care to give as to best implementation methods?
PATCH 2/3 cifs: Make CIFS_DEBUG possible to undefine
In reply to Joe PerchesMake the compilation work again when CIFS_DEBUG is not #define'd.
Add format and argument verification for the various macros when CIFS_DEBUG is not #define'd.
--- fs/cifs/cifs_debug.h | 64 ++++++++++++++++++++++++++++++++------------------ 1 files changed, 41 insertions(+), 23 deletions(-)
fs/cifs/cifs_debug.h +41 -23
@@ -37,6 +37,9 @@ void dump_smb(void *, int);#define CIFS_RC 0x02#define CIFS_TIMER 0x04 +extern int cifsFYI; +extern int cifsERROR; +/* * debug ON * --------
@@ -44,36 +47,33 @@ void dump_smb(void *, int);#ifdef CIFS_DEBUG /* information message: e.g., configuration, major event */ -extern int cifsFYI; -#define cifsfyi(fmt, arg...) \ +#define cifsfyi (fmt, ...) \ do { \ if (cifsFYI & CIFS_INFO) \ - printk(KERN_DEBUG "%s: " fmt "\n", __FILE__, ##arg); \ + printk(KERN_DEBUG "%s: " fmt "\n", \ + __FILE__, ##__VA_ARGS__); \ } while (0) -#define cFYI(set, fmt, arg...) \ -do { \ - if (set) \ - cifsfyi(fmt, ##arg); \ +#define cFYI (set, fmt, ...) \ +do { \ + if (set) \ + cifsfyi(fmt, ##__VA_ARGS__); \ } while (0) -#define cifswarn(fmt, arg...) \ - printk(KERN_WARNING fmt "\n", ##arg) - -/* debug event message: */ -extern int cifsERROR; +#define cifswarn (fmt, ...) \ + printk(KERN_WARNING fmt "\n", ##__VA_ARGS__)/* error event message: e.g., i/o error */ -#define cifserror(fmt, arg...) \ -do { \ - if (cifsERROR) \ - printk(KERN_ERR "CIFS VFS: " fmt "\n", ##arg); \ +#define cifserror (fmt, ...) \ +do { \ + if (cifsERROR) \ + printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__); \ } while (0) -#define cERROR(set, fmt, arg...) \ -do { \ - if (set) \ - cifserror(fmt, ##arg); \ +#define cERROR (set, fmt, ...) \ +do { \ + if (set) \ + cifserror(fmt, ##__VA_ARGS__); \ } while (0)/*
@@ -81,9 +81,27 @@ do { \* --------- */ #else /* _CIFS_DEBUG */ -#define cERROR(set, fmt, arg...) -#define cFYI(set, fmt, arg...) -#define cifserror(fmt, arg...) +#define cifsfyi (fmt, ...) \ +do { \ + if (0) \ + printk(KERN_DEBUG "%s: " fmt "\n", \ + __FILE__, ##__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 /* _H_CIFS_DEBUG */
Signature · 1 line
1.7.8.112.g3fd21
Re: [PATCH 2/3] cifs: Make CIFS_DEBUG possible to undefine
Quoted 96 lines · On Thu, 29 Nov 2012 11:37:19 -0800 Joe Perches <[email protected]> wrote:
Make the compilation work again when
CIFS_DEBUGis not #define'd.Add format and argument verification for the various macros when
CIFS_DEBUGis not #define'd.Signed-off-by Joe Perches <[email protected]>--- fs/cifs/cifs_debug.h | 64 ++++++++++++++++++++++++++++++++------------------ 1 files changed, 41 insertions(+), 23 deletions(-)fs/cifs/cifs_debug.h +31
-23@@ -37,6 +37,9 @@ void dump_smb(void *, int);#define CIFS_RC 0x02#define CIFS_TIMER 0x04 +extern int cifsFYI; +extern int cifsERROR; +/* * debug ON * -------- @@ -44,36 +47,33 @@ void dump_smb(void *, int);#ifdef CIFS_DEBUG /* information message: e.g., configuration, major event */ -extern int cifsFYI; -#define cifsfyi(fmt, arg...) \ +#define cifsfyi (fmt, ...) \ do { \ if (cifsFYI & CIFS_INFO) \ - printk(KERN_DEBUG "%s: " fmt "\n", __FILE__, ##arg); \ + printk(KERN_DEBUG "%s: " fmt "\n", \ + __FILE__, ##__VA_ARGS__); \ } while (0) -#define cFYI(set, fmt, arg...) \ -do { \ - if (set) \ - cifsfyi(fmt, ##arg); \ +#define cFYI (set, fmt, ...) \ +do { \ + if (set) \ + cifsfyi(fmt, ##__VA_ARGS__); \ } while (0) -#define cifswarn(fmt, arg...) \ - printk(KERN_WARNING fmt "\n", ##arg) - -/* debug event message: */ -extern int cifsERROR; +#define cifswarn (fmt, ...) \ + printk(KERN_WARNING fmt "\n", ##__VA_ARGS__)/* error event message: e.g., i/o error */ -#define cifserror(fmt, arg...) \ -do { \ - if (cifsERROR) \ - printk(KERN_ERR "CIFS VFS: " fmt "\n", ##arg); \ +#define cifserror (fmt, ...) \ +do { \ + if (cifsERROR) \ + printk(KERN_ERR "CIFS VFS: " fmt "\n", ##__VA_ARGS__); \ } while (0) -#define cERROR(set, fmt, arg...) \ -do { \ - if (set) \ - cifserror(fmt, ##arg); \ +#define cERROR (set, fmt, ...) \ +do { \ + if (set) \ + cifserror(fmt, ##__VA_ARGS__); \ } while (0)/* @@ -81,9 +81,27 @@ do { \* --------- */ #else /* _CIFS_DEBUG */ -#define cERROR(set, fmt, arg...) -#define cFYI(set, fmt, arg...) -#define cifserror(fmt, arg...) +#define cifsfyi (fmt, ...) \ +do { \ + if (0) \ + printk(KERN_DEBUG "%s: " fmt "\n", \ + __FILE__, ##__VA_ARGS__); \ +} while (0) +#define cFYI (set, fmt, ...) \ +do { \ + if (0 && set) \ + cifsfyi(fmt, ##__VA_ARGS__); \ +} while (0)
fs/cifs/cifs_debug.h do { \ · line 104
Quoted 10 lines
fs/cifs/cifs_debug.h +10
-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)
Would it be better to simply make those the standard
"do { ; } while(0)" noop macros?
I'm not sure I see the point in keeping the printk statements in there...
Quoted 3 lines
fs/cifs/cifs_debug.h +0
-0#endif /* _CIFS_DEBUG */ #endif /* _H_CIFS_DEBUG */
Signature · 1 line
Jeff Layton <[email protected]>
Re: [PATCH 2/3] cifs: Make CIFS_DEBUG possible to undefine
Quoted 7 lines · Jeff Layton
Quoted 4 lines · On Thu, 29 Nov 2012 11:37:19 -0800 Joe Perches…
Make the compilation work again when
CIFS_DEBUGis not #define'd.Add format and argument verification for the various macros when
CIFS_DEBUGis not #define'd.
[]
Quoted 5 lines
Would it be better to simply make those the standard
"do { ; } while(0)" noop macros?
I'm not sure I see the point in keeping the printk statements in there...
Actually, that's not standard.
Look at printk.h
It prevents developers from adding code that compiles without warnings in one mode but compiles with warnings in another.
The idea is to make sure that format and arguments always match regardless of whether or not you are compiling debug or non-debug.
Jeff Layton
Reviewed-byRe: [PATCH 2/3] cifs: Make CIFS_DEBUG possible to undefine
Quoted 18 lines · On Fri, 30 Nov 2012 06:52:09 -0800 Joe Perches <[email protected]> wrote:
Quoted 7 lines · Jeff Layton
Quoted 4 lines · On Thu, 29 Nov 2012 11:37:19 -0800 Joe Perches <[email protected]> wrote:
Make the compilation work again when
CIFS_DEBUGis not #define'd.Add format and argument verification for the various macros when
CIFS_DEBUGis not #define'd.[]
Quoted 5 lines
Would it be better to simply make those the standard
"do { ; } while(0)" noop macros?
I'm not sure I see the point in keeping the printk statements in there...
Actually, that's not standard.
Look atprintk.h
Quoted 9 lines
It prevents developers from adding code that compiles without warnings in one mode but compiles with warnings in another.
The idea is to make sure that format and arguments always match regardless of whether or not you are compiling debug or non-debug.
Fair enough then...
PATCH 3/3 cifs: Enable Kconfig control of CIFS_DEBUG
In reply to Joe PerchesRather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
--- fs/cifs/Kconfig | 10 +++++++++- fs/cifs/cifs_debug.h | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-)
fs/cifs/Kconfig +9 -1
@@ -122,9 +122,17 @@ config CIFS_ACL Allows fetching CIFS/NTFS ACL from the server. The DACL blob is handed over to the application/caller. +config CIFS_DEBUG + bool "Enable CIFS debugging routines" + default y + depends on CIFS + help + Enabling this option adds helpful debugging messages to + the cifs code which increases the size of the cifs module. + If unsure, say Y. config CIFS_DEBUG2 bool "Enable additional CIFS debugging routines" - depends on CIFS + depends on CIFS_DEBUG help Enabling this option adds a few more debugging routines to the cifs code which slightly increases the size of
fs/cifs/cifs_debug.h +4 -1
@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif #ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUG
Signature · 1 line
1.7.8.112.g3fd21
Re: [PATCH 3/3] cifs: Enable Kconfig control of CIFS_DEBUG
Quoted 41 lines · On Thu, 29 Nov 2012 11:37:20 -0800 Joe Perches <[email protected]> wrote:
Rather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
Signed-off-by Joe Perches <[email protected]>--- fs/cifs/Kconfig | 10 +++++++++- fs/cifs/cifs_debug.h | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-)fs/cifs/Kconfig +9
-1@@ -122,9 +122,17 @@ config CIFS_ACL Allows fetching CIFS/NTFS ACL from the server. The DACL blob is handed over to the application/caller. +config CIFS_DEBUG + bool "Enable CIFS debugging routines" + default y + depends on CIFS + help + Enabling this option adds helpful debugging messages to + the cifs code which increases the size of the cifs module. + If unsure, say Y. config CIFS_DEBUG2 bool "Enable additional CIFS debugging routines" - depends on CIFS + depends on CIFS_DEBUG help Enabling this option adds a few more debugging routines to the cifs code which slightly increases the size offs/cifs/cifs_debug.h +0
-0@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
fs/cifs/cifs_debug.h line 27
Quoted 10 lines
fs/cifs/cifs_debug.h +4
-1* */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif #ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUG
Looks fine, but I'd probably prefer to just get rid of CIFS_DEBUG altogether here and replace it with CONFIG_CIFS_DEBUG.
While you're at it, /proc/fs/cifs/cifsFYI should probably not be present when CONFIG_CIFS_DEBUG isn't set. Perhaps we should also disable traceSMB and its related functions too in that case?
Signature · 1 line
Jeff Layton <[email protected]>
Re: [PATCH 3/3] cifs: Enable Kconfig control of CIFS_DEBUG
Quoted 8 lines · Jeff Layton
Quoted 5 lines · On Thu, 29 Nov 2012 11:37:20 -0800 Joe Perches…
Rather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
[]
Quoted 14 lines
Quoted 14 lines
fs/cifs/cifs_debug.h +4
-1@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif
Quoted 5 lines
fs/cifs/cifs_debug.h line 27
Quoted 2 lines
fs/cifs/cifs_debug.h +0
-0#ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUGLooks fine, but I'd probably prefer to just get rid of
CIFS_DEBUGaltogether here and replace it withCONFIG_CIFS_DEBUG.
I wasn't sure if there was other code anywhere else that depends on CIFS_DEBUG. There isn't any in the kernel source tree. If not, then you are right, it should be just CONFIG_CIFS_DEBUG.
Quoted 3 lines
While you're at it,
/proc/fs/cifs/cifsFYIshould probably not be present whenCONFIG_CIFS_DEBUGisn't set. Perhaps we should also disable traceSMB and its related functions too in that case?
That could be considered a kernel api change.
If people agree to delete it, I'd be OK with it.
Re: [PATCH 3/3] cifs: Enable Kconfig control of CIFS_DEBUG
Quoted 30 lines · On Fri, 30 Nov 2012 06:57:40 -0800 Joe Perches…
Quoted 8 lines · Jeff Layton
Quoted 5 lines · On Thu, 29 Nov 2012 11:37:20 -0800 Joe Perches…
Rather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
[]
Quoted 14 lines
Quoted 14 lines
fs/cifs/cifs_debug.h +4
-1@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif Quoted 5 lines
fs/cifs/cifs_debug.h line 27
Quoted 2 lines
fs/cifs/cifs_debug.h +0
-0#ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUGLooks fine, but I'd probably prefer to just get rid of
CIFS_DEBUGaltogether here and replace it withCONFIG_CIFS_DEBUG.
Quoted 4 lines
I wasn't sure if there was other code anywhere else that depends on
CIFS_DEBUG. There isn't any in the kernel source tree. If not, then you are right, it should be justCONFIG_CIFS_DEBUG.
There shouldn't be. None of this stuff is exposed to userspace, AFAIK. I think it's ok to just make that CONFIG_CIFS_DEBUG.
Quoted 7 lines
Quoted 3 lines
While you're at it,
/proc/fs/cifs/cifsFYIshould probably not be present whenCONFIG_CIFS_DEBUGisn't set. Perhaps we should also disable traceSMB and its related functions too in that case?That could be considered a kernel api change.
If people agree to delete it, I'd be OK with it.
If you compile with CONFIG_CIFS_DEBUG turned off, then the cifsFYI file will be a knob that's not hooked up to anything. I think it's best not to present it at all in that case.
traceSMB is a little harder to make the case for. Theoretically, it'll still work after your changes, but I'm not sure it makes sense to leave that knob around since it's technically a flag for debugging. You're probably fine not to worry about that one. I'll look at that at some point in the future.
Thanks,
Signature · 1 line
Jeff Layton <[email protected]>
PATCH v2 3/3 cifs: Add CONFIG_CIFS_DEBUG and rename use of CIFS_DEBUG
Quoted 41 lines · Jeff Layton
Quoted 29 lines · On Fri, 30 Nov 2012 06:57:40 -0800 Joe Perches <[email protected]> wrote:
Quoted 7 lines · Jeff Layton
Quoted 5 lines · On Thu, 29 Nov 2012 11:37:20 -0800 Joe Perches <[email protected]> wrote:
Rather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
[]
Quoted 14 lines
Quoted 14 lines
fs/cifs/cifs_debug.h +4
-1@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif Quoted 5 lines
fs/cifs/cifs_debug.h line 27
Quoted 2 lines
fs/cifs/cifs_debug.h +0
-0#ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUGLooks fine, but I'd probably prefer to just get rid of
CIFS_DEBUGaltogether here and replace it withCONFIG_CIFS_DEBUG.Quoted 3 lines
I wasn't sure if there was other code anywhere else that depends on
CIFS_DEBUG. There isn't any in the kernel source tree. If not, then you are right, it should be justCONFIG_CIFS_DEBUG.There shouldn't be. None of this stuff is exposed to userspace, AFAIK. I think it's ok to just make that
CONFIG_CIFS_DEBUG.Quoted 4 lines
Quoted 3 lines
While you're at it,
/proc/fs/cifs/cifsFYIshould probably not be present whenCONFIG_CIFS_DEBUGisn't set. Perhaps we should also disable traceSMB and its related functions too in that case?
Quoted 7 lines
Quoted 3 lines
That could be considered a kernel api change.
If people agree to delete it, I'd be OK with it.If you compile with
CONFIG_CIFS_DEBUGturned off, then the cifsFYI file will be a knob that's not hooked up to anything. I think it's best not to present it at all in that case.
If there's a userspace script that does
echo 1 > /proc/fs/cifs/cifsFYI
it'd now get a file not found error.
Quoted 5 lines
traceSMB is a little harder to make the case for. Theoretically, it'll still work after your changes, but I'm not sure it makes sense to leave that knob around since it's technically a flag for debugging. You're probably fine not to worry about that one. I'll look at that at some point in the future.
This could be changed in a separate patch if appropriate. Here's the modified patch.
From: Joe Perches <[email protected]>
This can reduce the size of the module by ~120KB which could be useful for embedded systems.
$ size fs/cifs/built-in.o* text data bss dec hex filename 388567 34459 100440 523466 7fcca fs/cifs/built-in.o.new 495970 34599 117904 648473 9e519 fs/cifs/built-in.o.old
--- fs/cifs/Kconfig | 10 +++++++++- fs/cifs/cifs_debug.h | 3 +-- 2 files changed, 10 insertions(+), 3 deletions(-)
fs/cifs/Kconfig +9 -1
@@ -122,9 +122,17 @@ config CIFS_ACL Allows fetching CIFS/NTFS ACL from the server. The DACL blob is handed over to the application/caller. +config CIFS_DEBUG + bool "Enable CIFS debugging routines" + default y + depends on CIFS + help + Enabling this option adds helpful debugging messages to + the cifs code which increases the size of the cifs module. + If unsure, say Y. config CIFS_DEBUG2 bool "Enable additional CIFS debugging routines" - depends on CIFS + depends on CIFS_DEBUG help Enabling this option adds a few more debugging routines to the cifs code which slightly increases the size of
fs/cifs/cifs_debug.h +1 -2
@@ -18,7 +18,6 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ #ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUG
@@ -44,7 +43,7 @@ extern int cifsERROR;* debug ON * -------- */ -#ifdef CIFS_DEBUG +#ifdef CONFIG_CIFS_DEBUG /* information message: e.g., configuration, major event */ #define cifsfyi (fmt, ...) \
Signature · 1 line
1.7.8.112.g3fd21
Re: [PATCH V2 3/3] cifs: Add CONFIG_CIFS_DEBUG and rename use of CIFS_DEBUG
Quoted 46 lines · On Fri, 30 Nov 2012 07:31:56 -0800 Joe Perches <[email protected]> wrote:
Quoted 45 lines · Jeff Layton
Quoted 29 lines · On Fri, 30 Nov 2012 06:57:40 -0800 Joe Perches <[email protected]> wrote:
Quoted 7 lines · Jeff Layton
Quoted 5 lines · On Thu, 29 Nov 2012 11:37:20 -0800 Joe Perches <[email protected]> wrote:
Rather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
[]
Quoted 14 lines
Quoted 14 lines
fs/cifs/cifs_debug.h +4
-1@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif Quoted 5 lines
fs/cifs/cifs_debug.h line 23
Quoted 2 lines
fs/cifs/cifs_debug.h +0
-0#ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUGLooks fine, but I'd probably prefer to just get rid of
CIFS_DEBUGaltogether here and replace it withCONFIG_CIFS_DEBUG.Quoted 3 lines
I wasn't sure if there was other code anywhere else that depends on
CIFS_DEBUG. There isn't any in the kernel source tree. If not, then you are right, it should be justCONFIG_CIFS_DEBUG.There shouldn't be. None of this stuff is exposed to userspace, AFAIK. I think it's ok to just make that
CONFIG_CIFS_DEBUG.Quoted 7 lines
Quoted 3 lines
While you're at it,
/proc/fs/cifs/cifsFYIshould probably not be present whenCONFIG_CIFS_DEBUGisn't set. Perhaps we should also disable traceSMB and its related functions too in that case?That could be considered a kernel api change.
If people agree to delete it, I'd be OK with it.
Quoted 8 lines
Quoted 3 lines
If you compile with
CONFIG_CIFS_DEBUGturned off, then the cifsFYI file will be a knob that's not hooked up to anything. I think it's best not to present it at all in that case.If there's a userspace script that does
echo 1 >/proc/fs/cifs/cifsFYI
it'd now get a file not found error.
It probably should fail since it wouldn't get the results it was looking for anyway...
Quoted 61 lines
Quoted 5 lines
traceSMB is a little harder to make the case for. Theoretically, it'll still work after your changes, but I'm not sure it makes sense to leave that knob around since it's technically a flag for debugging. You're probably fine not to worry about that one. I'll look at that at some point in the future.
This could be changed in a separate patch if appropriate. Here's the modified patch.
From: Joe Perches <[email protected]>
This can reduce the size of the module by ~120KB which could be useful for embedded systems.
$ size fs/cifs/built-in.o* text data bss dec hex filename 388567 34459 100440 523466 7fcca fs/cifs/built-in.o.new 495970 34599 117904 648473 9e519 fs/cifs/built-in.o.oldSigned-off-by Joe Perches <[email protected]>--- fs/cifs/Kconfig | 10 +++++++++- fs/cifs/cifs_debug.h | 3 +-- 2 files changed, 10 insertions(+), 3 deletions(-)fs/cifs/Kconfig +9
-1@@ -122,9 +122,17 @@ config CIFS_ACL Allows fetching CIFS/NTFS ACL from the server. The DACL blob is handed over to the application/caller. +config CIFS_DEBUG + bool "Enable CIFS debugging routines" + default y + depends on CIFS + help + Enabling this option adds helpful debugging messages to + the cifs code which increases the size of the cifs module. + If unsure, say Y. config CIFS_DEBUG2 bool "Enable additional CIFS debugging routines" - depends on CIFS + depends on CIFS_DEBUG help Enabling this option adds a few more debugging routines to the cifs code which slightly increases the size offs/cifs/cifs_debug.h +0
-1@@ -18,7 +18,6 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ #ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUG
fs/cifs/cifs_debug.h extern int cifsERROR; · line 49
Quoted 8 lines
fs/cifs/cifs_debug.h +1
-1@@ -44,7 +43,7 @@ extern int cifsERROR;* debug ON * -------- */ -#ifdef CIFS_DEBUG +#ifdef CONFIG_CIFS_DEBUG /* information message: e.g., configuration, major event */ #define cifsfyi (fmt, ...) \
Yep, fair enough, we can move the cifsFYI file under CONFIG_CIFS_DEBUG in a separate patch.
Re: [PATCH 3/3] cifs: Enable Kconfig control of CIFS_DEBUG
In reply to Jeff LaytonQuoted 53 lines · Jeff Layton
Quoted 30 lines · On Fri, 30 Nov 2012 06:57:40 -0800 Joe Perches…
Quoted 8 lines · Jeff Layton
Quoted 5 lines · On Thu, 29 Nov 2012 11:37:20 -0800 Joe Perches…
Rather than havign a permanently enabled #define, allow control over it.
This can reduce the size of the module by ~100KB which could be useful for embedded systems.
[]
Quoted 14 lines
Quoted 14 lines
fs/cifs/cifs_debug.h +4
-1@@ -18,7 +18,10 @@* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * */ -#define CIFS_DEBUG /* BB temporary */ + +#ifdef CONFIG_CIFS_DEBUG +#define CIFS_DEBUG +#endif Quoted 5 lines
fs/cifs/cifs_debug.h line 27
Quoted 2 lines
fs/cifs/cifs_debug.h +0
-0#ifndef _H_CIFS_DEBUG#define _H_CIFS_DEBUGLooks fine, but I'd probably prefer to just get rid of
CIFS_DEBUGaltogether here and replace it withCONFIG_CIFS_DEBUG.Quoted 4 lines
I wasn't sure if there was other code anywhere else that depends on
CIFS_DEBUG. There isn't any in the kernel source tree. If not, then you are right, it should be justCONFIG_CIFS_DEBUG.There shouldn't be. None of this stuff is exposed to userspace, AFAIK. I think it's ok to just make that
CONFIG_CIFS_DEBUG.Quoted 7 lines
Quoted 3 lines
While you're at it,
/proc/fs/cifs/cifsFYIshould probably not be present whenCONFIG_CIFS_DEBUGisn't set. Perhaps we should also disable traceSMB and its related functions too in that case?That could be considered a kernel api change.
If people agree to delete it, I'd be OK with it.If you compile with
CONFIG_CIFS_DEBUGturned off, then the cifsFYI file will be a knob that's not hooked up to anything. I think it's best not to present it at all in that case.
Quoted 5 lines
traceSMB is a little harder to make the case for. Theoretically, it'll still work after your changes, but I'm not sure it makes sense to leave that knob around since it's technically a flag for debugging. You're probably fine not to worry about that one. I'll look at that at some point in the future.
I think it is fine to leave the traceSMB around with debug config flag off - it is not strongly related to cERROR and cFYI and is small.
Signature · 3 lines
Thanks, Steve