xfs
[Top] [All Lists]

[PATCH 5/5] xfs: using generic_file_direct_write() is unnecessary

To: xfs@xxxxxxxxxxx
Subject: [PATCH 5/5] xfs: using generic_file_direct_write() is unnecessary
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 10 Apr 2015 23:38:00 +1000
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1428673080-23052-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1428673080-23052-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

generic_file_direct_write() does all sorts of things to make DIO
work "sorta ok" with mixed buffered IO workloads. We already do
most of this work in xfs_file_aio_dio_write() because of the locking
requirements, so there's only a couple of things it does for us.

The first thing is that it does a page cache invalidation after the
->direct_IO callout. This can easily be added to the XFS code.

The second thing it does is that if data was written, it updates the
iov_iter structure to reflect the data written, and then does EOF
size updates if necessary. For XFS, these EOF size updates are now
not necessary, as we do them safely and race-free in IO completion
context. That leaves just the iov_iter update, and that's also exily
moved to the XFS code.

The result is that we don't need to call
generic_file_direct_write(), and hence remove a redundant buffered
writeback call and a redundant page cache invalidation call from the
DIO submission path. We also remove a racy EOF size update, and make
the DIO submission code in XFS much easier to follow. Wins all
round, really.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/xfs_file.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 7b872f4..7182cd2 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -665,6 +665,8 @@ xfs_file_dio_aio_write(
        int                     iolock;
        size_t                  count = iov_iter_count(from);
        loff_t                  pos = iocb->ki_pos;
+       loff_t                  end;
+       struct iov_iter         data;
        struct xfs_buftarg      *target = XFS_IS_REALTIME_INODE(ip) ?
                                        mp->m_rtdev_targp : mp->m_ddev_targp;
 
@@ -704,10 +706,11 @@ xfs_file_dio_aio_write(
        if (ret)
                goto out;
        iov_iter_truncate(from, count);
+       end = pos + count - 1;
 
        if (mapping->nrpages) {
                ret = filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
-                                                   pos, pos + count - 1);
+                                                  pos, end);
                if (ret)
                        goto out;
                /*
@@ -717,7 +720,7 @@ xfs_file_dio_aio_write(
                 */
                ret = invalidate_inode_pages2_range(VFS_I(ip)->i_mapping,
                                        pos >> PAGE_CACHE_SHIFT,
-                                       (pos + count - 1) >> PAGE_CACHE_SHIFT);
+                                       end >> PAGE_CACHE_SHIFT);
                WARN_ON_ONCE(ret);
                ret = 0;
        }
@@ -734,8 +737,22 @@ xfs_file_dio_aio_write(
        }
 
        trace_xfs_file_direct_write(ip, count, iocb->ki_pos, 0);
-       ret = generic_file_direct_write(iocb, from, pos);
 
+       data = *from;
+       ret = mapping->a_ops->direct_IO(WRITE, iocb, &data, pos);
+
+       /* see generic_file_direct_write() for why this is necessary */
+       if (mapping->nrpages) {
+               invalidate_inode_pages2_range(mapping,
+                                             pos >> PAGE_CACHE_SHIFT,
+                                             end >> PAGE_CACHE_SHIFT);
+       }
+
+       if (ret > 0) {
+               pos += ret;
+               iov_iter_advance(from, ret);
+               iocb->ki_pos = pos;
+       }
 out:
        xfs_rw_iunlock(ip, iolock);
 
-- 
2.0.0

<Prev in Thread] Current Thread [Next in Thread>