On Tue, Apr 14, 2015 at 05:26:51PM +1000, Dave Chinner wrote: > 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 moved > to the XFS code. > > Therefore we don't need to call generic_file_direct_write() and in > doing so remove redundant buffered writeback and page cache > invalidation calls 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> > --- Reviewed-by: Brian Foster <bfoster@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 c398ec7..3a5d305 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 > > _______________________________________________ > xfs mailing list > xfs@xxxxxxxxxxx > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs