Skip to contentSkip to conversation

optimize dmapi event tests w/o dmapi config

PATCH In review Eric Sandeen 3 participants 5 messages Aug 19, 2007 – Aug 20, 2007 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 optimize dmapi event tests w/o dmapi config

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Stack deltas on x86, gcc 4.1:

xfs_create -16
xfs_free_file_space -4
xfs_getbmap +4
xfs_link -8
xfs_mkidr -20
xfs_read -24
xfs_remove -12
xfs_rename -8
xfs_rmdir -16
xfs_setattr -20
xfs_splice_read -20
xfs_splice_write -20
xfs_unmount -48
xfs_write -20

fs/xfs/xfs_dmapi.h +6 -0
Index: linux-2.6.22.i386/fs/xfs/xfs_dmapi.h
===================================================================
@@ -66,6 +66,7 @@ typedef enum {
 } dm_right_t;
 #define HAVE_DM_RIGHT_T
 
+#ifdef CONFIG_XFS_DMAPI
 /* Defines for determining if an event message should be sent. */
 #define	DM_EVENT_ENABLED(vfsp, ip, event) ( \
 	unlikely ((vfsp)->vfs_flag & VFS_DMI) && \
@@ -78,6 +79,11 @@ typedef enum {
 		( ((io)->io_dmevmask & (1 << event)) || \
 		  ((io)->io_mount->m_dmevmask & (1 << event)) ) \
 	)
+#else
+#define DM_EVENT_ENABLED(vfsp, ip, event) (0)
+#define DM_EVENT_ENABLED_IO(vfsp, io, event) (0)
+#endif
+
 
 #define DM_XFS_VALID_FS_EVENTS		( \
 	(1 << DM_EVENT_PREUNMOUNT)	| \
fs/xfs/xfs_vfsops.c +2 -1
Index: linux-2.6.22.i386/fs/xfs/xfs_vfsops.c
===================================================================
@@ -568,6 +568,7 @@ xfs_unmount(
 	rip = mp->m_rootip;
 	rvp = XFS_ITOV(rip);
 
+#ifdef CONFIG_XFS_DMAPI
 	if (vfsp->vfs_flag & VFS_DMI) {
 		error = XFS_SEND_PREUNMOUNT(mp, vfsp,
 				rvp, DM_RIGHT_NULL, rvp, DM_RIGHT_NULL,
@@ -580,7 +581,7 @@ xfs_unmount(
 		unmount_event_flags = (mp->m_dmevmask & (1<<DM_EVENT_UNMOUNT))?
 					0 : DM_FLAGS_UNWANTED;
 	}
-
+#endif
 	/*
 	 * First blow any referenced inode from this file system
 	 * out of the reference cache, and delete the timer.
#1 lore raw
Quoted 4 lines · Eric Sandeen

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Sounds useful, especially as dmapi is not in mainline at all.

#2 lore raw
In reply to Eric Sandeen
Quoted 2 lines · Eric Sandeen

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Good idea Eric. A few remarks and a question below.

Quoted 3 lines

+#ifdef CONFIG_XFS_DMAPI

CONFIG_XFS_DMAPI is only defined when the DMAPI is statically linked to the kernel. When DMAPI is configured as a module, CONFIG_XFS_DMAPI_MODULE is defined. The HAVE_DMAPI is defined for both, static and module DMAPI configurations.

fs/xfs/xfs_dmapi.h typedef enum { · line 84

Quoted 10 lines

/* Defines for determining if an event message should be sent. */
#define DM_EVENT_ENABLED(vfsp, ip, event) ( \

fs/xfs/xfs_dmapi.h +2 -0
 	unlikely ((vfsp)->vfs_flag & VFS_DMI) && \
@@ -78,6 +79,11 @@ typedef enum {
 		( ((io)->io_dmevmask & (1 << event)) || \
 		  ((io)->io_mount->m_dmevmask & (1 << event)) ) \
 	)
+#else
+#define DM_EVENT_ENABLED(vfsp, ip, event) (0)
 

The above should use the new two arguments DM_EVENT_ENABLED macro.

fs/xfs/xfs_dmapi.h typedef enum { · line 87

Quoted 2 lines
fs/xfs/xfs_dmapi.h +1 -0
+#define DM_EVENT_ENABLED_IO(vfsp, io, event) (0)
 

What is this DM_EVENT_ENABLE_IO macro for?

Review typedef enum { · line 87

Quoted 2 lines
(quoted diff) +1 -0
+#ifdef CONFIG_XFS_DMAPI

The same, use HAVE_DMAPI instead of CONFIG_XFS_DMAPI.

Otherwise it is looking good.

Regards,
Vlad

#3 lore raw
Quoted 11 lines · Vlad Apostolov
Quoted 6 lines · Eric Sandeen

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Good idea Eric. A few remarks and a question below.

Quoted 1 line
Quoted 9 lines
Quoted 3 lines

+#ifdef CONFIG_XFS_DMAPI

CONFIG_XFS_DMAPI is only defined when the DMAPI is statically linked to the kernel. When DMAPI is configured as a module, CONFIG_XFS_DMAPI_MODULE
is defined. The HAVE_DMAPI is defined for both, static and module DMAPI configurations.

Hmm... ok. kernel.org doesn't have that so you'll need to take care moving this

patch to the kernel.org tree I guess.

Quoted 13 lines

Review typedef enum { · line 85

Quoted 11 lines

/* Defines for determining if an event message should be sent. */
#define DM_EVENT_ENABLED(vfsp, ip, event) ( \

(quoted diff) +2 -0
 	unlikely ((vfsp)->vfs_flag & VFS_DMI) && \
@@ -78,6 +79,11 @@ typedef enum {
 		( ((io)->io_dmevmask & (1 << event)) || \
 		  ((io)->io_mount->m_dmevmask & (1 << event)) ) \
 	)
+#else
+#define DM_EVENT_ENABLED(vfsp, ip, event) (0)
 
 

The above should use the new two arguments DM_EVENT_ENABLED macro.

Quoted 5 lines

Review typedef enum { · line 87

Quoted 3 lines
(quoted diff) +1 -0
+#define DM_EVENT_ENABLED_IO(vfsp, io, event) (0)
 

What is this DM_EVENT_ENABLE_IO macro for?

Ah, this is just the differernce between kernel.org & CVS I guess. I'm just re-defining existing macros in 2.6.22.1 to no-ops.

DM_EVENT_ENABLE_IO must have been removed from CVS w/o removing from kernel.org (yet) - and I guess same goes for the argument count.

Anyway here's a CVS patch, sorry:

------

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Stack deltas on x86, gcc 4.1:

xfs_create -16
xfs_free_file_space -4
xfs_getbmap +4
xfs_link -8
xfs_mkidr -20
xfs_read -24
xfs_remove -12
xfs_rename -8
xfs_rmdir -16
xfs_setattr -20
xfs_splice_read -20
xfs_splice_write -20
xfs_unmount -48
xfs_write -20

fs/xfs/xfs_dmapi.h +4 -0
Index: linux.orig/fs/xfs/xfs_dmapi.h
===================================================================
@@ -67,11 +67,15 @@ typedef enum {
 #define HAVE_DM_RIGHT_T
 
 /* Defines for determining if an event message should be sent. */
+#ifdef HAVE_DMAPI
 #define	DM_EVENT_ENABLED(ip, event) ( \
 	unlikely (XFS_MTOVFS((ip)->i_mount)->vfs_flag & VFS_DMI) && \
 		( ((ip)->i_d.di_dmevmask & (1 << event)) || \
 		  ((ip)->i_mount->m_dmevmask & (1 << event)) ) \
 	)
+#else
+#define DM_EVENT_ENABLED(ip, event)	(0)
+#endif
 
 #define DM_XFS_VALID_FS_EVENTS		( \
 	(1 << DM_EVENT_PREUNMOUNT)	| \
fs/xfs/xfs_vfsops.c +2 -1
Index: linux.orig/fs/xfs/xfs_vfsops.c
===================================================================
@@ -572,6 +572,7 @@ xfs_unmount(
 	rip = mp->m_rootip;
 	rvp = XFS_ITOV(rip);
 
+#ifdef HAVE_DMAPI
 	if (vfsp->vfs_flag & VFS_DMI) {
 		error = XFS_SEND_PREUNMOUNT(mp, vfsp,
 				rvp, DM_RIGHT_NULL, rvp, DM_RIGHT_NULL,
@@ -584,7 +585,7 @@ xfs_unmount(
 		unmount_event_flags = (mp->m_dmevmask & (1<<DM_EVENT_UNMOUNT))?
 					0 : DM_FLAGS_UNWANTED;
 	}
-
+#endif
 	/*
 	 * First blow any referenced inode from this file system
 	 * out of the reference cache, and delete the timer.
#4 lore raw

I ran the DMAPI QA with the updated patch and it was working fine. I will push it in the XFS development tree.
Thanks Eric.

Regards,
Vlad

Quoted 126 lines · Eric Sandeen
Quoted 11 lines · Vlad Apostolov
Quoted 4 lines · Eric Sandeen

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Good idea Eric. A few remarks and a question below.

Quoted 1 line
Quoted 10 lines
Quoted 4 lines

+#ifdef CONFIG_XFS_DMAPI

CONFIG_XFS_DMAPI is only defined when the DMAPI is statically linked to the kernel. When DMAPI is configured as a module, CONFIG_XFS_DMAPI_MODULE is defined. The HAVE_DMAPI is defined for both, static and module DMAPI configurations.

Hmm... ok. kernel.org doesn't have that so you'll need to take care moving this

patch to the kernel.org tree I guess.

Quoted 15 lines

Review typedef enum { · line 86

Quoted 12 lines

/* Defines for determining if an event message should be sent. */
#define DM_EVENT_ENABLED(vfsp, ip, event) ( \

(quoted diff) +2 -0
 	unlikely ((vfsp)->vfs_flag & VFS_DMI) && \
@@ -78,6 +79,11 @@ typedef enum {
 		( ((io)->io_dmevmask & (1 << event)) || \
 		  ((io)->io_mount->m_dmevmask & (1 << event)) ) \
 	)
+#else
+#define DM_EVENT_ENABLED(vfsp, ip, event) (0)
 
 
 

The above should use the new two arguments DM_EVENT_ENABLED macro.

Quoted 7 lines

Review typedef enum { · line 87

Quoted 4 lines
(quoted diff) +1 -0
+#define DM_EVENT_ENABLED_IO(vfsp, io, event) (0)

What is this DM_EVENT_ENABLE_IO macro for?

Ah, this is just the differernce between kernel.org & CVS I guess. I'm just re-defining existing macros in 2.6.22.1 to no-ops.

DM_EVENT_ENABLE_IO must have been removed from CVS w/o removing from kernel.org (yet) - and I guess same goes for the argument count.

Anyway here's a CVS patch, sorry:

------

Defining XFS_DM_EVENT* macros to 0 in the absence of CONFIG_XFS_DMAPI allows gcc to optimize away tests that should never be true. Also wrap one hunk of xfs_unmount in #ifdef CONFIG_XFS_DMAPI

Stack deltas on x86, gcc 4.1:

xfs_create -16
xfs_free_file_space -4
xfs_getbmap +4
xfs_link -8
xfs_mkidr -20
xfs_read -24
xfs_remove -12
xfs_rename -8
xfs_rmdir -16
xfs_setattr -20
xfs_splice_read -20
xfs_splice_write -20
xfs_unmount -48
xfs_write -20

Index: linux.orig/fs/xfs/xfs_dmapi.h
===================================================================

fs/xfs/xfs_dmapi.h +4 -0
@@ -67,11 +67,15 @@ typedef enum {
 #define HAVE_DM_RIGHT_T
 
 /* Defines for determining if an event message should be sent. */
+#ifdef HAVE_DMAPI
 #define	DM_EVENT_ENABLED(ip, event) ( \
 	unlikely (XFS_MTOVFS((ip)->i_mount)->vfs_flag & VFS_DMI) && \
 		( ((ip)->i_d.di_dmevmask & (1 << event)) || \
 		  ((ip)->i_mount->m_dmevmask & (1 << event)) ) \
 	)
+#else
+#define DM_EVENT_ENABLED(ip, event)	(0)
+#endif
 
 #define DM_XFS_VALID_FS_EVENTS		( \
 	(1 << DM_EVENT_PREUNMOUNT)	| \

Index: linux.orig/fs/xfs/xfs_vfsops.c
===================================================================

fs/xfs/xfs_vfsops.c +2 -1
@@ -572,6 +572,7 @@ xfs_unmount(
 	rip = mp->m_rootip;
 	rvp = XFS_ITOV(rip);
 
+#ifdef HAVE_DMAPI
 	if (vfsp->vfs_flag & VFS_DMI) {
 		error = XFS_SEND_PREUNMOUNT(mp, vfsp,
 				rvp, DM_RIGHT_NULL, rvp, DM_RIGHT_NULL,
@@ -584,7 +585,7 @@ xfs_unmount(
 		unmount_event_flags = (mp->m_dmevmask & (1<<DM_EVENT_UNMOUNT))?
 					0 : DM_FLAGS_UNWANTED;
 	}
-
+#endif
 	/*
 	 * First blow any referenced inode from this file system
 	 * out of the reference cache, and delete the timer.