[PATCH] xfs: optimize fsync on directories

[PATCH] xfs: optimize fsync on directories

[Christoph Hellwig]

Directories are only updated transactionally, which means fsync only
needs to flush the log the inode is currently dirty, but not bother
with checking for dirty data, non-transaction updates, an most importanly
doesn't have to flush disk caches except as part of a transaction commit.

While the first two optimizations can't easily be measured the latter
actually make a difference when doing lots of fsync that do not actually
have to commit the inode, e.g. becase an ealier fsync already pushed
the log far enough.

The new xfs_dir_fsync is identifical to xfs_nfs_commit_metadata except
for the prototype, but I'm not sure creating a common helper for the
two is worth it given how simple the functions are.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_file.c
===================================================================
--- xfs.orig/fs/xfs/xfs_file.c	2011-09-26 10:56:48.607884402 +0200
+++ xfs/fs/xfs/xfs_file.c	2011-09-26 10:59:56.922867311 +0200
@@ -124,6 +124,35 @@ xfs_iozero(
 	return (-status);
 }
 
+/*
+ * Fsync operations on directories are much simpler than on regular files,
+ * as there is no file data to flush, and thus also no need for explicit
+ * cache flush operations, and there are no non-transaction metadata updates
+ * on directories either.
+ */
+STATIC int
+xfs_dir_fsync(
+	struct file		*file,
+	loff_t			start,
+	loff_t			end,
+	int			datasync)
+{
+	struct xfs_inode	*ip = XFS_I(file->f_mapping->host);
+	struct xfs_mount	*mp = ip->i_mount;
+	xfs_lsn_t		lsn = 0;
+
+	trace_xfs_dir_fsync(ip);
+
+	xfs_ilock(ip, XFS_ILOCK_SHARED);
+	if (xfs_ipincount(ip))
+		lsn = ip->i_itemp->ili_last_lsn;
+	xfs_iunlock(ip, XFS_ILOCK_SHARED);
+
+	if (!lsn)
+		return 0;
+	return _xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL);
+}
+
 STATIC int
 xfs_file_fsync(
 	struct file		*file,
@@ -1140,7 +1169,7 @@ const struct file_operations xfs_dir_fil
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= xfs_file_compat_ioctl,
 #endif
-	.fsync		= xfs_file_fsync,
+	.fsync		= xfs_dir_fsync,
 };
 
 static const struct vm_operations_struct xfs_file_vm_ops = {
Index: xfs/fs/xfs/xfs_trace.h
===================================================================
--- xfs.orig/fs/xfs/xfs_trace.h	2011-09-26 11:00:05.367867252 +0200
+++ xfs/fs/xfs/xfs_trace.h	2011-09-26 11:00:22.090368120 +0200
@@ -576,6 +576,7 @@ DEFINE_INODE_EVENT(xfs_vm_bmap);
 DEFINE_INODE_EVENT(xfs_file_ioctl);
 DEFINE_INODE_EVENT(xfs_file_compat_ioctl);
 DEFINE_INODE_EVENT(xfs_ioctl_setattr);
+DEFINE_INODE_EVENT(xfs_dir_fsync);
 DEFINE_INODE_EVENT(xfs_file_fsync);
 DEFINE_INODE_EVENT(xfs_destroy_inode);
 DEFINE_INODE_EVENT(xfs_write_inode);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: optimize fsync on directories

[Alex Elder]

On Sun, 2011-10-02 at 10:25 -0400, Christoph Hellwig wrote:
> Directories are only updated transactionally, which means fsync only
> needs to flush the log the inode is currently dirty, but not bother
> with checking for dirty data, non-transaction updates, an most importanly
> doesn't have to flush disk caches except as part of a transaction commit.
> 
> While the first two optimizations can't easily be measured the latter
> actually make a difference when doing lots of fsync that do not actually
> have to commit the inode, e.g. becase an ealier fsync already pushed
> the log far enough.
> 
> The new xfs_dir_fsync is identifical to xfs_nfs_commit_metadata except
> for the prototype, but I'm not sure creating a common helper for the
> two is worth it given how simple the functions are.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs