RE: [PATCH 1/6] xfs: preallocation transactions do not need to be synchronous

brian.foster
2011-03-24T17:15:56Z

> -----Original Message-----
> From: xfs-bounces@oss.sgi.com [mailto:xfs-bounces@oss.sgi.com] On
> Behalf Of Dave Chinner
> Sent: Wednesday, March 23, 2011 2:14 AM
> To: xfs@oss.sgi.com
> Cc: aelder@sgi.com
> Subject: [PATCH 1/6] xfs: preallocation transactions do not need to be
> synchronous

> diff --git a/fs/xfs/linux-2.6/xfs_file.c b/fs/xfs/linux-2.6/xfs_file.c index
> ae59865..baa2cb3 100644
> --- a/fs/xfs/linux-2.6/xfs_file.c
> +++ b/fs/xfs/linux-2.6/xfs_file.c
> @@ -896,6 +896,7 @@ xfs_file_fallocate(
> xfs_flock64_t bf;
> xfs_inode_t *ip = XFS_I(inode);
> int cmd = XFS_IOC_RESVSP;
> + int attr_flags = XFS_ATTR_NOLOCK;
>
> if (mode & ~(FALLOC_FL_KEEP_SIZE |
> FALLOC_FL_PUNCH_HOLE |
> @@ -922,6 +923,9 @@ xfs_file_fallocate(
> goto out_unlock;
> }
>
> + if (file->f_flags & O_DSYNC)
> + attr_flags |= XFS_ATTR_SYNC;
> +
> error = -xfs_change_file_space(ip, cmd, &bf, 0, XFS_ATTR_NOLOCK);
> if (error)
> goto out_unlock;

Where are you passing attr_flags?

I was looking at this because I noticed a nice performance improvement in some Samba tests with the xfs_trans_set_sync() call removed, but I have a follow up question... I'd like to back port this patch to 2.6.34. The majority of this patch applies (manually), but the segment above is problematic in that I have an xfs_vn_fallocate() inode operations handler (xfs_iops.c) without the file pointer rather than the above file operations handler.

Without really knowing any of this code, I assume I can IS_SYNC() the inode to check for the sync mount situation? Assuming that is correct, is there a straightforward way to cover the open(..., O_SYNC) situation? I suppose I'm open to hacking the VFS to one off this call if I have to, but would rather avoid the ugliness. Any comments are appreciated, thanks.

Brian