From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: with ECARTIS (v1.0.0; list xfs); Sun, 19 Aug 2007 20:37:19 -0700 (PDT) Received: from sandeen.net (sandeen.net [209.173.210.139]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id l7K3bEbm023586 for ; Sun, 19 Aug 2007 20:37:15 -0700 Message-ID: <46C90C77.5070502@sandeen.net> Date: Sun, 19 Aug 2007 22:37:27 -0500 From: Eric Sandeen MIME-Version: 1.0 Subject: Re: [PATCH] optimize dmapi event tests w/o dmapi config References: <46C7DD96.6030001@sandeen.net> <46C8D6E8.2080500@sgi.com> In-Reply-To: <46C8D6E8.2080500@sgi.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: xfs-bounce@oss.sgi.com Errors-to: xfs-bounce@oss.sgi.com List-Id: xfs To: Vlad Apostolov Cc: xfs-oss Vlad Apostolov wrote: > Eric Sandeen wrote: > >> 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. > >> >> +#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. >> /* 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) >> >> > The above should use the new two arguments DM_EVENT_ENABLED macro. > >> +#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 Signed-off-by: Eric Sandeen Index: linux.orig/fs/xfs/xfs_dmapi.h =================================================================== --- linux.orig/fs/xfs/xfs_dmapi.h +++ linux/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) | \ Index: linux.orig/fs/xfs/xfs_vfsops.c =================================================================== --- linux.orig/fs/xfs/xfs_vfsops.c +++ linux/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<