From 5724be5de88f5f6863d44c859f42f70d5cc667ed Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:09 -0800 Subject: [PATCH 01/11] iomap: rename the flags variable in __iomap_dio_rw Rename flags to iomap_flags to make the usage a little more clear. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/iomap/direct-io.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 933f234d5bec..604103ab76f9 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -427,7 +427,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, size_t count = iov_iter_count(iter); loff_t pos = iocb->ki_pos; loff_t end = iocb->ki_pos + count - 1, ret = 0; - unsigned int flags = IOMAP_DIRECT; + unsigned int iomap_flags = IOMAP_DIRECT; struct blk_plug plug; struct iomap_dio *dio; @@ -461,7 +461,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, if (iter_is_iovec(iter)) dio->flags |= IOMAP_DIO_DIRTY; } else { - flags |= IOMAP_WRITE; + iomap_flags |= IOMAP_WRITE; dio->flags |= IOMAP_DIO_WRITE; /* for data sync or sync, we need sync completion processing */ @@ -483,7 +483,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, ret = -EAGAIN; goto out_free_dio; } - flags |= IOMAP_NOWAIT; + iomap_flags |= IOMAP_NOWAIT; } ret = filemap_write_and_wait_range(mapping, pos, end); @@ -514,7 +514,7 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, blk_start_plug(&plug); do { - ret = iomap_apply(inode, pos, count, flags, ops, dio, + ret = iomap_apply(inode, pos, count, iomap_flags, ops, dio, iomap_dio_actor); if (ret <= 0) { /* magic error code to fall back to buffered I/O */ From 2f63296578cad1ae681152d5b2122a4595195f16 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:09 -0800 Subject: [PATCH 02/11] iomap: pass a flags argument to iomap_dio_rw Pass a set of flags to iomap_dio_rw instead of the boolean wait_for_completion argument. The IOMAP_DIO_FORCE_WAIT flag replaces the wait_for_completion, but only needs to be passed when the iocb isn't synchronous to start with to simplify the callers. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster [djwong: rework xfs_file.c so that we can push iomap changes separately] Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/btrfs/file.c | 7 +++---- fs/ext4/file.c | 5 ++--- fs/gfs2/file.c | 7 ++----- fs/iomap/direct-io.c | 11 +++++------ fs/xfs/xfs_file.c | 5 ++--- fs/zonefs/super.c | 4 ++-- include/linux/iomap.h | 10 ++++++++-- 7 files changed, 24 insertions(+), 25 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 0e41459b8de6..ddfd2e2adedf 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1949,8 +1949,8 @@ relock: goto buffered; } - dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops, - &btrfs_dio_ops, is_sync_kiocb(iocb)); + dio = __iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops, &btrfs_dio_ops, + 0); btrfs_inode_unlock(inode, ilock_flags); @@ -3622,8 +3622,7 @@ static ssize_t btrfs_direct_read(struct kiocb *iocb, struct iov_iter *to) return 0; btrfs_inode_lock(inode, BTRFS_ILOCK_SHARED); - ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, - is_sync_kiocb(iocb)); + ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dio_ops, 0); btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED); return ret; } diff --git a/fs/ext4/file.c b/fs/ext4/file.c index 349b27f0dda0..194f5d00fa32 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -74,8 +74,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to) return generic_file_read_iter(iocb, to); } - ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, - is_sync_kiocb(iocb)); + ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0); inode_unlock_shared(inode); file_accessed(iocb->ki_filp); @@ -550,7 +549,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) if (ilock_shared) iomap_ops = &ext4_iomap_overwrite_ops; ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, - is_sync_kiocb(iocb) || unaligned_io || extend); + (unaligned_io || extend) ? IOMAP_DIO_FORCE_WAIT : 0); if (ret == -ENOTBLK) ret = 0; diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index b39b339feddc..89609c299717 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -797,9 +797,7 @@ static ssize_t gfs2_file_direct_read(struct kiocb *iocb, struct iov_iter *to, if (ret) goto out_uninit; - ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, - is_sync_kiocb(iocb)); - + ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL, 0); gfs2_glock_dq(gh); out_uninit: gfs2_holder_uninit(gh); @@ -833,8 +831,7 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, struct iov_iter *from, if (offset + len > i_size_read(&ip->i_inode)) goto out; - ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, - is_sync_kiocb(iocb)); + ret = iomap_dio_rw(iocb, from, &gfs2_iomap_ops, NULL, 0); if (ret == -ENOTBLK) ret = 0; out: diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 604103ab76f9..947343730e2c 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -420,13 +420,15 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length, struct iomap_dio * __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const struct iomap_ops *ops, const struct iomap_dio_ops *dops, - bool wait_for_completion) + unsigned int dio_flags) { struct address_space *mapping = iocb->ki_filp->f_mapping; struct inode *inode = file_inode(iocb->ki_filp); size_t count = iov_iter_count(iter); loff_t pos = iocb->ki_pos; loff_t end = iocb->ki_pos + count - 1, ret = 0; + bool wait_for_completion = + is_sync_kiocb(iocb) || (dio_flags & IOMAP_DIO_FORCE_WAIT); unsigned int iomap_flags = IOMAP_DIRECT; struct blk_plug plug; struct iomap_dio *dio; @@ -434,9 +436,6 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, if (!count) return NULL; - if (WARN_ON(is_sync_kiocb(iocb) && !wait_for_completion)) - return ERR_PTR(-EIO); - dio = kmalloc(sizeof(*dio), GFP_KERNEL); if (!dio) return ERR_PTR(-ENOMEM); @@ -598,11 +597,11 @@ EXPORT_SYMBOL_GPL(__iomap_dio_rw); ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const struct iomap_ops *ops, const struct iomap_dio_ops *dops, - bool wait_for_completion) + unsigned int dio_flags) { struct iomap_dio *dio; - dio = __iomap_dio_rw(iocb, iter, ops, dops, wait_for_completion); + dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags); if (IS_ERR_OR_NULL(dio)) return PTR_ERR_OR_ZERO(dio); return iomap_dio_complete(dio); diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 5b0f93f73837..7f18dae84584 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -219,8 +219,7 @@ xfs_file_dio_aio_read( } else { xfs_ilock(ip, XFS_IOLOCK_SHARED); } - ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, - is_sync_kiocb(iocb)); + ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0); xfs_iunlock(ip, XFS_IOLOCK_SHARED); return ret; @@ -584,7 +583,7 @@ xfs_file_dio_aio_write( */ ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, &xfs_dio_write_ops, - is_sync_kiocb(iocb) || unaligned_io); + unaligned_io ? IOMAP_DIO_FORCE_WAIT : 0); out: xfs_iunlock(ip, iolock); diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c index bec47f2d074b..0e7ab0bc00ae 100644 --- a/fs/zonefs/super.c +++ b/fs/zonefs/super.c @@ -780,7 +780,7 @@ static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from) ret = zonefs_file_dio_append(iocb, from); else ret = iomap_dio_rw(iocb, from, &zonefs_iomap_ops, - &zonefs_write_dio_ops, sync); + &zonefs_write_dio_ops, 0); if (zi->i_ztype == ZONEFS_ZTYPE_SEQ && (ret > 0 || ret == -EIOCBQUEUED)) { if (ret > 0) @@ -917,7 +917,7 @@ static ssize_t zonefs_file_read_iter(struct kiocb *iocb, struct iov_iter *to) } file_accessed(iocb->ki_filp); ret = iomap_dio_rw(iocb, to, &zonefs_iomap_ops, - &zonefs_read_dio_ops, is_sync_kiocb(iocb)); + &zonefs_read_dio_ops, 0); } else { ret = generic_file_read_iter(iocb, to); if (ret == -EIO) diff --git a/include/linux/iomap.h b/include/linux/iomap.h index 5bd3cac4df9c..be4e1e1e01e8 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -256,12 +256,18 @@ struct iomap_dio_ops { struct bio *bio, loff_t file_offset); }; +/* + * Wait for the I/O to complete in iomap_dio_rw even if the kiocb is not + * synchronous. + */ +#define IOMAP_DIO_FORCE_WAIT (1 << 0) + ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const struct iomap_ops *ops, const struct iomap_dio_ops *dops, - bool wait_for_completion); + unsigned int dio_flags); struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const struct iomap_ops *ops, const struct iomap_dio_ops *dops, - bool wait_for_completion); + unsigned int dio_flags); ssize_t iomap_dio_complete(struct iomap_dio *dio); int iomap_dio_iopoll(struct kiocb *kiocb, bool spin); From 213f627104daf8589aad8ee73fcaeb603ab0af15 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:10 -0800 Subject: [PATCH 03/11] iomap: add a IOMAP_DIO_OVERWRITE_ONLY flag Add a flag to signal that only pure overwrites are allowed. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/iomap/direct-io.c | 7 +++++++ include/linux/iomap.h | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 947343730e2c..65d32364345d 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -485,6 +485,13 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, iomap_flags |= IOMAP_NOWAIT; } + if (dio_flags & IOMAP_DIO_OVERWRITE_ONLY) { + ret = -EAGAIN; + if (pos >= dio->i_size || pos + count > dio->i_size) + goto out_free_dio; + iomap_flags |= IOMAP_OVERWRITE_ONLY; + } + ret = filemap_write_and_wait_range(mapping, pos, end); if (ret) goto out_free_dio; diff --git a/include/linux/iomap.h b/include/linux/iomap.h index be4e1e1e01e8..1a86f520de56 100644 --- a/include/linux/iomap.h +++ b/include/linux/iomap.h @@ -122,6 +122,7 @@ struct iomap_page_ops { #define IOMAP_FAULT (1 << 3) /* mapping for page fault */ #define IOMAP_DIRECT (1 << 4) /* direct I/O */ #define IOMAP_NOWAIT (1 << 5) /* do not block */ +#define IOMAP_OVERWRITE_ONLY (1 << 6) /* only pure overwrites allowed */ struct iomap_ops { /* @@ -262,6 +263,13 @@ struct iomap_dio_ops { */ #define IOMAP_DIO_FORCE_WAIT (1 << 0) +/* + * Do not allocate blocks or zero partial blocks, but instead fall back to + * the caller by returning -EAGAIN. Used to optimize direct I/O writes that + * are not aligned to the file system block size. + */ +#define IOMAP_DIO_OVERWRITE_ONLY (1 << 1) + ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, const struct iomap_ops *ops, const struct iomap_dio_ops *dops, unsigned int dio_flags); From f50b8f475a2c70ae8309c16b6d4ecb305a4aa9d6 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:27 -0800 Subject: [PATCH 04/11] xfs: factor out a xfs_ilock_iocb helper Add a helper to factor out the nowait locking logical for the read/write helpers. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 55 +++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 7f18dae84584..13ca049c44fd 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -197,6 +197,23 @@ xfs_file_fsync( return error; } +static int +xfs_ilock_iocb( + struct kiocb *iocb, + unsigned int lock_mode) +{ + struct xfs_inode *ip = XFS_I(file_inode(iocb->ki_filp)); + + if (iocb->ki_flags & IOCB_NOWAIT) { + if (!xfs_ilock_nowait(ip, lock_mode)) + return -EAGAIN; + } else { + xfs_ilock(ip, lock_mode); + } + + return 0; +} + STATIC ssize_t xfs_file_dio_aio_read( struct kiocb *iocb, @@ -213,12 +230,9 @@ xfs_file_dio_aio_read( file_accessed(iocb->ki_filp); - if (iocb->ki_flags & IOCB_NOWAIT) { - if (!xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) - return -EAGAIN; - } else { - xfs_ilock(ip, XFS_IOLOCK_SHARED); - } + ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); + if (ret) + return ret; ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL, 0); xfs_iunlock(ip, XFS_IOLOCK_SHARED); @@ -239,13 +253,9 @@ xfs_file_dax_read( if (!count) return 0; /* skip atime */ - if (iocb->ki_flags & IOCB_NOWAIT) { - if (!xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) - return -EAGAIN; - } else { - xfs_ilock(ip, XFS_IOLOCK_SHARED); - } - + ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); + if (ret) + return ret; ret = dax_iomap_rw(iocb, to, &xfs_read_iomap_ops); xfs_iunlock(ip, XFS_IOLOCK_SHARED); @@ -263,12 +273,9 @@ xfs_file_buffered_aio_read( trace_xfs_file_buffered_read(ip, iov_iter_count(to), iocb->ki_pos); - if (iocb->ki_flags & IOCB_NOWAIT) { - if (!xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) - return -EAGAIN; - } else { - xfs_ilock(ip, XFS_IOLOCK_SHARED); - } + ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); + if (ret) + return ret; ret = generic_file_read_iter(iocb, to); xfs_iunlock(ip, XFS_IOLOCK_SHARED); @@ -607,13 +614,9 @@ xfs_file_dax_write( size_t count; loff_t pos; - if (iocb->ki_flags & IOCB_NOWAIT) { - if (!xfs_ilock_nowait(ip, iolock)) - return -EAGAIN; - } else { - xfs_ilock(ip, iolock); - } - + ret = xfs_ilock_iocb(iocb, iolock); + if (ret) + return ret; ret = xfs_file_aio_write_checks(iocb, from, &iolock); if (ret) goto out; From 354be7e3b2baf32e63c0599cc131d393591ba299 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:28 -0800 Subject: [PATCH 05/11] xfs: make xfs_file_aio_write_checks IOCB_NOWAIT-aware Ensure we don't block on the iolock, or waiting for I/O in xfs_file_aio_write_checks if the caller asked to avoid that. Fixes: 29a5d29ec181 ("xfs: nowait aio support") Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 13ca049c44fd..df6a9a51b2d8 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -334,7 +334,14 @@ restart: if (error <= 0) return error; - error = xfs_break_layouts(inode, iolock, BREAK_WRITE); + if (iocb->ki_flags & IOCB_NOWAIT) { + error = break_layout(inode, false); + if (error == -EWOULDBLOCK) + error = -EAGAIN; + } else { + error = xfs_break_layouts(inode, iolock, BREAK_WRITE); + } + if (error) return error; @@ -345,7 +352,11 @@ restart: if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) { xfs_iunlock(ip, *iolock); *iolock = XFS_IOLOCK_EXCL; - xfs_ilock(ip, *iolock); + error = xfs_ilock_iocb(iocb, *iolock); + if (error) { + *iolock = 0; + return error; + } goto restart; } /* @@ -367,6 +378,10 @@ restart: isize = i_size_read(inode); if (iocb->ki_pos > isize) { spin_unlock(&ip->i_flags_lock); + + if (iocb->ki_flags & IOCB_NOWAIT) + return -EAGAIN; + if (!drained_dio) { if (*iolock == XFS_IOLOCK_SHARED) { xfs_iunlock(ip, *iolock); @@ -592,7 +607,8 @@ xfs_file_dio_aio_write( &xfs_dio_write_ops, unaligned_io ? IOMAP_DIO_FORCE_WAIT : 0); out: - xfs_iunlock(ip, iolock); + if (iolock) + xfs_iunlock(ip, iolock); /* * No fallback to buffered IO after short writes for XFS, direct I/O @@ -631,7 +647,8 @@ xfs_file_dax_write( error = xfs_setfilesize(ip, pos, ret); } out: - xfs_iunlock(ip, iolock); + if (iolock) + xfs_iunlock(ip, iolock); if (error) return error; From ee1b218b09560982010e4dfffa8f9d4f05f62220 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:28 -0800 Subject: [PATCH 06/11] xfs: cleanup the read/write helper naming Drop a few pointless aio_ prefixes. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index df6a9a51b2d8..3dc5f8ca64ec 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -215,7 +215,7 @@ xfs_ilock_iocb( } STATIC ssize_t -xfs_file_dio_aio_read( +xfs_file_dio_read( struct kiocb *iocb, struct iov_iter *to) { @@ -264,7 +264,7 @@ xfs_file_dax_read( } STATIC ssize_t -xfs_file_buffered_aio_read( +xfs_file_buffered_read( struct kiocb *iocb, struct iov_iter *to) { @@ -299,9 +299,9 @@ xfs_file_read_iter( if (IS_DAX(inode)) ret = xfs_file_dax_read(iocb, to); else if (iocb->ki_flags & IOCB_DIRECT) - ret = xfs_file_dio_aio_read(iocb, to); + ret = xfs_file_dio_read(iocb, to); else - ret = xfs_file_buffered_aio_read(iocb, to); + ret = xfs_file_buffered_read(iocb, to); if (ret > 0) XFS_STATS_ADD(mp, xs_read_bytes, ret); @@ -316,7 +316,7 @@ xfs_file_read_iter( * if called for a direct write beyond i_size. */ STATIC ssize_t -xfs_file_aio_write_checks( +xfs_file_write_checks( struct kiocb *iocb, struct iov_iter *from, int *iolock) @@ -501,7 +501,7 @@ static const struct iomap_dio_ops xfs_dio_write_ops = { }; /* - * xfs_file_dio_aio_write - handle direct IO writes + * xfs_file_dio_write - handle direct IO writes * * Lock the inode appropriately to prepare for and issue a direct IO write. * By separating it from the buffered write path we remove all the tricky to @@ -526,7 +526,7 @@ static const struct iomap_dio_ops xfs_dio_write_ops = { * negative return values. */ STATIC ssize_t -xfs_file_dio_aio_write( +xfs_file_dio_write( struct kiocb *iocb, struct iov_iter *from) { @@ -548,7 +548,7 @@ xfs_file_dio_aio_write( /* * Don't take the exclusive iolock here unless the I/O is unaligned to * the file system block size. We don't need to consider the EOF - * extension case here because xfs_file_aio_write_checks() will relock + * extension case here because xfs_file_write_checks() will relock * the inode as necessary for EOF zeroing cases and fill out the new * inode size as appropriate. */ @@ -579,7 +579,7 @@ xfs_file_dio_aio_write( xfs_ilock(ip, iolock); } - ret = xfs_file_aio_write_checks(iocb, from, &iolock); + ret = xfs_file_write_checks(iocb, from, &iolock); if (ret) goto out; count = iov_iter_count(from); @@ -589,7 +589,7 @@ xfs_file_dio_aio_write( * in-flight at the same time or we risk data corruption. Wait for all * other IO to drain before we submit. If the IO is aligned, demote the * iolock if we had to take the exclusive lock in - * xfs_file_aio_write_checks() for other reasons. + * xfs_file_write_checks() for other reasons. */ if (unaligned_io) { inode_dio_wait(inode); @@ -633,7 +633,7 @@ xfs_file_dax_write( ret = xfs_ilock_iocb(iocb, iolock); if (ret) return ret; - ret = xfs_file_aio_write_checks(iocb, from, &iolock); + ret = xfs_file_write_checks(iocb, from, &iolock); if (ret) goto out; @@ -662,7 +662,7 @@ out: } STATIC ssize_t -xfs_file_buffered_aio_write( +xfs_file_buffered_write( struct kiocb *iocb, struct iov_iter *from) { @@ -681,7 +681,7 @@ write_retry: iolock = XFS_IOLOCK_EXCL; xfs_ilock(ip, iolock); - ret = xfs_file_aio_write_checks(iocb, from, &iolock); + ret = xfs_file_write_checks(iocb, from, &iolock); if (ret) goto out; @@ -768,12 +768,12 @@ xfs_file_write_iter( * CoW. In all other directio scenarios we do not * allow an operation to fall back to buffered mode. */ - ret = xfs_file_dio_aio_write(iocb, from); + ret = xfs_file_dio_write(iocb, from); if (ret != -ENOTBLK) return ret; } - return xfs_file_buffered_aio_write(iocb, from); + return xfs_file_buffered_write(iocb, from); } static void From 670654b004b0bf7a0bc749f4f555fdefd5c89dcb Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:29 -0800 Subject: [PATCH 07/11] xfs: remove the buffered I/O fallback assert The iomap code has been designed from the start not to do magic fallback, so remove the assert in preparation for further code cleanups. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 3dc5f8ca64ec..b38002ddfa6c 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -609,12 +609,6 @@ xfs_file_dio_write( out: if (iolock) xfs_iunlock(ip, iolock); - - /* - * No fallback to buffered IO after short writes for XFS, direct I/O - * will either complete fully or return an error. - */ - ASSERT(ret < 0 || ret == count); return ret; } From 3e40b13c3b57108a118de639d4af04805ac6873f Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:29 -0800 Subject: [PATCH 08/11] xfs: simplify the read/write tracepoints Pass the iocb and iov_iter to the tracepoints and leave decoding of actual arguments to the code only run when tracing is enabled. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 20 ++++++++------------ fs/xfs/xfs_trace.h | 18 +++++++++--------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index b38002ddfa6c..27b81925f92a 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -220,12 +220,11 @@ xfs_file_dio_read( struct iov_iter *to) { struct xfs_inode *ip = XFS_I(file_inode(iocb->ki_filp)); - size_t count = iov_iter_count(to); ssize_t ret; - trace_xfs_file_direct_read(ip, count, iocb->ki_pos); + trace_xfs_file_direct_read(iocb, to); - if (!count) + if (!iov_iter_count(to)) return 0; /* skip atime */ file_accessed(iocb->ki_filp); @@ -245,12 +244,11 @@ xfs_file_dax_read( struct iov_iter *to) { struct xfs_inode *ip = XFS_I(iocb->ki_filp->f_mapping->host); - size_t count = iov_iter_count(to); ssize_t ret = 0; - trace_xfs_file_dax_read(ip, count, iocb->ki_pos); + trace_xfs_file_dax_read(iocb, to); - if (!count) + if (!iov_iter_count(to)) return 0; /* skip atime */ ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); @@ -271,7 +269,7 @@ xfs_file_buffered_read( struct xfs_inode *ip = XFS_I(file_inode(iocb->ki_filp)); ssize_t ret; - trace_xfs_file_buffered_read(ip, iov_iter_count(to), iocb->ki_pos); + trace_xfs_file_buffered_read(iocb, to); ret = xfs_ilock_iocb(iocb, XFS_IOLOCK_SHARED); if (ret) @@ -598,7 +596,7 @@ xfs_file_dio_write( iolock = XFS_IOLOCK_SHARED; } - trace_xfs_file_direct_write(ip, count, iocb->ki_pos); + trace_xfs_file_direct_write(iocb, from); /* * If unaligned, this is the only IO in-flight. Wait on it before we * release the iolock to prevent subsequent overlapping IO. @@ -621,7 +619,6 @@ xfs_file_dax_write( struct xfs_inode *ip = XFS_I(inode); int iolock = XFS_IOLOCK_EXCL; ssize_t ret, error = 0; - size_t count; loff_t pos; ret = xfs_ilock_iocb(iocb, iolock); @@ -632,9 +629,8 @@ xfs_file_dax_write( goto out; pos = iocb->ki_pos; - count = iov_iter_count(from); - trace_xfs_file_dax_write(ip, count, pos); + trace_xfs_file_dax_write(iocb, from); ret = dax_iomap_rw(iocb, from, &xfs_direct_write_iomap_ops); if (ret > 0 && iocb->ki_pos > i_size_read(inode)) { i_size_write(inode, iocb->ki_pos); @@ -682,7 +678,7 @@ write_retry: /* We can write back this queue in page reclaim */ current->backing_dev_info = inode_to_bdi(inode); - trace_xfs_file_buffered_write(ip, iov_iter_count(from), iocb->ki_pos); + trace_xfs_file_buffered_write(iocb, from); ret = iomap_file_buffered_write(iocb, from, &xfs_buffered_write_iomap_ops); if (likely(ret >= 0)) diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 5a263ae3d4f0..a6d04d860a56 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -1287,8 +1287,8 @@ TRACE_EVENT(xfs_log_assign_tail_lsn, ) DECLARE_EVENT_CLASS(xfs_file_class, - TP_PROTO(struct xfs_inode *ip, size_t count, loff_t offset), - TP_ARGS(ip, count, offset), + TP_PROTO(struct kiocb *iocb, struct iov_iter *iter), + TP_ARGS(iocb, iter), TP_STRUCT__entry( __field(dev_t, dev) __field(xfs_ino_t, ino) @@ -1297,11 +1297,11 @@ DECLARE_EVENT_CLASS(xfs_file_class, __field(size_t, count) ), TP_fast_assign( - __entry->dev = VFS_I(ip)->i_sb->s_dev; - __entry->ino = ip->i_ino; - __entry->size = ip->i_d.di_size; - __entry->offset = offset; - __entry->count = count; + __entry->dev = file_inode(iocb->ki_filp)->i_sb->s_dev; + __entry->ino = XFS_I(file_inode(iocb->ki_filp))->i_ino; + __entry->size = XFS_I(file_inode(iocb->ki_filp))->i_d.di_size; + __entry->offset = iocb->ki_pos; + __entry->count = iov_iter_count(iter); ), TP_printk("dev %d:%d ino 0x%llx size 0x%llx offset 0x%llx count 0x%zx", MAJOR(__entry->dev), MINOR(__entry->dev), @@ -1313,8 +1313,8 @@ DECLARE_EVENT_CLASS(xfs_file_class, #define DEFINE_RW_EVENT(name) \ DEFINE_EVENT(xfs_file_class, name, \ - TP_PROTO(struct xfs_inode *ip, size_t count, loff_t offset), \ - TP_ARGS(ip, count, offset)) + TP_PROTO(struct kiocb *iocb, struct iov_iter *iter), \ + TP_ARGS(iocb, iter)) DEFINE_RW_EVENT(xfs_file_buffered_read); DEFINE_RW_EVENT(xfs_file_direct_read); DEFINE_RW_EVENT(xfs_file_dax_read); From 896f72d067a5e9a90d2178b311bb89a18c7b5cfb Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sat, 23 Jan 2021 10:06:30 -0800 Subject: [PATCH 09/11] xfs: improve the reflink_bounce_dio_write tracepoint Use a more suitable event class. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 2 +- fs/xfs/xfs_trace.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 27b81925f92a..94bc2ea83734 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -559,7 +559,7 @@ xfs_file_dio_write( * files yet, as we can't unshare a partial block. */ if (xfs_is_cow_inode(ip)) { - trace_xfs_reflink_bounce_dio_write(ip, iocb->ki_pos, count); + trace_xfs_reflink_bounce_dio_write(iocb, from); return -ENOTBLK; } iolock = XFS_IOLOCK_EXCL; diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index a6d04d860a56..0cfd65cd67c1 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -1321,6 +1321,8 @@ DEFINE_RW_EVENT(xfs_file_dax_read); DEFINE_RW_EVENT(xfs_file_buffered_write); DEFINE_RW_EVENT(xfs_file_direct_write); DEFINE_RW_EVENT(xfs_file_dax_write); +DEFINE_RW_EVENT(xfs_reflink_bounce_dio_write); + DECLARE_EVENT_CLASS(xfs_imap_class, TP_PROTO(struct xfs_inode *ip, xfs_off_t offset, ssize_t count, @@ -3294,8 +3296,6 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_found); DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_enospc); DEFINE_INODE_IREC_EVENT(xfs_reflink_convert_cow); -DEFINE_SIMPLE_IO_EVENT(xfs_reflink_bounce_dio_write); - DEFINE_SIMPLE_IO_EVENT(xfs_reflink_cancel_cow_range); DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow); DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap); From caa89dbc4303d9bc7caa6c1124a84aa3efa47e00 Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Sat, 23 Jan 2021 10:06:30 -0800 Subject: [PATCH 10/11] xfs: split the unaligned DIO write code out The unaligned DIO write path is more convolted than the normal path, and we are about to make it more complex. Keep the block aligned fast path dio write code trim and simple by splitting out the unaligned DIO code from it. Signed-off-by: Dave Chinner [hch: rebased, fixed a few minor nits] Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 170 +++++++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 94bc2ea83734..c60ff7b5dd82 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -499,117 +499,117 @@ static const struct iomap_dio_ops xfs_dio_write_ops = { }; /* - * xfs_file_dio_write - handle direct IO writes - * - * Lock the inode appropriately to prepare for and issue a direct IO write. - * By separating it from the buffered write path we remove all the tricky to - * follow locking changes and looping. - * - * If there are cached pages or we're extending the file, we need IOLOCK_EXCL - * until we're sure the bytes at the new EOF have been zeroed and/or the cached - * pages are flushed out. - * - * In most cases the direct IO writes will be done holding IOLOCK_SHARED - * allowing them to be done in parallel with reads and other direct IO writes. - * However, if the IO is not aligned to filesystem blocks, the direct IO layer - * needs to do sub-block zeroing and that requires serialisation against other - * direct IOs to the same block. In this case we need to serialise the - * submission of the unaligned IOs so that we don't get racing block zeroing in - * the dio layer. To avoid the problem with aio, we also need to wait for - * outstanding IOs to complete so that unwritten extent conversion is completed - * before we try to map the overlapping block. This is currently implemented by - * hitting it with a big hammer (i.e. inode_dio_wait()). - * - * Returns with locks held indicated by @iolock and errors indicated by - * negative return values. + * Handle block aligned direct I/O writes */ -STATIC ssize_t -xfs_file_dio_write( +static noinline ssize_t +xfs_file_dio_write_aligned( + struct xfs_inode *ip, struct kiocb *iocb, struct iov_iter *from) { - struct file *file = iocb->ki_filp; - struct address_space *mapping = file->f_mapping; - struct inode *inode = mapping->host; - struct xfs_inode *ip = XFS_I(inode); - struct xfs_mount *mp = ip->i_mount; - ssize_t ret = 0; - int unaligned_io = 0; - int iolock; - size_t count = iov_iter_count(from); - struct xfs_buftarg *target = xfs_inode_buftarg(ip); + int iolock = XFS_IOLOCK_SHARED; + ssize_t ret; - /* DIO must be aligned to device logical sector size */ - if ((iocb->ki_pos | count) & target->bt_logical_sectormask) - return -EINVAL; + ret = xfs_ilock_iocb(iocb, iolock); + if (ret) + return ret; + ret = xfs_file_write_checks(iocb, from, &iolock); + if (ret) + goto out_unlock; /* - * Don't take the exclusive iolock here unless the I/O is unaligned to - * the file system block size. We don't need to consider the EOF - * extension case here because xfs_file_write_checks() will relock - * the inode as necessary for EOF zeroing cases and fill out the new - * inode size as appropriate. + * We don't need to hold the IOLOCK exclusively across the IO, so demote + * the iolock back to shared if we had to take the exclusive lock in + * xfs_file_write_checks() for other reasons. */ - if ((iocb->ki_pos & mp->m_blockmask) || - ((iocb->ki_pos + count) & mp->m_blockmask)) { - unaligned_io = 1; - - /* - * We can't properly handle unaligned direct I/O to reflink - * files yet, as we can't unshare a partial block. - */ - if (xfs_is_cow_inode(ip)) { - trace_xfs_reflink_bounce_dio_write(iocb, from); - return -ENOTBLK; - } - iolock = XFS_IOLOCK_EXCL; - } else { + if (iolock == XFS_IOLOCK_EXCL) { + xfs_ilock_demote(ip, XFS_IOLOCK_EXCL); iolock = XFS_IOLOCK_SHARED; } + trace_xfs_file_direct_write(iocb, from); + ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, + &xfs_dio_write_ops, 0); +out_unlock: + if (iolock) + xfs_iunlock(ip, iolock); + return ret; +} - if (iocb->ki_flags & IOCB_NOWAIT) { - /* unaligned dio always waits, bail */ - if (unaligned_io) - return -EAGAIN; - if (!xfs_ilock_nowait(ip, iolock)) - return -EAGAIN; - } else { - xfs_ilock(ip, iolock); +/* + * Handle block unaligned direct I/O writes + * + * In most cases direct I/O writes will be done holding IOLOCK_SHARED, allowing + * them to be done in parallel with reads and other direct I/O writes. However, + * if the I/O is not aligned to filesystem blocks, the direct I/O layer may need + * to do sub-block zeroing and that requires serialisation against other direct + * I/O to the same block. In this case we need to serialise the submission of + * the unaligned I/O so that we don't get racing block zeroing in the dio layer. + * + * This means that unaligned dio writes always block. There is no "nowait" fast + * path in this code - if IOCB_NOWAIT is set we simply return -EAGAIN up front + * and we don't have to worry about that anymore. + */ +static noinline ssize_t +xfs_file_dio_write_unaligned( + struct xfs_inode *ip, + struct kiocb *iocb, + struct iov_iter *from) +{ + int iolock = XFS_IOLOCK_EXCL; + ssize_t ret; + + /* unaligned dio always waits, bail */ + if (iocb->ki_flags & IOCB_NOWAIT) + return -EAGAIN; + xfs_ilock(ip, iolock); + + /* + * We can't properly handle unaligned direct I/O to reflink files yet, + * as we can't unshare a partial block. + */ + if (xfs_is_cow_inode(ip)) { + trace_xfs_reflink_bounce_dio_write(iocb, from); + ret = -ENOTBLK; + goto out_unlock; } ret = xfs_file_write_checks(iocb, from, &iolock); if (ret) - goto out; - count = iov_iter_count(from); + goto out_unlock; /* - * If we are doing unaligned IO, we can't allow any other overlapping IO - * in-flight at the same time or we risk data corruption. Wait for all - * other IO to drain before we submit. If the IO is aligned, demote the - * iolock if we had to take the exclusive lock in - * xfs_file_write_checks() for other reasons. + * If we are doing unaligned I/O, this must be the only I/O in-flight. + * Otherwise we risk data corruption due to unwritten extent conversions + * from the AIO end_io handler. Wait for all other I/O to drain first. */ - if (unaligned_io) { - inode_dio_wait(inode); - } else if (iolock == XFS_IOLOCK_EXCL) { - xfs_ilock_demote(ip, XFS_IOLOCK_EXCL); - iolock = XFS_IOLOCK_SHARED; - } + inode_dio_wait(VFS_I(ip)); trace_xfs_file_direct_write(iocb, from); - /* - * If unaligned, this is the only IO in-flight. Wait on it before we - * release the iolock to prevent subsequent overlapping IO. - */ ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, - &xfs_dio_write_ops, - unaligned_io ? IOMAP_DIO_FORCE_WAIT : 0); -out: + &xfs_dio_write_ops, IOMAP_DIO_FORCE_WAIT); +out_unlock: if (iolock) xfs_iunlock(ip, iolock); return ret; } +static ssize_t +xfs_file_dio_write( + struct kiocb *iocb, + struct iov_iter *from) +{ + struct xfs_inode *ip = XFS_I(file_inode(iocb->ki_filp)); + struct xfs_buftarg *target = xfs_inode_buftarg(ip); + size_t count = iov_iter_count(from); + + /* direct I/O must be aligned to device logical sector size */ + if ((iocb->ki_pos | count) & target->bt_logical_sectormask) + return -EINVAL; + if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask) + return xfs_file_dio_write_unaligned(ip, iocb, from); + return xfs_file_dio_write_aligned(ip, iocb, from); +} + static noinline ssize_t xfs_file_dax_write( struct kiocb *iocb, From ed1128c2d0c87e5ff49c40f5529f06bc35f4251b Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Sat, 23 Jan 2021 10:06:31 -0800 Subject: [PATCH 11/11] xfs: reduce exclusive locking on unaligned dio Attempt shared locking for unaligned DIO, but only if the the underlying extent is already allocated and in written state. On failure, retry with the existing exclusive locking. Test case is fio randrw of 512 byte IOs using AIO and an iodepth of 32 IOs. Vanilla: READ: bw=4560KiB/s (4670kB/s), 4560KiB/s-4560KiB/s (4670kB/s-4670kB/s), io=134MiB (140MB), run=30001-30001msec WRITE: bw=4567KiB/s (4676kB/s), 4567KiB/s-4567KiB/s (4676kB/s-4676kB/s), io=134MiB (140MB), run=30001-30001msec Patched: READ: bw=37.6MiB/s (39.4MB/s), 37.6MiB/s-37.6MiB/s (39.4MB/s-39.4MB/s), io=1127MiB (1182MB), run=30002-30002msec WRITE: bw=37.6MiB/s (39.4MB/s), 37.6MiB/s-37.6MiB/s (39.4MB/s-39.4MB/s), io=1128MiB (1183MB), run=30002-30002msec That's an improvement from ~18k IOPS to a ~150k IOPS, which is about the IOPS limit of the VM block device setup I'm testing on. 4kB block IO comparison: READ: bw=296MiB/s (310MB/s), 296MiB/s-296MiB/s (310MB/s-310MB/s), io=8868MiB (9299MB), run=30002-30002msec WRITE: bw=296MiB/s (310MB/s), 296MiB/s-296MiB/s (310MB/s-310MB/s), io=8878MiB (9309MB), run=30002-30002msec Which is ~150k IOPS, same as what the test gets for sub-block AIO+DIO writes with this patch. Signed-off-by: Dave Chinner [hch: rebased, split unaligned from nowait] Signed-off-by: Christoph Hellwig Reviewed-by: Brian Foster Reviewed-by: Darrick J. Wong Signed-off-by: Darrick J. Wong --- fs/xfs/xfs_file.c | 58 +++++++++++++++++++++++++++++++++++----------- fs/xfs/xfs_iomap.c | 29 ++++++++++++++++------- 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index c60ff7b5dd82..39695b59dfcc 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -544,10 +544,13 @@ out_unlock: * to do sub-block zeroing and that requires serialisation against other direct * I/O to the same block. In this case we need to serialise the submission of * the unaligned I/O so that we don't get racing block zeroing in the dio layer. + * In the case where sub-block zeroing is not required, we can do concurrent + * sub-block dios to the same block successfully. * - * This means that unaligned dio writes always block. There is no "nowait" fast - * path in this code - if IOCB_NOWAIT is set we simply return -EAGAIN up front - * and we don't have to worry about that anymore. + * Optimistically submit the I/O using the shared lock first, but use the + * IOMAP_DIO_OVERWRITE_ONLY flag to tell the lower layers to return -EAGAIN + * if block allocation or partial block zeroing would be required. In that case + * we try again with the exclusive lock. */ static noinline ssize_t xfs_file_dio_write_unaligned( @@ -555,13 +558,28 @@ xfs_file_dio_write_unaligned( struct kiocb *iocb, struct iov_iter *from) { - int iolock = XFS_IOLOCK_EXCL; + size_t isize = i_size_read(VFS_I(ip)); + size_t count = iov_iter_count(from); + int iolock = XFS_IOLOCK_SHARED; + unsigned int flags = IOMAP_DIO_OVERWRITE_ONLY; ssize_t ret; - /* unaligned dio always waits, bail */ - if (iocb->ki_flags & IOCB_NOWAIT) - return -EAGAIN; - xfs_ilock(ip, iolock); + /* + * Extending writes need exclusivity because of the sub-block zeroing + * that the DIO code always does for partial tail blocks beyond EOF, so + * don't even bother trying the fast path in this case. + */ + if (iocb->ki_pos > isize || iocb->ki_pos + count >= isize) { +retry_exclusive: + if (iocb->ki_flags & IOCB_NOWAIT) + return -EAGAIN; + iolock = XFS_IOLOCK_EXCL; + flags = IOMAP_DIO_FORCE_WAIT; + } + + ret = xfs_ilock_iocb(iocb, iolock); + if (ret) + return ret; /* * We can't properly handle unaligned direct I/O to reflink files yet, @@ -578,15 +596,29 @@ xfs_file_dio_write_unaligned( goto out_unlock; /* - * If we are doing unaligned I/O, this must be the only I/O in-flight. - * Otherwise we risk data corruption due to unwritten extent conversions - * from the AIO end_io handler. Wait for all other I/O to drain first. + * If we are doing exclusive unaligned I/O, this must be the only I/O + * in-flight. Otherwise we risk data corruption due to unwritten extent + * conversions from the AIO end_io handler. Wait for all other I/O to + * drain first. */ - inode_dio_wait(VFS_I(ip)); + if (flags & IOMAP_DIO_FORCE_WAIT) + inode_dio_wait(VFS_I(ip)); trace_xfs_file_direct_write(iocb, from); ret = iomap_dio_rw(iocb, from, &xfs_direct_write_iomap_ops, - &xfs_dio_write_ops, IOMAP_DIO_FORCE_WAIT); + &xfs_dio_write_ops, flags); + + /* + * Retry unaligned I/O with exclusive blocking semantics if the DIO + * layer rejected it for mapping or locking reasons. If we are doing + * nonblocking user I/O, propagate the error. + */ + if (ret == -EAGAIN && !(iocb->ki_flags & IOCB_NOWAIT)) { + ASSERT(flags & IOMAP_DIO_OVERWRITE_ONLY); + xfs_iunlock(ip, iolock); + goto retry_exclusive; + } + out_unlock: if (iolock) xfs_iunlock(ip, iolock); diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c index 7b9ff824e82d..ef76f775fabf 100644 --- a/fs/xfs/xfs_iomap.c +++ b/fs/xfs/xfs_iomap.c @@ -784,15 +784,28 @@ xfs_direct_write_iomap_begin( goto allocate_blocks; /* - * NOWAIT IO needs to span the entire requested IO with a single map so - * that we avoid partial IO failures due to the rest of the IO range not - * covered by this map triggering an EAGAIN condition when it is - * subsequently mapped and aborting the IO. + * NOWAIT and OVERWRITE I/O needs to span the entire requested I/O with + * a single map so that we avoid partial IO failures due to the rest of + * the I/O range not covered by this map triggering an EAGAIN condition + * when it is subsequently mapped and aborting the I/O. */ - if ((flags & IOMAP_NOWAIT) && - !imap_spans_range(&imap, offset_fsb, end_fsb)) { + if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY)) { error = -EAGAIN; - goto out_unlock; + if (!imap_spans_range(&imap, offset_fsb, end_fsb)) + goto out_unlock; + } + + /* + * For overwrite only I/O, we cannot convert unwritten extents without + * requiring sub-block zeroing. This can only be done under an + * exclusive IOLOCK, hence return -EAGAIN if this is not a written + * extent to tell the caller to try again. + */ + if (flags & IOMAP_OVERWRITE_ONLY) { + error = -EAGAIN; + if (imap.br_state != XFS_EXT_NORM && + ((offset | length) & mp->m_blockmask)) + goto out_unlock; } xfs_iunlock(ip, lockmode); @@ -801,7 +814,7 @@ xfs_direct_write_iomap_begin( allocate_blocks: error = -EAGAIN; - if (flags & IOMAP_NOWAIT) + if (flags & (IOMAP_NOWAIT | IOMAP_OVERWRITE_ONLY)) goto out_unlock; /*