make xfs stats Kconfigurable

PATCH Superseded Eric Sandeen 2 participants 5 messages Aug 18, 2007 – Aug 19, 2007 XFS filesystem

lore

Share & tools

lore

Eric Sandeen

author

2007-08-18 20:04 UTC #0 lore raw

PATCH 1/1 make xfs stats Kconfigurable

Hey guys, what do you think, is it worth doing this for when space is tight:

xfs stats are already optional based on CONFIG_PROC_FS, and there is already a (never-set) macro XFS_STATS_OFF which can turn them off. Making it a configurable option, if I turn them off I can get a little stack savings on x86, gcc 4.1:

xfs_attr_list -8
xfs_bmap_rtalloc -16
xfs_bmap_search_extents -8
xfs_buf_find +8 (not sure why)
xfs_iflush -4
xfs_imap -12
xfs_log_reserve -4
xfs_log_sync -8

It'd reduce the size of the binary a bit too.

Signed-off-by Eric Sandeen <[email protected]>

fs/xfs/Kconfig +11 -0

Index: linux-2.6.22.i386/fs/xfs/Kconfig
===================================================================
@@ -59,6 +59,17 @@ config XFS_POSIX_ACL
 
 	  If you don't know what Access Control Lists are, say N.
 
+	config XFS_STATS
+	bool "XFS statistics"
+	depends on XFS_FS && PROC_FS
+	help
+	  If you say Y here xfs will gather detailed statistics during
+	  its operation, and present them in /proc/fs/xfs/stat and
+	  (CONFIG_XFS_QUOTA is enabled) /proc/fs/xfs/xqmstat.  Disabling
+	  this will make the compiled code and stack usage slightly smaller.
+
+	  If unsure, say Y.
+
 config XFS_RT
 	bool "XFS Realtime subvolume support"
 	depends on XFS_FS

fs/xfs/Makefile-linux-2.6 +2 -2

Index: linux-2.6.22.i386/fs/xfs/Makefile-linux-2.6
===================================================================
@@ -35,12 +35,12 @@ xfs-$(CONFIG_XFS_QUOTA)		+= $(addprefix 
 				   xfs_qm.o)
 
 ifeq ($(CONFIG_XFS_QUOTA),y)
-xfs-$(CONFIG_PROC_FS)		+= quota/xfs_qm_stats.o
+xfs-$(CONFIG_XFS_STAT)		+= quota/xfs_qm_stats.o
 endif
 
 xfs-$(CONFIG_XFS_RT)		+= xfs_rtalloc.o
 xfs-$(CONFIG_XFS_POSIX_ACL)	+= xfs_acl.o
-xfs-$(CONFIG_PROC_FS)		+= $(XFS_LINUX)/xfs_stats.o
+xfs-$(CONFIG_XFS_STATS)		+= $(XFS_LINUX)/xfs_stats.o
 xfs-$(CONFIG_SYSCTL)		+= $(XFS_LINUX)/xfs_sysctl.o
 xfs-$(CONFIG_COMPAT)		+= $(XFS_LINUX)/xfs_ioctl32.o

fs/xfs/linux-2.6/xfs_stats.h +3 -4

Index: linux-2.6.22.i386/fs/xfs/linux-2.6/xfs_stats.h
===================================================================
@@ -19,7 +19,7 @@
 #define __XFS_STATS_H__
 
 
-#if defined(CONFIG_PROC_FS) && !defined(XFS_STATS_OFF)
+#if defined(CONFIG_XFS_STATS)
 
 #include <linux/percpu.h>

@@ -137,8 +137,7 @@ DECLARE_PER_CPU(struct xfsstats, xfsstat
 extern void xfs_init_procfs(void);
 extern void xfs_cleanup_procfs(void);
 
-
-#else	/* !CONFIG_PROC_FS */
+#else	/* !CONFIG_XFS_STATS */
 
 # define XFS_STATS_INC(count)
 # define XFS_STATS_DEC(count)

@@ -147,6 +146,6 @@ extern void xfs_cleanup_procfs(void);
 static __inline void xfs_init_procfs(void) { };
 static __inline void xfs_cleanup_procfs(void) { };
 
-#endif	/* !CONFIG_PROC_FS */
+#endif /* !CONFIG_XFS_STATS */
 
 #endif /* __XFS_STATS_H__ */

fs/xfs/linux-2.6/xfs_sysctl.c +3 -3

Index: linux-2.6.22.i386/fs/xfs/linux-2.6/xfs_sysctl.c
===================================================================
@@ -21,7 +21,7 @@
 
 static struct ctl_table_header *xfs_table_header;
 
-#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_XFS_STATS
 STATIC int
 xfs_stats_clear_proc_handler(
 	ctl_table	*ctl,

@@ -52,7 +52,7 @@ xfs_stats_clear_proc_handler(
 
 	return ret;
 }
-#endif /* CONFIG_PROC_FS */
+#endif /* CONFIG_XFS_STATS */
 
 static ctl_table xfs_table[] = {
 	{

@@ -222,7 +222,7 @@ static ctl_table xfs_table[] = {
 		.extra2		= &xfs_params.fstrm_timer.max,
 	},
 	/* please keep this the last entry */
-#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_XFS_STATS
 	{
 		.ctl_name	= XFS_STATS_CLEAR,
 		.procname	= "stats_clear",

fs/xfs/quota/xfs_qm_stats.h +1 -1

Index: linux-2.6.22.i386/fs/xfs/quota/xfs_qm_stats.h
===================================================================
@@ -18,7 +18,7 @@
 #ifndef __XFS_QM_STATS_H__
 #define __XFS_QM_STATS_H__
 
-#if defined(CONFIG_PROC_FS) && !defined(XFS_STATS_OFF)
+#ifdef CONFIG_XFS_STATS
 
 /*
  * XQM global statistics

Eric Sandeen

author

2007-08-19 17:21 UTC #1 lore raw

PATCH v2 1/1 make xfs stats Kconfigurable

Oops, typo in Makefile (CONFIG_XFS_STAT vs _STATS).

----

xfs_attr_list -8
xfs_bmap_rtalloc -16
xfs_bmap_search_extents -8
xfs_buf_find +8 (not sure why)
xfs_iflush -4
xfs_imap -12
xfs_log_reserve -4
xfs_log_sync -8

It'd reduce the size of the binary a bit too.

Signed-off-by Eric Sandeen <[email protected]>

fs/xfs/Kconfig +11 -0

Index: linux-2.6.22.i386/fs/xfs/Kconfig
===================================================================
@@ -59,6 +59,17 @@ config XFS_POSIX_ACL
 
 	  If you don't know what Access Control Lists are, say N.
 
+	config XFS_STATS
+	bool "XFS statistics"
+	depends on XFS_FS && PROC_FS
+	help
+	  If you say Y here xfs will gather detailed statistics during
+	  its operation, and present them in /proc/fs/xfs/stat and
+	  (CONFIG_XFS_QUOTA is enabled) /proc/fs/xfs/xqmstat.  Disabling
+	  this will make the compiled code and stack usage slightly smaller.
+
+	  If unsure, say Y.
+
 config XFS_RT
 	bool "XFS Realtime subvolume support"
 	depends on XFS_FS

fs/xfs/Makefile-linux-2.6 +2 -2

Index: linux-2.6.22.i386/fs/xfs/Makefile-linux-2.6
===================================================================
@@ -35,12 +35,12 @@ xfs-$(CONFIG_XFS_QUOTA)		+= $(addprefix 
 				   xfs_qm.o)
 
 ifeq ($(CONFIG_XFS_QUOTA),y)
-xfs-$(CONFIG_PROC_FS)		+= quota/xfs_qm_stats.o
+xfs-$(CONFIG_XFS_STATS)		+= quota/xfs_qm_stats.o
 endif
 
 xfs-$(CONFIG_XFS_RT)		+= xfs_rtalloc.o
 xfs-$(CONFIG_XFS_POSIX_ACL)	+= xfs_acl.o
-xfs-$(CONFIG_PROC_FS)		+= $(XFS_LINUX)/xfs_stats.o
+xfs-$(CONFIG_XFS_STATS)		+= $(XFS_LINUX)/xfs_stats.o
 xfs-$(CONFIG_SYSCTL)		+= $(XFS_LINUX)/xfs_sysctl.o
 xfs-$(CONFIG_COMPAT)		+= $(XFS_LINUX)/xfs_ioctl32.o

fs/xfs/linux-2.6/xfs_stats.h +3 -4

Index: linux-2.6.22.i386/fs/xfs/linux-2.6/xfs_stats.h
===================================================================
@@ -19,7 +19,7 @@
 #define __XFS_STATS_H__
 
 
-#if defined(CONFIG_PROC_FS) && !defined(XFS_STATS_OFF)
+#if defined(CONFIG_XFS_STATS)
 
 #include <linux/percpu.h>

@@ -137,8 +137,7 @@ DECLARE_PER_CPU(struct xfsstats, xfsstat
 extern void xfs_init_procfs(void);
 extern void xfs_cleanup_procfs(void);
 
-
-#else	/* !CONFIG_PROC_FS */
+#else	/* !CONFIG_XFS_STATS */
 
 # define XFS_STATS_INC(count)
 # define XFS_STATS_DEC(count)

@@ -147,6 +146,6 @@ extern void xfs_cleanup_procfs(void);
 static __inline void xfs_init_procfs(void) { };
 static __inline void xfs_cleanup_procfs(void) { };
 
-#endif	/* !CONFIG_PROC_FS */
+#endif /* !CONFIG_XFS_STATS */
 
 #endif /* __XFS_STATS_H__ */

fs/xfs/linux-2.6/xfs_sysctl.c +3 -3

Index: linux-2.6.22.i386/fs/xfs/linux-2.6/xfs_sysctl.c
===================================================================
@@ -21,7 +21,7 @@
 
 static struct ctl_table_header *xfs_table_header;
 
-#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_XFS_STATS
 STATIC int
 xfs_stats_clear_proc_handler(
 	ctl_table	*ctl,

@@ -52,7 +52,7 @@ xfs_stats_clear_proc_handler(
 
 	return ret;
 }
-#endif /* CONFIG_PROC_FS */
+#endif /* CONFIG_XFS_STATS */
 
 static ctl_table xfs_table[] = {
 	{

@@ -222,7 +222,7 @@ static ctl_table xfs_table[] = {
 		.extra2		= &xfs_params.fstrm_timer.max,
 	},
 	/* please keep this the last entry */
-#ifdef CONFIG_PROC_FS
+#ifdef CONFIG_XFS_STATS
 	{
 		.ctl_name	= XFS_STATS_CLEAR,
 		.procname	= "stats_clear",

fs/xfs/quota/xfs_qm_stats.h +1 -1

Index: linux-2.6.22.i386/fs/xfs/quota/xfs_qm_stats.h
===================================================================
@@ -18,7 +18,7 @@
 #ifndef __XFS_QM_STATS_H__
 #define __XFS_QM_STATS_H__
 
-#if defined(CONFIG_PROC_FS) && !defined(XFS_STATS_OFF)
+#ifdef CONFIG_XFS_STATS
 
 /*
  * XQM global statistics

Eric Sandeen

author

2007-08-19 18:33 UTC #2 lore raw

Quoted 9 lines · Eric Sandeen

Oops, typo in Makefile (CONFIG_XFS_STAT vs _STATS).
----
xfs stats are already optional based on CONFIG_PROC_FS, and there is already a (never-set) macro XFS_STATS_OFF which can turn them off. Making it a configurable option, if I turn them off I can get a little stack savings on x86, gcc 4.1:

This causes feature -q in xfsqa to report no quotas, because /proc/fs/xfs/xqm isn't present, but I think that check should just be removed...?

-Eric

Christoph Hellwig

2007-08-19 19:29 UTC #3 lore raw

In reply to Eric Sandeen

Quoted 9 lines · Eric Sandeen

Oops, typo in Makefile (CONFIG_XFS_STAT vs _STATS).
----
xfs stats are already optional based on CONFIG_PROC_FS, and there is already a (never-set) macro XFS_STATS_OFF which can turn them off. Making it a configurable option, if I turn them off I can get a little stack savings on x86, gcc 4.1:

Why would we want to disable the statistics?

fs/xfs/Kconfig config XFS_POSIX_ACL · line 62

Quoted 1 line

fs/xfs/Kconfig +1 -0
+	config XFS_STATS

Everywhere else the config statement is not indented, so it probably shouldn't be here either.

Eric Sandeen

author

2007-08-19 19:13 UTC #4 lore raw

Quoted 6 lines · Christoph Hellwig

Quoted 5 lines · Eric Sandeen
Oops, typo in Makefile (CONFIG_XFS_STAT vs _STATS).
----

Quoted 6 lines

Quoted 4 lines
xfs stats are already optional based on CONFIG_PROC_FS, and there is already a (never-set) macro XFS_STATS_OFF which can turn them off. Making it a configurable option, if I turn them off I can get a little stack savings on x86, gcc 4.1:
Why would we want to disable the statistics?

To save a little stack space. :) *shrug*

This one's not so critical. FWIW, JFS has configurable stats, and I think an upcoming jbd patch adds (config-optional) stats.

OTOH when are stats really used outside of sgi? Though I suppose occasionally they'd be useful in field debugging.

*shrug* this one may not be worth it; I won't be heartbroken if it doesn't go in... it helps a tiny bit on the stack side.

-Eric

Quoted 5 lines

fs/xfs/Kconfig config XFS_POSIX_ACL · line 62
Quoted 1 line
fs/xfs/Kconfig +1 -0
+	config XFS_STATS
Everywhere else the config statement is not indented, so it probably shouldn't be here either.