* [PATCH] xfs: preallocation transactions do not need to be synchronous
@ 2011-03-09 23:37 Dave Chinner
2011-03-10 7:36 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2011-03-09 23:37 UTC (permalink / raw)
To: xfs
From: Dave Chinner <dchinner@redhat.com>
Preallocation and hole punch transactions are currently synchronous
and this is causing performance problems in some cases. The
transactions don't need to be synchronous as we don't need to
guarantee the preallocation is persistent on disk until a
fdatasync, fsync, sync operation occurs. If the file is opened
O_SYNC or O_DATASYNC, only then should the transaction be issued
synchronously.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/linux-2.6/xfs_file.c | 4 ++++
fs/xfs/linux-2.6/xfs_ioctl.c | 4 ++++
fs/xfs/xfs_vnodeops.c | 3 ++-
fs/xfs/xfs_vnodeops.h | 1 +
4 files changed, 11 insertions(+), 1 deletions(-)
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;
diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c
index 0ca0e3c..acca2c5 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl.c
+++ b/fs/xfs/linux-2.6/xfs_ioctl.c
@@ -624,6 +624,10 @@ xfs_ioc_space(
if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
attr_flags |= XFS_ATTR_NONBLOCK;
+
+ if (filp->f_flags & O_DSYNC)
+ attr_flags |= XFS_ATTR_SYNC;
+
if (ioflags & IO_INVIS)
attr_flags |= XFS_ATTR_DMI;
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 37d8146..c48b421 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -2831,7 +2831,8 @@ xfs_change_file_space(
ip->i_d.di_flags &= ~XFS_DIFLAG_PREALLOC;
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- xfs_trans_set_sync(tp);
+ if (attr_flags & XFS_ATTR_SYNC)
+ xfs_trans_set_sync(tp);
error = xfs_trans_commit(tp, 0);
diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h
index f670292..3bcd233 100644
--- a/fs/xfs/xfs_vnodeops.h
+++ b/fs/xfs/xfs_vnodeops.h
@@ -18,6 +18,7 @@ int xfs_setattr(struct xfs_inode *ip, struct iattr *vap, int flags);
#define XFS_ATTR_NONBLOCK 0x02 /* return EAGAIN if operation would block */
#define XFS_ATTR_NOLOCK 0x04 /* Don't grab any conflicting locks */
#define XFS_ATTR_NOACL 0x08 /* Don't call xfs_acl_chmod */
+#define XFS_ATTR_SYNC 0x10 /* synchronous operation required */
int xfs_readlink(struct xfs_inode *ip, char *link);
int xfs_release(struct xfs_inode *ip);
--
1.7.2.3
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: preallocation transactions do not need to be synchronous
2011-03-09 23:37 [PATCH] xfs: preallocation transactions do not need to be synchronous Dave Chinner
@ 2011-03-10 7:36 ` Christoph Hellwig
2011-03-17 14:23 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2011-03-10 7:36 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On Thu, Mar 10, 2011 at 10:37:32AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Preallocation and hole punch transactions are currently synchronous
> and this is causing performance problems in some cases. The
> transactions don't need to be synchronous as we don't need to
> guarantee the preallocation is persistent on disk until a
> fdatasync, fsync, sync operation occurs. If the file is opened
> O_SYNC or O_DATASYNC, only then should the transaction be issued
> synchronously.
I had the same patch in my queue, but it turns out even that isn't
enough for good performance when used fallocate in the fast path (e.g.
recent Samba). I'll send a more comprehensive optimization that
includes this soon.
I also refactored the whole area to make the various flags more
sensible.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: preallocation transactions do not need to be synchronous
2011-03-10 7:36 ` Christoph Hellwig
@ 2011-03-17 14:23 ` Christoph Hellwig
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2011-03-17 14:23 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On Thu, Mar 10, 2011 at 02:36:34AM -0500, Christoph Hellwig wrote:
> I had the same patch in my queue, but it turns out even that isn't
> enough for good performance when used fallocate in the fast path (e.g.
> recent Samba). I'll send a more comprehensive optimization that
> includes this soon.
>
> I also refactored the whole area to make the various flags more
> sensible.
Given that 2.6.38 is out now and I won't have my series ready in time
I'm fine with putting your minimal version in for now, I'll port mine
on top.
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-03-17 14:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-09 23:37 [PATCH] xfs: preallocation transactions do not need to be synchronous Dave Chinner
2011-03-10 7:36 ` Christoph Hellwig
2011-03-17 14:23 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox