decouple simple direct I/O reads from iomap_dio_rw

decouple simple direct I/O reads from iomap_dio_rw

[Christoph Hellwig]

Hi all,

this series reworks how the iomap simple dio read fast path is called and
decouples it from the generic iomap_dio_rw and the iomap iter.  This is
to make the iomap_iter_next transition easier and cleaner.  It slots in
after the first patch from Joanne's

[PATCH v3 00/20] iomap: convert to in-iter iomap_next() model

series, and the first patch from that is included as the first patch
here unmodified as reference.  If the approach looks good to everyone,
I'd suggest to include it in the next spin of that series.

[PATCH 1/3] iomap: split iomap_iter() logic into iomap_iter_next()

[Christoph Hellwig]

From: Joanne Koong <joannelkoong@gmail.com>

In preparation for changing iomap to use an in-iter (->iomap_next())
model, move the iomap_iter() logic out into the new iomap_iter_next()
helper function.

iomap_iter_next() is added as an inlined helper so it can be called
directly by ->iomap_next() implementations where the begin()/end()
callbacks can be direct calls.

The DEFINE_IOMAP_ITER_NEXT() and DEFINE_IOMAP_ITER_NEXT_END() macros are
also provided to generate the boilerplate ->iomap_next() wrapper
functions that simply forward to iomap_iter_next() with the appropriate
begin/end callbacks. DEFINE_IOMAP_ITER_NEXT() is for the common case
where there is no end() callback. DEFINE_IOMAP_ITER_NEXT_END() is for
the case where there is an explicit end() callback.

No functional change intended. The only code-level difference is that on
the iomap_end() error path (ret < 0 && !advanced), the old code returned
with iter.status left as the caller's last value whereas the new code
zeroes it, but this is not observable in practice as there are no in-tree
callers that read iter.status after the iteration loop.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/iomap/iter.c       | 123 +++++++++++++++++++++---------------------
 include/linux/iomap.h | 102 +++++++++++++++++++++++++++++------
 2 files changed, 147 insertions(+), 78 deletions(-)

diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
index e4a29829591a..66ccb87441ab 100644
--- a/fs/iomap/iter.c
+++ b/fs/iomap/iter.c
@@ -6,15 +6,6 @@
 #include <linux/iomap.h>
 #include "trace.h"
 
-static inline void iomap_iter_clean_fbatch(struct iomap_iter *iter)
-{
-	if (iter->iomap.flags & IOMAP_F_FOLIO_BATCH) {
-		folio_batch_release(iter->fbatch);
-		folio_batch_reinit(iter->fbatch);
-		iter->iomap.flags &= ~IOMAP_F_FOLIO_BATCH;
-	}
-}
-
 /* Advance the current iterator position and decrement the remaining length */
 int iomap_iter_advance(struct iomap_iter *iter, u64 count)
 {
@@ -40,51 +31,28 @@ static inline void iomap_iter_done(struct iomap_iter *iter)
 }
 
 /**
- * iomap_iter - iterate over a ranges in a file
- * @iter: iteration structue
- * @ops: iomap ops provided by the file system
+ * iomap_iter_continue - decide whether iteration should continue
+ * @iter: iteration structure
+ * @iomap: the mapping that was just processed
+ * @srcmap: the source mapping that was just processed
  *
- * Iterate over filesystem-provided space mappings for the provided file range.
+ * Helper normally called via iomap_iter_next(). Called after the previous
+ * mapping has been finished to determine whether there is more of the file
+ * range left to process.
  *
- * This function handles cleanup of resources acquired for iteration when the
- * filesystem indicates there are no more space mappings, which means that this
- * function must be called in a loop that continues as long it returns a
- * positive value.  If 0 or a negative value is returned, the caller must not
- * return to the loop body.  Within a loop body, there are two ways to break out
- * of the loop body:  leave @iter.status unchanged, or set it to a negative
- * errno.
+ * Returns 1 if there is more work to do, in which case @iomap and @srcmap are
+ * cleared so the caller can produce the next mapping; zero if the range is
+ * fully consumed; or a negative errno on error. Any folio batch attached to
+ * the mapping is released before returning.
  */
-int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
+int iomap_iter_continue(const struct iomap_iter *iter, struct iomap *iomap,
+		struct iomap *srcmap, int ret)
 {
-	bool stale = iter->iomap.flags & IOMAP_F_STALE;
-	ssize_t advanced;
-	u64 olen;
-	int ret;
-
-	trace_iomap_iter(iter, ops, _RET_IP_);
-
-	if (!iter->iomap.length)
-		goto begin;
-
-	/*
-	 * Calculate how far the iter was advanced and the original length bytes
-	 * for ->iomap_end().
-	 */
-	advanced = iter->pos - iter->iter_start_pos;
-	olen = iter->len + advanced;
-
-	if (ops->iomap_end) {
-		ret = ops->iomap_end(iter->inode, iter->iter_start_pos,
-				iomap_length_trim(iter, iter->iter_start_pos,
-						  olen),
-				advanced, iter->flags, &iter->iomap);
-		if (ret < 0 && !advanced)
-			return ret;
-	}
+	const bool stale = iomap->flags & IOMAP_F_STALE;
+	const ssize_t advanced = iter->pos - iter->iter_start_pos;
 
-	/* detect old return semantics where this would advance */
-	if (WARN_ON_ONCE(iter->status > 0))
-		iter->status = -EIO;
+	if (ret < 0 && !advanced)
+		return ret;
 
 	/*
 	 * Use iter->len to determine whether to continue onto the next mapping.
@@ -92,25 +60,58 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
 	 * advanced at all (i.e. no work was done for some reason) unless the
 	 * mapping has been marked stale and needs to be reprocessed.
 	 */
-	if (iter->status < 0)
+	if (WARN_ON_ONCE(iter->status > 0))
+		/* detect old return semantics where this would advance */
+		ret = -EIO;
+	else if (iter->status < 0)
 		ret = iter->status;
 	else if (iter->len == 0 || (!advanced && !stale))
 		ret = 0;
 	else
 		ret = 1;
-	iomap_iter_clean_fbatch(iter);
-	iter->status = 0;
+
+	if (iomap->flags & IOMAP_F_FOLIO_BATCH) {
+		folio_batch_release(iter->fbatch);
+		folio_batch_reinit(iter->fbatch);
+		iomap->flags &= ~IOMAP_F_FOLIO_BATCH;
+	}
+
 	if (ret <= 0)
 		return ret;
 
-	memset(&iter->iomap, 0, sizeof(iter->iomap));
-	memset(&iter->srcmap, 0, sizeof(iter->srcmap));
+	memset(iomap, 0, sizeof(*iomap));
+	memset(srcmap, 0, sizeof(*srcmap));
 
-begin:
-	ret = ops->iomap_begin(iter->inode, iter->pos, iter->len, iter->flags,
-			       &iter->iomap, &iter->srcmap);
-	if (ret < 0)
-		return ret;
-	iomap_iter_done(iter);
-	return 1;
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iomap_iter_continue);
+
+/**
+ * iomap_iter - iterate over ranges in a file
+ * @iter: iteration structure
+ * @ops: iomap ops provided by the filesystem
+ *
+ * Iterate over filesystem-provided space mappings for the provided file range.
+ *
+ * This function handles cleanup of resources acquired for iteration when the
+ * filesystem indicates there are no more space mappings, which means that this
+ * function must be called in a loop that continues as long it returns a
+ * positive value.  If 0 or a negative value is returned, the caller must not
+ * return to the loop body.  Within a loop body, there are two ways to break out
+ * of the loop body:  leave @iter.status unchanged, or set it to a negative
+ * errno.
+ */
+int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
+{
+	int ret;
+
+	trace_iomap_iter(iter, ops, _RET_IP_);
+
+	ret = iomap_iter_next(iter, &iter->iomap, &iter->srcmap,
+			ops->iomap_begin, ops->iomap_end);
+	iter->status = 0;
+	if (ret > 0)
+		iomap_iter_done(iter);
+
+	return ret;
 }
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 3582ed1fe236..3df851d0f76b 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -212,24 +212,27 @@ struct iomap_write_ops {
 #define IOMAP_ATOMIC		(1 << 9) /* torn-write protection */
 #define IOMAP_DONTCACHE		(1 << 10)
 
-struct iomap_ops {
-	/*
-	 * Return the existing mapping at pos, or reserve space starting at
-	 * pos for up to length, as long as we can do it as a single mapping.
-	 * The actual length is returned in iomap->length.
-	 */
-	int (*iomap_begin)(struct inode *inode, loff_t pos, loff_t length,
-			unsigned flags, struct iomap *iomap,
-			struct iomap *srcmap);
+/*
+ * Return the existing mapping at pos, or reserve space starting at pos for up
+ * to length, as long as we can do it as a single mapping.
+ * The actual length is returned in iomap->length.
+ */
+typedef int (*iomap_iter_begin_fn)(struct inode *inode, loff_t pos,
+		loff_t length, unsigned flags, struct iomap *iomap,
+		struct iomap *srcmap);
 
-	/*
-	 * Commit and/or unreserve space previous allocated using iomap_begin.
-	 * Written indicates the length of the successful write operation which
-	 * needs to be commited, while the rest needs to be unreserved.
-	 * Written might be zero if no data was written.
-	 */
-	int (*iomap_end)(struct inode *inode, loff_t pos, loff_t length,
-			ssize_t written, unsigned flags, struct iomap *iomap);
+/*
+ * Commit and/or unreserve space previously allocated by iomap_iter_begin_fn.
+ * Written indicates the length of the successful write operation which needs
+ * to be committed, while the rest needs to be unreserved.
+ * Written might be zero if no data was written.
+ */
+typedef int (*iomap_iter_end_fn)(struct inode *inode, loff_t pos, loff_t length,
+		ssize_t written, unsigned flags, struct iomap *iomap);
+
+struct iomap_ops {
+	iomap_iter_begin_fn iomap_begin;
+	iomap_iter_end_fn iomap_end;
 };
 
 /**
@@ -317,6 +320,71 @@ static inline const struct iomap *iomap_iter_srcmap(const struct iomap_iter *i)
 	return &i->iomap;
 }
 
+int iomap_iter_continue(const struct iomap_iter *iter, struct iomap *iomap,
+		struct iomap *srcmap, int ret);
+
+/**
+ * iomap_iter_next - finish the previous mapping and produce the next one
+ * @iter: iteration structure
+ * @iomap: mapping to finish and then repopulate
+ * @srcmap: source mapping to finish and then repopulate
+ * @begin: callback that produces a mapping for the current position
+ * @end: optional callback that finishes the previous mapping, or NULL
+ *
+ * Inline helper that implements the common body of an ->iomap_next()
+ * callback: it finishes the previous mapping via @end (if present), decides
+ * via iomap_iter_continue() whether to keep going, and obtains the next
+ * mapping via @begin.
+ *
+ * This helper is marked __always_inline so that when a caller passes
+ * compile-time-constant @begin and @end callbacks, the compiler can call them
+ * directly, avoiding the indirect-call overhead.
+ *
+ * Returns 1 to continue iterating, 0 once the range is fully consumed, or a
+ * negative errno on error.
+ */
+static __always_inline int iomap_iter_next(const struct iomap_iter *iter,
+		struct iomap *iomap, struct iomap *srcmap,
+		iomap_iter_begin_fn begin, iomap_iter_end_fn end)
+{
+	int ret = 0;
+
+	if (iomap->length) {
+		if (end) {
+			/*
+			 * Calculate how far the iter was advanced and the
+			 * original length bytes for end().
+			 */
+			ssize_t advanced = iter->pos - iter->iter_start_pos;
+			loff_t len;
+
+			len = iomap_length_trim(iter, iter->iter_start_pos,
+					iter->len + advanced);
+
+			ret = end(iter->inode, iter->iter_start_pos, len,
+					advanced, iter->flags, iomap);
+		}
+		ret = iomap_iter_continue(iter, iomap, srcmap, ret);
+		if (ret <= 0)
+			return ret;
+	}
+
+	ret = begin(iter->inode, iter->pos, iter->len, iter->flags, iomap,
+			srcmap);
+
+	return ret < 0 ? ret : 1;
+}
+
+#define DEFINE_IOMAP_ITER_NEXT_END(name, begin_fn, end_fn)		\
+int name(const struct iomap_iter *iter, struct iomap *iomap,		\
+		struct iomap *srcmap)					\
+{									\
+	return iomap_iter_next(iter, iomap, srcmap, begin_fn, end_fn);	\
+}
+
+#define DEFINE_IOMAP_ITER_NEXT(name, begin_fn)				\
+	DEFINE_IOMAP_ITER_NEXT_END(name, begin_fn, NULL)
+
 /*
  * Return the file offset for the first unchanged block after a short write.
  *
-- 
2.53.0

[PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw

[Christoph Hellwig]

The pending iomap_iter_next conversion creates performance issues for the
new simple direct I/O read fast path, because it assumes a model where
the iterator must be advanced at the end, which the direct I/O read fast
path tries to avoid.

Side step this by splitting the simple path from iomap_dio_rw, and
require the file systems to call into it explicitly, and pass only a
->begin callback.  This allows to drop various checks for incompatible
features while creating a requirement for the file system to only call
the simple path for cases that it can handle.

As a side-benefit we can now inline the initial part of the simple
direct I/O read fast path and let the compiler convert the indirect call
to ->begin into a direct call.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/ext4/ext4.h        |   3 +
 fs/ext4/file.c        |   4 +-
 fs/ext4/inode.c       |   2 +-
 fs/iomap/direct-io.c  | 209 ++++++++++--------------------------------
 fs/xfs/xfs_file.c     |  14 +--
 fs/xfs/xfs_iomap.c    |   2 +-
 fs/xfs/xfs_iomap.h    |   4 +
 include/linux/iomap.h |  65 +++++++++++++
 8 files changed, 133 insertions(+), 170 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..e134c0193e2b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4007,6 +4007,9 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
 extern const struct iomap_ops ext4_iomap_ops;
 extern const struct iomap_ops ext4_iomap_report_ops;
 
+int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+		unsigned flags, struct iomap *iomap, struct iomap *srcmap);
+
 static inline int ext4_buffer_uptodate(struct buffer_head *bh)
 {
 	/*
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index eb1a323962b1..f20d92255546 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -91,7 +91,9 @@ 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, 0, NULL, 0);
+	ret = iomap_dio_read_simple(iocb, to, ext4_iomap_begin);
+	if (ret == -ENOTBLK)
+		ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0, NULL, 0);
 	inode_unlock_shared(inode);
 
 	file_accessed(iocb->ki_filp);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce99807c5f5b..b48c54f3312b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3771,7 +3771,7 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
 }
 
 
-static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 		unsigned flags, struct iomap *iomap, struct iomap *srcmap)
 {
 	int ret;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index ca790239e5eb..ca35da9e6cab 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -894,6 +894,21 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 }
 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,
+		unsigned int dio_flags, void *private, size_t done_before)
+{
+	struct iomap_dio *dio;
+
+	dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, private,
+			     done_before);
+	if (IS_ERR_OR_NULL(dio))
+		return PTR_ERR_OR_ZERO(dio);
+	return iomap_dio_complete(dio);
+}
+EXPORT_SYMBOL_GPL(iomap_dio_rw);
+
 struct iomap_dio_simple {
 	struct kiocb		*iocb;
 	size_t			size;
@@ -968,146 +983,47 @@ static void iomap_dio_simple_end_io(struct bio *bio)
 	iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
 }
 
-static inline bool
-iomap_dio_simple_supported(struct kiocb *iocb, struct iov_iter *iter,
-			   const struct iomap_dio_ops *dops,
-			   unsigned int dio_flags, size_t done_before)
+ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
+		struct iomap_iter *iomi)
 {
-	struct inode *inode = file_inode(iocb->ki_filp);
-	size_t count = iov_iter_count(iter);
-
-	if (dops || done_before)
-		return false;
-	if (iov_iter_rw(iter) != READ)
-		return false;
-	if (!count)
-		return false;
-	/*
-	 * Simple dio is an optimization for small IO. Filter out large IO
-	 * early as it's the most common case to fail for typical direct IO
-	 * workloads.
-	 */
-	if (count > inode->i_sb->s_blocksize)
-		return false;
-	if (dio_flags & (IOMAP_DIO_FORCE_WAIT | IOMAP_DIO_PARTIAL |
-			 IOMAP_DIO_BOUNCE))
-		return false;
-	if (iocb->ki_pos + count > i_size_read(inode))
-		return false;
-	if (IS_ENCRYPTED(inode))
-		return false;
-
-	return true;
-}
-
-/*
- * Fast path for small, block-aligned direct I/Os that map to a single
- * contiguous on-disk extent.
- *
- * iomap_dio_simple_supported() enforces the cheap up-front constraints before
- * entering this path.
- *
- * @dops must be NULL: a non-NULL @dops means the caller wants its
- * ->end_io / ->submit_io hooks invoked, and in particular wants its bios to be
- * allocated from the filesystem-private @dops->bio_set (whose front_pad sizes a
- * filesystem-private wrapper around the bio).  The fast path instead allocates
- * from the shared iomap_dio_simple_pool, whose front_pad matches struct
- * iomap_dio_simple; the two wrappers are not interchangeable, so we must fall
- * back to __iomap_dio_rw() in that case.
- *
- * @done_before must be zero: a non-zero caller-accumulated residual cannot be
- * carried through a single-bio inline completion.
- *
- * @iter must describe a non-empty READ no larger than the inode block size:
- * writes, zero-length I/O, and larger requests need the generic iomap direct
- * I/O path.
- *
- * @dio_flags must not request IOMAP_DIO_FORCE_WAIT, IOMAP_DIO_PARTIAL, or
- * IOMAP_DIO_BOUNCE: this path does not support forced waiting, partial direct
- * I/O, or bouncing.  The range must also stay within i_size and encrypted
- * inodes must use the generic iomap direct I/O path.
- *
- * -ENOTBLK is the private sentinel returned by iomap_dio_simple() when it
- * decides the request does not fit the fast path.  In that case we proceed to
- * the generic __iomap_dio_rw() slow path.  Any other errno is a real result and
- * is propagated as-is, in particular -EAGAIN for IOCB_NOWAIT must reach the
- * caller.
- */
-static ssize_t
-iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
-		 const struct iomap_ops *ops, void *private,
-		 unsigned int dio_flags)
-{
-	struct inode *inode = file_inode(iocb->ki_filp);
-	size_t count = iov_iter_count(iter);
-	bool wait_for_completion = is_sync_kiocb(iocb);
-	struct iomap_iter iomi = {
-		.inode		= inode,
-		.pos		= iocb->ki_pos,
-		.len		= count,
-		.flags		= IOMAP_DIRECT,
-		.private	= private,
-	};
 	struct iomap_dio_simple *sr;
 	unsigned int alignment;
 	struct bio *bio;
 	ssize_t ret;
 
-	if (iocb->ki_flags & IOCB_NOWAIT)
-		iomi.flags |= IOMAP_NOWAIT;
-
-	ret = kiocb_write_and_wait(iocb, count);
-	if (ret)
-		return ret;
-
-	inode_dio_begin(inode);
+	if (iomi->iomap.type != IOMAP_MAPPED ||
+	    iomi->iomap.offset + iomi->iomap.length < iomi->pos + iomi->len ||
+	    (iomi->iomap.flags & IOMAP_F_INTEGRITY))
+		return -ENOTBLK;
 
-	ret = ops->iomap_begin(inode, iomi.pos, count, iomi.flags,
-			       &iomi.iomap, &iomi.srcmap);
-	if (ret) {
-		inode_dio_end(inode);
-		return ret;
-	}
-
-	if (iomi.iomap.type != IOMAP_MAPPED ||
-	    iomi.iomap.offset + iomi.iomap.length < iomi.pos + count ||
-	    (iomi.iomap.flags & IOMAP_F_INTEGRITY)) {
-		ret = -ENOTBLK;
-		goto out_iomap_end;
-	}
-
-	alignment = iomap_dio_alignment(inode, iomi.iomap.bdev, dio_flags);
-	if ((iomi.pos | count) & (alignment - 1)) {
-		ret = -EINVAL;
-		goto out_iomap_end;
-	}
+	alignment = iomap_dio_alignment(iomi->inode, iomi->iomap.bdev, 0);
+	if ((iomi->pos | iomi->len) & (alignment - 1))
+		return -EINVAL;
 
-	if (!wait_for_completion && unlikely(!inode->i_sb->s_dio_done_wq)) {
-		ret = sb_init_dio_done_wq(inode->i_sb);
+	if (unlikely(!iomi->inode->i_sb->s_dio_done_wq &&
+			!is_sync_kiocb(iocb))) {
+		ret = sb_init_dio_done_wq(iomi->inode->i_sb);
 		if (ret < 0)
-			goto out_iomap_end;
+			return ret;
 	}
 
-	trace_iomap_dio_rw_begin(iocb, iter, dio_flags, 0);
-
-	if (user_backed_iter(iter))
-		dio_flags |= IOMAP_DIO_USER_BACKED;
+	trace_iomap_dio_rw_begin(iocb, iter, 0, 0);
 
-	bio = bio_alloc_bioset(iomi.iomap.bdev,
+	bio = bio_alloc_bioset(iomi->iomap.bdev,
 			       bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS),
 			       REQ_OP_READ, GFP_KERNEL, &iomap_dio_simple_pool);
 	sr = container_of(bio, struct iomap_dio_simple, bio);
 	sr->iocb = iocb;
-	sr->dio_flags = dio_flags;
+	sr->dio_flags = 0;
 
-	bio->bi_iter.bi_sector = iomap_sector(&iomi.iomap, iomi.pos);
+	bio->bi_iter.bi_sector = iomap_sector(&iomi->iomap, iomi->pos);
 	bio->bi_ioprio = iocb->ki_ioprio;
 
 	ret = bio_iov_iter_get_pages(bio, iter, alignment - 1);
 	if (unlikely(ret))
 		goto out_bio_put;
 
-	if (bio->bi_iter.bi_size != count) {
+	if (bio->bi_iter.bi_size != iomi->len) {
 		iov_iter_revert(iter, bio->bi_iter.bi_size);
 		ret = -ENOTBLK;
 		goto out_bio_release_pages;
@@ -1115,64 +1031,35 @@ iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
 
 	sr->size = bio->bi_iter.bi_size;
 
-	if (dio_flags & IOMAP_DIO_USER_BACKED)
+	if (user_backed_iter(iter)) {
 		bio_set_pages_dirty(bio);
+		sr->dio_flags |= IOMAP_DIO_USER_BACKED;
+	}
 
 	if (iocb->ki_flags & IOCB_NOWAIT)
 		bio->bi_opf |= REQ_NOWAIT;
-	if ((iocb->ki_flags & IOCB_HIPRI) && !wait_for_completion) {
-		bio->bi_opf |= REQ_POLLED;
-		WRITE_ONCE(iocb->private, bio);
-	}
 
-	if (ops->iomap_end)
-		ops->iomap_end(inode, iomi.pos, count, count, iomi.flags,
-			       &iomi.iomap);
-
-	if (!wait_for_completion) {
-		bio->bi_end_io = iomap_dio_simple_end_io;
-		submit_bio(bio);
-		trace_iomap_dio_rw_queued(inode, iomi.pos, count);
-		return -EIOCBQUEUED;
+	if (is_sync_kiocb(iocb)) {
+		submit_bio_wait(bio);
+		return iomap_dio_simple_complete(sr);
 	}
 
-	submit_bio_wait(bio);
-	return iomap_dio_simple_complete(sr);
+	if ((iocb->ki_flags & IOCB_HIPRI)) {
+		bio->bi_opf |= REQ_POLLED;
+		WRITE_ONCE(iocb->private, bio);
+	}
+	bio->bi_end_io = iomap_dio_simple_end_io;
+	submit_bio(bio);
+	trace_iomap_dio_rw_queued(iomi->inode, iocb->ki_pos, iomi->len);
+	return -EIOCBQUEUED;
 
 out_bio_release_pages:
 	bio_release_pages(bio, false);
 out_bio_put:
 	bio_put(bio);
-out_iomap_end:
-	if (ops->iomap_end)
-		ops->iomap_end(inode, iomi.pos, count, 0, iomi.flags,
-			       &iomi.iomap);
-	inode_dio_end(inode);
 	return ret;
 }
-
-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, void *private, size_t done_before)
-{
-	struct iomap_dio *dio;
-	ssize_t ret;
-
-	if (iomap_dio_simple_supported(iocb, iter, dops, dio_flags,
-				       done_before)) {
-		ret = iomap_dio_simple(iocb, iter, ops, private, dio_flags);
-		if (ret != -ENOTBLK)
-			return ret;
-	}
-
-	dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, private,
-			     done_before);
-	if (IS_ERR_OR_NULL(dio))
-		return PTR_ERR_OR_ZERO(dio);
-	return iomap_dio_complete(dio);
-}
-EXPORT_SYMBOL_GPL(iomap_dio_rw);
+EXPORT_SYMBOL_GPL(__iomap_dio_simple);
 
 static int __init iomap_dio_init(void)
 {
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 845a97c9b063..b733225d2864 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -251,8 +251,6 @@ xfs_file_dio_read(
 	struct iov_iter		*to)
 {
 	struct xfs_inode	*ip = XFS_I(file_inode(iocb->ki_filp));
-	unsigned int		dio_flags = 0;
-	const struct iomap_dio_ops *dio_ops = NULL;
 	ssize_t			ret;
 
 	trace_xfs_file_direct_read(iocb, to);
@@ -266,11 +264,15 @@ xfs_file_dio_read(
 	if (ret)
 		return ret;
 	if (mapping_stable_writes(iocb->ki_filp->f_mapping)) {
-		dio_ops = &xfs_dio_read_bounce_ops;
-		dio_flags |= IOMAP_DIO_BOUNCE;
+		ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops,
+				&xfs_dio_read_bounce_ops, IOMAP_DIO_BOUNCE,
+				NULL, 0);
+	} else {
+		ret = iomap_dio_read_simple(iocb, to, xfs_read_iomap_begin);
+		if (ret == -ENOTBLK)
+			ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL,
+					0, NULL, 0);
 	}
-	ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, dio_ops, dio_flags,
-			NULL, 0);
 	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
 
 	return ret;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 225c3de88d03..0536e2aeddcc 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -2173,7 +2173,7 @@ const struct iomap_ops xfs_buffered_write_iomap_ops = {
 	.iomap_end		= xfs_buffered_write_iomap_end,
 };
 
-static int
+int
 xfs_read_iomap_begin(
 	struct inode		*inode,
 	loff_t			offset,
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index ebcce7d49446..cffcec532ea6 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -49,6 +49,10 @@ xfs_aligned_fsb_count(
 	return count_fsb;
 }
 
+int xfs_read_iomap_begin(struct inode *inode, loff_t offset,
+		loff_t length, unsigned flags, struct iomap *iomap,
+		struct iomap *srcmap);
+
 extern const struct iomap_ops xfs_buffered_write_iomap_ops;
 extern const struct iomap_ops xfs_direct_write_iomap_ops;
 extern const struct iomap_ops xfs_zoned_direct_write_iomap_ops;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 3df851d0f76b..810dc4b8dc94 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -10,6 +10,7 @@
 #include <linux/mm_types.h>
 #include <linux/blkdev.h>
 #include <linux/folio_batch.h>
+#include <linux/pagemap.h>
 
 struct address_space;
 struct fiemap_extent_info;
@@ -674,6 +675,70 @@ struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 ssize_t iomap_dio_complete(struct iomap_dio *dio);
 void iomap_dio_bio_end_io(struct bio *bio);
 
+/*
+ * Fast path for small, block-aligned direct I/Os that map to a single
+ * contiguous on-disk extent.
+ *
+ * @iter must describe a non-empty READ no larger than the inode block size:
+ * writes, zero-length I/O, and larger requests need the generic iomap direct
+ * I/O path.
+ *
+ * Does not support iomap_dio_ops, dio_flags, done_before or private data.
+ * The range must also stay within i_size and encrypted inodes must use the
+ * generic iomap direct I/O path.
+ *
+ * -ENOTBLK inidicates the generic path must be used by the caller instead.
+ * Any other errno is a real result and is propagated as-is, in particular
+ * -EAGAIN for IOCB_NOWAIT must reach the caller.
+ *
+ * The caller can only provide an iomap begin handler, and the iterator
+ * is never advanced.
+ */
+ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
+		struct iomap_iter *iomi);
+static __always_inline ssize_t iomap_dio_read_simple(struct kiocb *iocb,
+		struct iov_iter *iter, iomap_iter_begin_fn begin)
+{
+	struct iomap_iter iomi = {
+		.inode		= file_inode(iocb->ki_filp),
+		.pos		= iocb->ki_pos,
+		.len		= iov_iter_count(iter),
+		.flags		= IOMAP_DIRECT,
+	};
+	ssize_t ret;
+
+	if (!iomi.len)
+		return 0;
+
+	/*
+	 * Simple dio is an optimization for small IO. Filter out large IO
+	 * early as it's the most common case to fail for typical direct IO
+	 * workloads.
+	 */
+	if (iomi.len > iomi.inode->i_sb->s_blocksize)
+		return -ENOTBLK;
+	if (iocb->ki_pos + iomi.len > i_size_read(iomi.inode))
+		return -ENOTBLK;
+	if (IS_ENCRYPTED(iomi.inode))
+		return -ENOTBLK;
+
+	ret = kiocb_write_and_wait(iocb, iomi.len);
+	if (ret)
+		return ret;
+
+	if (iocb->ki_flags & IOCB_NOWAIT)
+		iomi.flags |= IOMAP_NOWAIT;
+
+	inode_dio_begin(iomi.inode);
+	ret = begin(iomi.inode, iomi.pos, iomi.len, iomi.flags, &iomi.iomap,
+			&iomi.srcmap);
+	if (!ret)
+		ret = __iomap_dio_simple(iocb, iter, &iomi);
+	if (ret <= 0 && ret != -EIOCBQUEUED)
+		inode_dio_end(iomi.inode);
+	return ret;
+}
+
 #ifdef CONFIG_SWAP
 struct file;
 struct swap_info_struct;
-- 
2.53.0

[PATCH 3/3] iomap: use GFP_NOWAIT when application for iomap_dio_simple allocations

[Christoph Hellwig]

For non-blocking iocbs we should avoid blocking allocation where
possible, so switch to a GFP_NOWAIT allocation here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap/direct-io.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index ca35da9e6cab..3bdfbd0efa07 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -986,6 +986,7 @@ static void iomap_dio_simple_end_io(struct bio *bio)
 ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
 		struct iomap_iter *iomi)
 {
+	gfp_t gfp = (iomi->flags & IOMAP_NOWAIT) ? GFP_NOWAIT : GFP_KERNEL;
 	struct iomap_dio_simple *sr;
 	unsigned int alignment;
 	struct bio *bio;
@@ -1011,7 +1012,9 @@ ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
 
 	bio = bio_alloc_bioset(iomi->iomap.bdev,
 			       bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS),
-			       REQ_OP_READ, GFP_KERNEL, &iomap_dio_simple_pool);
+			       REQ_OP_READ, gfp, &iomap_dio_simple_pool);
+	if (!bio)
+		return -EAGAIN;
 	sr = container_of(bio, struct iomap_dio_simple, bio);
 	sr->iocb = iocb;
 	sr->dio_flags = 0;
-- 
2.53.0

Re: [PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw

[Joanne Koong]

On Wed, Jul 22, 2026 at 5:49 AM Christoph Hellwig <hch@lst.de> wrote:
>
> The pending iomap_iter_next conversion creates performance issues for the
> new simple direct I/O read fast path, because it assumes a model where
> the iterator must be advanced at the end, which the direct I/O read fast
> path tries to avoid.
>
> Side step this by splitting the simple path from iomap_dio_rw, and
> require the file systems to call into it explicitly, and pass only a
> ->begin callback.  This allows to drop various checks for incompatible
> features while creating a requirement for the file system to only call
> the simple path for cases that it can handle.

Nice, this approach looks cleaner to me.

>
> As a side-benefit we can now inline the initial part of the simple
> direct I/O read fast path and let the compiler convert the indirect call
> to ->begin into a direct call.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/ext4/ext4.h        |   3 +
>  fs/ext4/file.c        |   4 +-
>  fs/ext4/inode.c       |   2 +-
>  fs/iomap/direct-io.c  | 209 ++++++++++--------------------------------
>  fs/xfs/xfs_file.c     |  14 +--
>  fs/xfs/xfs_iomap.c    |   2 +-
>  fs/xfs/xfs_iomap.h    |   4 +
>  include/linux/iomap.h |  65 +++++++++++++
>  8 files changed, 133 insertions(+), 170 deletions(-)
>
> index 3df851d0f76b..810dc4b8dc94 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
>
> +/*
> + * Fast path for small, block-aligned direct I/Os that map to a single
> + * contiguous on-disk extent.
> + *
> + * @iter must describe a non-empty READ no larger than the inode block size:
> + * writes, zero-length I/O, and larger requests need the generic iomap direct
> + * I/O path.
> + *
> + * Does not support iomap_dio_ops, dio_flags, done_before or private data.
> + * The range must also stay within i_size and encrypted inodes must use the
> + * generic iomap direct I/O path.
> + *
> + * -ENOTBLK inidicates the generic path must be used by the caller instead.
> + * Any other errno is a real result and is propagated as-is, in particular
> + * -EAGAIN for IOCB_NOWAIT must reach the caller.
> + *
> + * The caller can only provide an iomap begin handler, and the iterator
> + * is never advanced.
> + */
> +ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
> +               struct iomap_iter *iomi);

Might be nice to have _read_ in this name as well, eg __iomap_dio_read_simple

> +static __always_inline ssize_t iomap_dio_read_simple(struct kiocb *iocb,
> +               struct iov_iter *iter, iomap_iter_begin_fn begin)
> +{
> +       struct iomap_iter iomi = {
> +               .inode          = file_inode(iocb->ki_filp),
> +               .pos            = iocb->ki_pos,
> +               .len            = iov_iter_count(iter),
> +               .flags          = IOMAP_DIRECT,
> +       };
> +       ssize_t ret;
> +
> +       if (!iomi.len)
> +               return 0;
> +
> +       /*
> +        * Simple dio is an optimization for small IO. Filter out large IO
> +        * early as it's the most common case to fail for typical direct IO
> +        * workloads.
> +        */
> +       if (iomi.len > iomi.inode->i_sb->s_blocksize)
> +               return -ENOTBLK;
> +       if (iocb->ki_pos + iomi.len > i_size_read(iomi.inode))
> +               return -ENOTBLK;
> +       if (IS_ENCRYPTED(iomi.inode))
> +               return -ENOTBLK;
> +
> +       ret = kiocb_write_and_wait(iocb, iomi.len);
> +       if (ret)
> +               return ret;
> +
> +       if (iocb->ki_flags & IOCB_NOWAIT)
> +               iomi.flags |= IOMAP_NOWAIT;
> +
> +       inode_dio_begin(iomi.inode);
> +       ret = begin(iomi.inode, iomi.pos, iomi.len, iomi.flags, &iomi.iomap,
> +                       &iomi.srcmap);
> +       if (!ret)
> +               ret = __iomap_dio_simple(iocb, iter, &iomi);
> +       if (ret <= 0 && ret != -EIOCBQUEUED)
> +               inode_dio_end(iomi.inode);

afaict, the inode_dio_end() error handling has to be done by
__iomap_dio_simple() to properly account for the synchronous case. I
ran into this earlier when doing the iomap_next_fn inlining. For the
synchronous case, __iomap_dio_simple() calls
iomap_dio_simple_complete() which calls inode_dio_end() even on
errors.

Thanks,
Joanne

> +       return ret;
> +}

Re: [PATCH 3/3] iomap: use GFP_NOWAIT when application for iomap_dio_simple allocations

[Joanne Koong]

On Wed, Jul 22, 2026 at 5:49 AM Christoph Hellwig <hch@lst.de> wrote:
>
> For non-blocking iocbs we should avoid blocking allocation where
> possible, so switch to a GFP_NOWAIT allocation here.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Joanne Koong <joannelkoong@gmail.com>

Re: [PATCH 1/3] iomap: split iomap_iter() logic into iomap_iter_next()

[changfengnan]

> From: "Christoph Hellwig"<hch@lst.de>
> Date:  Wed, Jul 22, 2026, 20:49
> Subject:  [PATCH 1/3] iomap: split iomap_iter() logic into iomap_iter_next()
> To: <changfengnan@bytedance.com>, "Joanne Koong"<joannelkoong@gmail.com>, "Darrick J. Wong"<djwong@kernel.org>, "Christian Brauner"<brauner@kernel.org>
> Cc: "Theodore Ts'o"<tytso@mit.edu>, "Carlos Maiolino"<cem@kernel.org>, <linux-ext4@vger.kernel.org>, <linux-xfs@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
> From: Joanne Koong <joannelkoong@gmail.com>
> 
> In preparation for changing iomap to use an in-iter (->iomap_next())
> model, move the iomap_iter() logic out into the new iomap_iter_next()
> helper function.
> 
> iomap_iter_next() is added as an inlined helper so it can be called
> directly by ->iomap_next() implementations where the begin()/end()
> callbacks can be direct calls.
> 
> The DEFINE_IOMAP_ITER_NEXT() and DEFINE_IOMAP_ITER_NEXT_END() macros are
> also provided to generate the boilerplate ->iomap_next() wrapper
> functions that simply forward to iomap_iter_next() with the appropriate
> begin/end callbacks. DEFINE_IOMAP_ITER_NEXT() is for the common case
> where there is no end() callback. DEFINE_IOMAP_ITER_NEXT_END() is for
> the case where there is an explicit end() callback.
> 
> No functional change intended. The only code-level difference is that on
> the iomap_end() error path (ret < 0 && !advanced), the old code returned
> with iter.status left as the caller's last value whereas the new code
> zeroes it, but this is not observable in practice as there are no in-tree
> callers that read iter.status after the iteration loop.
> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

Reviewed-by: Fengnan Chang <changfengnan@bytedance.com>

> ---
>  fs/iomap/iter.c       | 123 +++++++++++++++++++++---------------------
>  include/linux/iomap.h | 102 +++++++++++++++++++++++++++++------
>  2 files changed, 147 insertions(+), 78 deletions(-)
> 
> diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
> index e4a29829591a..66ccb87441ab 100644
> --- a/fs/iomap/iter.c
> +++ b/fs/iomap/iter.c
> @@ -6,15 +6,6 @@
>  #include <linux/iomap.h>
>  #include "trace.h"
>  
> -static inline void iomap_iter_clean_fbatch(struct iomap_iter *iter)
> -{
> -        if (iter->iomap.flags & IOMAP_F_FOLIO_BATCH) {
> -                folio_batch_release(iter->fbatch);
> -                folio_batch_reinit(iter->fbatch);
> -                iter->iomap.flags &= ~IOMAP_F_FOLIO_BATCH;
> -        }
> -}
> -
>  /* Advance the current iterator position and decrement the remaining length */
>  int iomap_iter_advance(struct iomap_iter *iter, u64 count)
>  {
> @@ -40,51 +31,28 @@ static inline void iomap_iter_done(struct iomap_iter *iter)
>  }
>  
>  /**
> - * iomap_iter - iterate over a ranges in a file
> - * @iter: iteration structue
> - * @ops: iomap ops provided by the file system
> + * iomap_iter_continue - decide whether iteration should continue
> + * @iter: iteration structure
> + * @iomap: the mapping that was just processed
> + * @srcmap: the source mapping that was just processed
>   *
> - * Iterate over filesystem-provided space mappings for the provided file range.
> + * Helper normally called via iomap_iter_next(). Called after the previous
> + * mapping has been finished to determine whether there is more of the file
> + * range left to process.
>   *
> - * This function handles cleanup of resources acquired for iteration when the
> - * filesystem indicates there are no more space mappings, which means that this
> - * function must be called in a loop that continues as long it returns a
> - * positive value.  If 0 or a negative value is returned, the caller must not
> - * return to the loop body.  Within a loop body, there are two ways to break out
> - * of the loop body:  leave @iter.status unchanged, or set it to a negative
> - * errno.
> + * Returns 1 if there is more work to do, in which case @iomap and @srcmap are
> + * cleared so the caller can produce the next mapping; zero if the range is
> + * fully consumed; or a negative errno on error. Any folio batch attached to
> + * the mapping is released before returning.
>   */
> -int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
> +int iomap_iter_continue(const struct iomap_iter *iter, struct iomap *iomap,
> +                struct iomap *srcmap, int ret)
>  {
> -        bool stale = iter->iomap.flags & IOMAP_F_STALE;
> -        ssize_t advanced;
> -        u64 olen;
> -        int ret;
> -
> -        trace_iomap_iter(iter, ops, _RET_IP_);
> -
> -        if (!iter->iomap.length)
> -                goto begin;
> -
> -        /*
> -         * Calculate how far the iter was advanced and the original length bytes
> -         * for ->iomap_end().
> -         */
> -        advanced = iter->pos - iter->iter_start_pos;
> -        olen = iter->len + advanced;
> -
> -        if (ops->iomap_end) {
> -                ret = ops->iomap_end(iter->inode, iter->iter_start_pos,
> -                                iomap_length_trim(iter, iter->iter_start_pos,
> -                                                  olen),
> -                                advanced, iter->flags, &iter->iomap);
> -                if (ret < 0 && !advanced)
> -                        return ret;
> -        }
> +        const bool stale = iomap->flags & IOMAP_F_STALE;
> +        const ssize_t advanced = iter->pos - iter->iter_start_pos;
>  
> -        /* detect old return semantics where this would advance */
> -        if (WARN_ON_ONCE(iter->status > 0))
> -                iter->status = -EIO;
> +        if (ret < 0 && !advanced)
> +                return ret;
>  
>          /*
>           * Use iter->len to determine whether to continue onto the next mapping.
> @@ -92,25 +60,58 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
>           * advanced at all (i.e. no work was done for some reason) unless the
>           * mapping has been marked stale and needs to be reprocessed.
>           */
> -        if (iter->status < 0)
> +        if (WARN_ON_ONCE(iter->status > 0))
> +                /* detect old return semantics where this would advance */
> +                ret = -EIO;
> +        else if (iter->status < 0)
>                  ret = iter->status;
>          else if (iter->len == 0 || (!advanced && !stale))
>                  ret = 0;
>          else
>                  ret = 1;
> -        iomap_iter_clean_fbatch(iter);
> -        iter->status = 0;
> +
> +        if (iomap->flags & IOMAP_F_FOLIO_BATCH) {
> +                folio_batch_release(iter->fbatch);
> +                folio_batch_reinit(iter->fbatch);
> +                iomap->flags &= ~IOMAP_F_FOLIO_BATCH;
> +        }
> +
>          if (ret <= 0)
>                  return ret;
>  
> -        memset(&iter->iomap, 0, sizeof(iter->iomap));
> -        memset(&iter->srcmap, 0, sizeof(iter->srcmap));
> +        memset(iomap, 0, sizeof(*iomap));
> +        memset(srcmap, 0, sizeof(*srcmap));
>  
> -begin:
> -        ret = ops->iomap_begin(iter->inode, iter->pos, iter->len, iter->flags,
> -                               &iter->iomap, &iter->srcmap);
> -        if (ret < 0)
> -                return ret;
> -        iomap_iter_done(iter);
> -        return 1;
> +        return ret;
> +}
> +EXPORT_SYMBOL_GPL(iomap_iter_continue);
> +
> +/**
> + * iomap_iter - iterate over ranges in a file
> + * @iter: iteration structure
> + * @ops: iomap ops provided by the filesystem
> + *
> + * Iterate over filesystem-provided space mappings for the provided file range.
> + *
> + * This function handles cleanup of resources acquired for iteration when the
> + * filesystem indicates there are no more space mappings, which means that this
> + * function must be called in a loop that continues as long it returns a
> + * positive value.  If 0 or a negative value is returned, the caller must not
> + * return to the loop body.  Within a loop body, there are two ways to break out
> + * of the loop body:  leave @iter.status unchanged, or set it to a negative
> + * errno.
> + */
> +int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
> +{
> +        int ret;
> +
> +        trace_iomap_iter(iter, ops, _RET_IP_);
> +
> +        ret = iomap_iter_next(iter, &iter->iomap, &iter->srcmap,
> +                        ops->iomap_begin, ops->iomap_end);
> +        iter->status = 0;
> +        if (ret > 0)
> +                iomap_iter_done(iter);
> +
> +        return ret;
>  }
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 3582ed1fe236..3df851d0f76b 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -212,24 +212,27 @@ struct iomap_write_ops {
>  #define IOMAP_ATOMIC                (1 << 9) /* torn-write protection */
>  #define IOMAP_DONTCACHE                (1 << 10)
>  
> -struct iomap_ops {
> -        /*
> -         * Return the existing mapping at pos, or reserve space starting at
> -         * pos for up to length, as long as we can do it as a single mapping.
> -         * The actual length is returned in iomap->length.
> -         */
> -        int (*iomap_begin)(struct inode *inode, loff_t pos, loff_t length,
> -                        unsigned flags, struct iomap *iomap,
> -                        struct iomap *srcmap);
> +/*
> + * Return the existing mapping at pos, or reserve space starting at pos for up
> + * to length, as long as we can do it as a single mapping.
> + * The actual length is returned in iomap->length.
> + */
> +typedef int (*iomap_iter_begin_fn)(struct inode *inode, loff_t pos,
> +                loff_t length, unsigned flags, struct iomap *iomap,
> +                struct iomap *srcmap);
>  
> -        /*
> -         * Commit and/or unreserve space previous allocated using iomap_begin.
> -         * Written indicates the length of the successful write operation which
> -         * needs to be commited, while the rest needs to be unreserved.
> -         * Written might be zero if no data was written.
> -         */
> -        int (*iomap_end)(struct inode *inode, loff_t pos, loff_t length,
> -                        ssize_t written, unsigned flags, struct iomap *iomap);
> +/*
> + * Commit and/or unreserve space previously allocated by iomap_iter_begin_fn.
> + * Written indicates the length of the successful write operation which needs
> + * to be committed, while the rest needs to be unreserved.
> + * Written might be zero if no data was written.
> + */
> +typedef int (*iomap_iter_end_fn)(struct inode *inode, loff_t pos, loff_t length,
> +                ssize_t written, unsigned flags, struct iomap *iomap);
> +
> +struct iomap_ops {
> +        iomap_iter_begin_fn iomap_begin;
> +        iomap_iter_end_fn iomap_end;
>  };
>  
>  /**
> @@ -317,6 +320,71 @@ static inline const struct iomap *iomap_iter_srcmap(const struct iomap_iter *i)
>          return &i->iomap;
>  }
>  
> +int iomap_iter_continue(const struct iomap_iter *iter, struct iomap *iomap,
> +                struct iomap *srcmap, int ret);
> +
> +/**
> + * iomap_iter_next - finish the previous mapping and produce the next one
> + * @iter: iteration structure
> + * @iomap: mapping to finish and then repopulate
> + * @srcmap: source mapping to finish and then repopulate
> + * @begin: callback that produces a mapping for the current position
> + * @end: optional callback that finishes the previous mapping, or NULL
> + *
> + * Inline helper that implements the common body of an ->iomap_next()
> + * callback: it finishes the previous mapping via @end (if present), decides
> + * via iomap_iter_continue() whether to keep going, and obtains the next
> + * mapping via @begin.
> + *
> + * This helper is marked __always_inline so that when a caller passes
> + * compile-time-constant @begin and @end callbacks, the compiler can call them
> + * directly, avoiding the indirect-call overhead.
> + *
> + * Returns 1 to continue iterating, 0 once the range is fully consumed, or a
> + * negative errno on error.
> + */
> +static __always_inline int iomap_iter_next(const struct iomap_iter *iter,
> +                struct iomap *iomap, struct iomap *srcmap,
> +                iomap_iter_begin_fn begin, iomap_iter_end_fn end)
> +{
> +        int ret = 0;
> +
> +        if (iomap->length) {
> +                if (end) {
> +                        /*
> +                         * Calculate how far the iter was advanced and the
> +                         * original length bytes for end().
> +                         */
> +                        ssize_t advanced = iter->pos - iter->iter_start_pos;
> +                        loff_t len;
> +
> +                        len = iomap_length_trim(iter, iter->iter_start_pos,
> +                                        iter->len + advanced);
> +
> +                        ret = end(iter->inode, iter->iter_start_pos, len,
> +                                        advanced, iter->flags, iomap);
> +                }
> +                ret = iomap_iter_continue(iter, iomap, srcmap, ret);
> +                if (ret <= 0)
> +                        return ret;
> +        }
> +
> +        ret = begin(iter->inode, iter->pos, iter->len, iter->flags, iomap,
> +                        srcmap);
> +
> +        return ret < 0 ? ret : 1;
> +}
> +
> +#define DEFINE_IOMAP_ITER_NEXT_END(name, begin_fn, end_fn)                \
> +int name(const struct iomap_iter *iter, struct iomap *iomap,                \
> +                struct iomap *srcmap)                                        \
> +{                                                                        \
> +        return iomap_iter_next(iter, iomap, srcmap, begin_fn, end_fn);        \
> +}
> +
> +#define DEFINE_IOMAP_ITER_NEXT(name, begin_fn)                                \
> +        DEFINE_IOMAP_ITER_NEXT_END(name, begin_fn, NULL)
> +
>  /*
>   * Return the file offset for the first unchanged block after a short write.
>   *
> -- 
> 2.53.0
> 

Re: [PATCH 3/3] iomap: use GFP_NOWAIT when application for iomap_dio_simple allocations

[changfengnan]

> From: "Joanne Koong"<joannelkoong@gmail.com>
> Date:  Thu, Jul 23, 2026, 05:02
> Subject:  Re: [PATCH 3/3] iomap: use GFP_NOWAIT when application for iomap_dio_simple allocations
> To: "Christoph Hellwig"<hch@lst.de>
> Cc: <changfengnan@bytedance.com>, "Darrick J. Wong"<djwong@kernel.org>, "Christian Brauner"<brauner@kernel.org>, "Theodore Ts'o"<tytso@mit.edu>, "Carlos Maiolino"<cem@kernel.org>, <linux-ext4@vger.kernel.org>, <linux-xfs@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
> On Wed, Jul 22, 2026 at 5:49 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > For non-blocking iocbs we should avoid blocking allocation where
> > possible, so switch to a GFP_NOWAIT allocation here.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Reviewed-by: Joanne Koong <joannelkoong@gmail.com>

Reviewed-by: Fengnan Chang <changfengnan@bytedance.com>

> 

Re: [PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw

[changfengnan]

> From: "Joanne Koong"<joannelkoong@gmail.com>
> Date:  Thu, Jul 23, 2026, 03:04
> Subject:  Re: [PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw
> To: "Christoph Hellwig"<hch@lst.de>
> Cc: <changfengnan@bytedance.com>, "Darrick J. Wong"<djwong@kernel.org>, "Christian Brauner"<brauner@kernel.org>, "Theodore Ts'o"<tytso@mit.edu>, "Carlos Maiolino"<cem@kernel.org>, <linux-ext4@vger.kernel.org>, <linux-xfs@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>
> On Wed, Jul 22, 2026 at 5:49 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > The pending iomap_iter_next conversion creates performance issues for the
> > new simple direct I/O read fast path, because it assumes a model where
> > the iterator must be advanced at the end, which the direct I/O read fast
> > path tries to avoid.
> >
> > Side step this by splitting the simple path from iomap_dio_rw, and
> > require the file systems to call into it explicitly, and pass only a
> > ->begin callback.  This allows to drop various checks for incompatible
> > features while creating a requirement for the file system to only call
> > the simple path for cases that it can handle.
> 
> Nice, this approach looks cleaner to me.
> 
> >
> > As a side-benefit we can now inline the initial part of the simple
> > direct I/O read fast path and let the compiler convert the indirect call
> > to ->begin into a direct call.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/ext4/ext4.h        |   3 +
> >  fs/ext4/file.c        |   4 +-
> >  fs/ext4/inode.c       |   2 +-
> >  fs/iomap/direct-io.c  | 209 ++++++++++--------------------------------
> >  fs/xfs/xfs_file.c     |  14 +--
> >  fs/xfs/xfs_iomap.c    |   2 +-
> >  fs/xfs/xfs_iomap.h    |   4 +
> >  include/linux/iomap.h |  65 +++++++++++++
> >  8 files changed, 133 insertions(+), 170 deletions(-)
> >
> > index 3df851d0f76b..810dc4b8dc94 100644
> > --- a/include/linux/iomap.h
> > +++ b/include/linux/iomap.h
> >
> > +/*
> > + * Fast path for small, block-aligned direct I/Os that map to a single
> > + * contiguous on-disk extent.
> > + *
> > + * @iter must describe a non-empty READ no larger than the inode block size:
> > + * writes, zero-length I/O, and larger requests need the generic iomap direct
> > + * I/O path.
> > + *
> > + * Does not support iomap_dio_ops, dio_flags, done_before or private data.
> > + * The range must also stay within i_size and encrypted inodes must use the
> > + * generic iomap direct I/O path.
> > + *
> > + * -ENOTBLK inidicates the generic path must be used by the caller instead.
> > + * Any other errno is a real result and is propagated as-is, in particular
> > + * -EAGAIN for IOCB_NOWAIT must reach the caller.
> > + *
> > + * The caller can only provide an iomap begin handler, and the iterator
> > + * is never advanced.
> > + */
> > +ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
> > +               struct iomap_iter *iomi);
> 
> Might be nice to have _read_ in this name as well, eg __iomap_dio_read_simple
> 
> > +static __always_inline ssize_t iomap_dio_read_simple(struct kiocb *iocb,
> > +               struct iov_iter *iter, iomap_iter_begin_fn begin)
> > +{
> > +       struct iomap_iter iomi = {
> > +               .inode          = file_inode(iocb->ki_filp),
> > +               .pos            = iocb->ki_pos,
> > +               .len            = iov_iter_count(iter),
> > +               .flags          = IOMAP_DIRECT,
> > +       };
> > +       ssize_t ret;
> > +
> > +       if (!iomi.len)
> > +               return 0;
> > +
> > +       /*
> > +        * Simple dio is an optimization for small IO. Filter out large IO
> > +        * early as it's the most common case to fail for typical direct IO
> > +        * workloads.
> > +        */
> > +       if (iomi.len > iomi.inode->i_sb->s_blocksize)
> > +               return -ENOTBLK;
> > +       if (iocb->ki_pos + iomi.len > i_size_read(iomi.inode))
> > +               return -ENOTBLK;
> > +       if (IS_ENCRYPTED(iomi.inode))
> > +               return -ENOTBLK;
> > +
> > +       ret = kiocb_write_and_wait(iocb, iomi.len);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (iocb->ki_flags & IOCB_NOWAIT)
> > +               iomi.flags |= IOMAP_NOWAIT;
> > +
> > +       inode_dio_begin(iomi.inode);
> > +       ret = begin(iomi.inode, iomi.pos, iomi.len, iomi.flags, &iomi.iomap,
> > +                       &iomi.srcmap);
> > +       if (!ret)
> > +               ret = __iomap_dio_simple(iocb, iter, &iomi);
> > +       if (ret <= 0 && ret != -EIOCBQUEUED)
> > +               inode_dio_end(iomi.inode);
> 
> afaict, the inode_dio_end() error handling has to be done by
> __iomap_dio_simple() to properly account for the synchronous case. I
> ran into this earlier when doing the iomap_next_fn inlining. For the
> synchronous case, __iomap_dio_simple() calls
> iomap_dio_simple_complete() which calls inode_dio_end() even on
> errors.

Yes, there is an issue with the handling of the inode_dio_end event in the
synchronization scenario.
How about this ?

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 3bdfbd0efa076..f6c8091a65ac9 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -994,18 +994,22 @@ ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
 
         if (iomi->iomap.type != IOMAP_MAPPED ||
             iomi->iomap.offset + iomi->iomap.length < iomi->pos + iomi->len ||
-            (iomi->iomap.flags & IOMAP_F_INTEGRITY))
-                return -ENOTBLK;
+            (iomi->iomap.flags & IOMAP_F_INTEGRITY)) {
+                ret = -ENOTBLK;
+                goto out_dio_end;
+        }
 
         alignment = iomap_dio_alignment(iomi->inode, iomi->iomap.bdev, 0);
-        if ((iomi->pos | iomi->len) & (alignment - 1))
-                return -EINVAL;
+        if ((iomi->pos | iomi->len) & (alignment - 1)) {
+                ret = -EINVAL;
+                goto out_dio_end;
+        }
 
         if (unlikely(!iomi->inode->i_sb->s_dio_done_wq &&
                         !is_sync_kiocb(iocb))) {
                 ret = sb_init_dio_done_wq(iomi->inode->i_sb);
                 if (ret < 0)
-                        return ret;
+                        goto out_dio_end;
         }
 
         trace_iomap_dio_rw_begin(iocb, iter, 0, 0);
@@ -1013,8 +1017,10 @@ ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
         bio = bio_alloc_bioset(iomi->iomap.bdev,
                                bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS),
                                REQ_OP_READ, gfp, &iomap_dio_simple_pool);
-        if (!bio)
-                return -EAGAIN;
+        if (!bio) {
+                ret = -EAGAIN;
+                goto out_dio_end;
+        }
         sr = container_of(bio, struct iomap_dio_simple, bio);
         sr->iocb = iocb;
         sr->dio_flags = 0;
@@ -1060,6 +1066,8 @@ ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
         bio_release_pages(bio, false);
 out_bio_put:
         bio_put(bio);
+out_dio_end:
+        inode_dio_end(iomi->inode);
         return ret;
 }
 EXPORT_SYMBOL_GPL(__iomap_dio_simple);
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 810dc4b8dc94d..3cdf475059048 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -732,11 +732,11 @@ static __always_inline ssize_t iomap_dio_read_simple(struct kiocb *iocb,
         inode_dio_begin(iomi.inode);
         ret = begin(iomi.inode, iomi.pos, iomi.len, iomi.flags, &iomi.iomap,
                         &iomi.srcmap);
-        if (!ret)
-                ret = __iomap_dio_simple(iocb, iter, &iomi);
-        if (ret <= 0 && ret != -EIOCBQUEUED)
+        if (ret) {
                 inode_dio_end(iomi.inode);
-        return ret;
+                return ret;
+        }
+        return __iomap_dio_simple(iocb, iter, &iomi);
 }
 
 #ifdef CONFIG_SWAP
> 
> Thanks,
> Joanne
> 
> > +       return ret;
> > +}
> 

Re: [PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw

[Christoph Hellwig]

On Wed, Jul 22, 2026 at 12:03:49PM -0700, Joanne Koong wrote:
> > +ssize_t __iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
> > +               struct iomap_iter *iomi);
> 
> Might be nice to have _read_ in this name as well, eg __iomap_dio_read_simple

Yeah.  I wanted the read out of the guts of the implementation, as
most of the code is not strictly limited to reads.  But with the public
API needing it, we might as well be consistent in the function naming.

> > +       if (!ret)
> > +               ret = __iomap_dio_simple(iocb, iter, &iomi);
> > +       if (ret <= 0 && ret != -EIOCBQUEUED)
> > +               inode_dio_end(iomi.inode);
> 
> afaict, the inode_dio_end() error handling has to be done by
> __iomap_dio_simple() to properly account for the synchronous case. I
> ran into this earlier when doing the iomap_next_fn inlining. For the
> synchronous case, __iomap_dio_simple() calls
> iomap_dio_simple_complete() which calls inode_dio_end() even on
> errors.

Yes.  Sashiko also reported that yesterday and I replied with a patch,
but I now noticed the report and thus my reply only went to me,
a sashiko-review list and Ted, but not everone else or the lists.

WTF?

Re: [PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw

[Christoph Hellwig]

On Thu, Jul 23, 2026 at 10:17:49AM +0800, changfengnan wrote:
> Yes, there is an issue with the handling of the inode_dio_end event in the
> synchronization scenario.
> How about this ?

Yes, minus the label naming that is exactly what I came up with
yesterday and it has survived testing over night.  Looks like xfstests
currently never triggers the sychronous error return path, I'll see
if I can increase coverage for that.

[PATCH 2/3] iomap: decouple simple direct I/O reads from iomap_dio_rw

[Christoph Hellwig]

The pending iomap_iter_next conversion creates performance issues for the
new simple direct I/O read fast path, because it assumes a model where
the iterator must be advanced at the end, which the direct I/O read fast
path tries to avoid.

Side step this by splitting the simple path from iomap_dio_rw, and
require the file systems to call into it explicitly, and pass only a
->begin callback.  This allows to drop various checks for incompatible
features while creating a requirement for the file system to only call
the simple path for cases that it can handle.

As a side-benefit we can now inline the initial part of the simple
direct I/O read fast path and let the compiler convert the indirect call
to ->begin into a direct call.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/ext4/ext4.h        |   3 +
 fs/ext4/file.c        |   4 +-
 fs/ext4/inode.c       |   2 +-
 fs/iomap/direct-io.c  | 208 ++++++++++--------------------------------
 fs/xfs/xfs_file.c     |  14 +--
 fs/xfs/xfs_iomap.c    |   2 +-
 fs/xfs/xfs_iomap.h    |   4 +
 include/linux/iomap.h |  66 ++++++++++++++
 8 files changed, 136 insertions(+), 167 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b37c136ea3ab..e134c0193e2b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4007,6 +4007,9 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
 extern const struct iomap_ops ext4_iomap_ops;
 extern const struct iomap_ops ext4_iomap_report_ops;
 
+int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+		unsigned flags, struct iomap *iomap, struct iomap *srcmap);
+
 static inline int ext4_buffer_uptodate(struct buffer_head *bh)
 {
 	/*
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index eb1a323962b1..f20d92255546 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -91,7 +91,9 @@ 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, 0, NULL, 0);
+	ret = iomap_dio_read_simple(iocb, to, ext4_iomap_begin);
+	if (ret == -ENOTBLK)
+		ret = iomap_dio_rw(iocb, to, &ext4_iomap_ops, NULL, 0, NULL, 0);
 	inode_unlock_shared(inode);
 
 	file_accessed(iocb->ki_filp);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce99807c5f5b..b48c54f3312b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3771,7 +3771,7 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
 }
 
 
-static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 		unsigned flags, struct iomap *iomap, struct iomap *srcmap)
 {
 	int ret;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index ca790239e5eb..36c976cf0848 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -894,6 +894,21 @@ __iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 }
 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,
+		unsigned int dio_flags, void *private, size_t done_before)
+{
+	struct iomap_dio *dio;
+
+	dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, private,
+			     done_before);
+	if (IS_ERR_OR_NULL(dio))
+		return PTR_ERR_OR_ZERO(dio);
+	return iomap_dio_complete(dio);
+}
+EXPORT_SYMBOL_GPL(iomap_dio_rw);
+
 struct iomap_dio_simple {
 	struct kiocb		*iocb;
 	size_t			size;
@@ -968,211 +983,88 @@ static void iomap_dio_simple_end_io(struct bio *bio)
 	iocb->ki_complete(iocb, iomap_dio_simple_complete(sr));
 }
 
-static inline bool
-iomap_dio_simple_supported(struct kiocb *iocb, struct iov_iter *iter,
-			   const struct iomap_dio_ops *dops,
-			   unsigned int dio_flags, size_t done_before)
-{
-	struct inode *inode = file_inode(iocb->ki_filp);
-	size_t count = iov_iter_count(iter);
-
-	if (dops || done_before)
-		return false;
-	if (iov_iter_rw(iter) != READ)
-		return false;
-	if (!count)
-		return false;
-	/*
-	 * Simple dio is an optimization for small IO. Filter out large IO
-	 * early as it's the most common case to fail for typical direct IO
-	 * workloads.
-	 */
-	if (count > inode->i_sb->s_blocksize)
-		return false;
-	if (dio_flags & (IOMAP_DIO_FORCE_WAIT | IOMAP_DIO_PARTIAL |
-			 IOMAP_DIO_BOUNCE))
-		return false;
-	if (iocb->ki_pos + count > i_size_read(inode))
-		return false;
-	if (IS_ENCRYPTED(inode))
-		return false;
-
-	return true;
-}
-
-/*
- * Fast path for small, block-aligned direct I/Os that map to a single
- * contiguous on-disk extent.
- *
- * iomap_dio_simple_supported() enforces the cheap up-front constraints before
- * entering this path.
- *
- * @dops must be NULL: a non-NULL @dops means the caller wants its
- * ->end_io / ->submit_io hooks invoked, and in particular wants its bios to be
- * allocated from the filesystem-private @dops->bio_set (whose front_pad sizes a
- * filesystem-private wrapper around the bio).  The fast path instead allocates
- * from the shared iomap_dio_simple_pool, whose front_pad matches struct
- * iomap_dio_simple; the two wrappers are not interchangeable, so we must fall
- * back to __iomap_dio_rw() in that case.
- *
- * @done_before must be zero: a non-zero caller-accumulated residual cannot be
- * carried through a single-bio inline completion.
- *
- * @iter must describe a non-empty READ no larger than the inode block size:
- * writes, zero-length I/O, and larger requests need the generic iomap direct
- * I/O path.
- *
- * @dio_flags must not request IOMAP_DIO_FORCE_WAIT, IOMAP_DIO_PARTIAL, or
- * IOMAP_DIO_BOUNCE: this path does not support forced waiting, partial direct
- * I/O, or bouncing.  The range must also stay within i_size and encrypted
- * inodes must use the generic iomap direct I/O path.
- *
- * -ENOTBLK is the private sentinel returned by iomap_dio_simple() when it
- * decides the request does not fit the fast path.  In that case we proceed to
- * the generic __iomap_dio_rw() slow path.  Any other errno is a real result and
- * is propagated as-is, in particular -EAGAIN for IOCB_NOWAIT must reach the
- * caller.
- */
-static ssize_t
-iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
-		 const struct iomap_ops *ops, void *private,
-		 unsigned int dio_flags)
+ssize_t __iomap_dio_read_simple(struct kiocb *iocb, struct iov_iter *iter,
+		struct iomap_iter *iomi)
 {
-	struct inode *inode = file_inode(iocb->ki_filp);
-	size_t count = iov_iter_count(iter);
-	bool wait_for_completion = is_sync_kiocb(iocb);
-	struct iomap_iter iomi = {
-		.inode		= inode,
-		.pos		= iocb->ki_pos,
-		.len		= count,
-		.flags		= IOMAP_DIRECT,
-		.private	= private,
-	};
 	struct iomap_dio_simple *sr;
 	unsigned int alignment;
 	struct bio *bio;
 	ssize_t ret;
 
-	if (iocb->ki_flags & IOCB_NOWAIT)
-		iomi.flags |= IOMAP_NOWAIT;
-
-	ret = kiocb_write_and_wait(iocb, count);
-	if (ret)
-		return ret;
-
-	inode_dio_begin(inode);
-
-	ret = ops->iomap_begin(inode, iomi.pos, count, iomi.flags,
-			       &iomi.iomap, &iomi.srcmap);
-	if (ret) {
-		inode_dio_end(inode);
-		return ret;
-	}
-
-	if (iomi.iomap.type != IOMAP_MAPPED ||
-	    iomi.iomap.offset + iomi.iomap.length < iomi.pos + count ||
-	    (iomi.iomap.flags & IOMAP_F_INTEGRITY)) {
+	if (iomi->iomap.type != IOMAP_MAPPED ||
+	    iomi->iomap.offset + iomi->iomap.length < iomi->pos + iomi->len ||
+	    (iomi->iomap.flags & IOMAP_F_INTEGRITY)) {
 		ret = -ENOTBLK;
-		goto out_iomap_end;
+		goto out_dio_end;
 	}
 
-	alignment = iomap_dio_alignment(inode, iomi.iomap.bdev, dio_flags);
-	if ((iomi.pos | count) & (alignment - 1)) {
+	alignment = iomap_dio_alignment(iomi->inode, iomi->iomap.bdev, 0);
+	if ((iomi->pos | iomi->len) & (alignment - 1)) {
 		ret = -EINVAL;
-		goto out_iomap_end;
+		goto out_dio_end;
 	}
 
-	if (!wait_for_completion && unlikely(!inode->i_sb->s_dio_done_wq)) {
-		ret = sb_init_dio_done_wq(inode->i_sb);
+	if (unlikely(!iomi->inode->i_sb->s_dio_done_wq &&
+			!is_sync_kiocb(iocb))) {
+		ret = sb_init_dio_done_wq(iomi->inode->i_sb);
 		if (ret < 0)
-			goto out_iomap_end;
+			goto out_dio_end;
 	}
 
-	trace_iomap_dio_rw_begin(iocb, iter, dio_flags, 0);
-
-	if (user_backed_iter(iter))
-		dio_flags |= IOMAP_DIO_USER_BACKED;
+	trace_iomap_dio_rw_begin(iocb, iter, 0, 0);
 
-	bio = bio_alloc_bioset(iomi.iomap.bdev,
+	bio = bio_alloc_bioset(iomi->iomap.bdev,
 			       bio_iov_vecs_to_alloc(iter, BIO_MAX_VECS),
 			       REQ_OP_READ, GFP_KERNEL, &iomap_dio_simple_pool);
 	sr = container_of(bio, struct iomap_dio_simple, bio);
 	sr->iocb = iocb;
-	sr->dio_flags = dio_flags;
+	sr->dio_flags = 0;
 
-	bio->bi_iter.bi_sector = iomap_sector(&iomi.iomap, iomi.pos);
+	bio->bi_iter.bi_sector = iomap_sector(&iomi->iomap, iomi->pos);
 	bio->bi_ioprio = iocb->ki_ioprio;
 
 	ret = bio_iov_iter_get_pages(bio, iter, alignment - 1);
 	if (unlikely(ret))
 		goto out_bio_put;
 
-	if (bio->bi_iter.bi_size != count) {
+	if (bio->bi_iter.bi_size != iomi->len) {
 		iov_iter_revert(iter, bio->bi_iter.bi_size);
 		ret = -ENOTBLK;
 		goto out_bio_release_pages;
 	}
 
 	sr->size = bio->bi_iter.bi_size;
-
-	if (dio_flags & IOMAP_DIO_USER_BACKED)
+	if (user_backed_iter(iter)) {
 		bio_set_pages_dirty(bio);
+		sr->dio_flags |= IOMAP_DIO_USER_BACKED;
+	}
 
 	if (iocb->ki_flags & IOCB_NOWAIT)
 		bio->bi_opf |= REQ_NOWAIT;
-	if ((iocb->ki_flags & IOCB_HIPRI) && !wait_for_completion) {
-		bio->bi_opf |= REQ_POLLED;
-		WRITE_ONCE(iocb->private, bio);
-	}
 
-	if (ops->iomap_end)
-		ops->iomap_end(inode, iomi.pos, count, count, iomi.flags,
-			       &iomi.iomap);
-
-	if (!wait_for_completion) {
-		bio->bi_end_io = iomap_dio_simple_end_io;
-		submit_bio(bio);
-		trace_iomap_dio_rw_queued(inode, iomi.pos, count);
-		return -EIOCBQUEUED;
+	if (is_sync_kiocb(iocb)) {
+		submit_bio_wait(bio);
+		return iomap_dio_simple_complete(sr);
 	}
 
-	submit_bio_wait(bio);
-	return iomap_dio_simple_complete(sr);
+	if ((iocb->ki_flags & IOCB_HIPRI)) {
+		bio->bi_opf |= REQ_POLLED;
+		WRITE_ONCE(iocb->private, bio);
+	}
+	bio->bi_end_io = iomap_dio_simple_end_io;
+	submit_bio(bio);
+	trace_iomap_dio_rw_queued(iomi->inode, iocb->ki_pos, iomi->len);
+	return -EIOCBQUEUED;
 
 out_bio_release_pages:
 	bio_release_pages(bio, false);
 out_bio_put:
 	bio_put(bio);
-out_iomap_end:
-	if (ops->iomap_end)
-		ops->iomap_end(inode, iomi.pos, count, 0, iomi.flags,
-			       &iomi.iomap);
-	inode_dio_end(inode);
+out_dio_end:
+	inode_dio_end(iomi->inode);
 	return ret;
 }
-
-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, void *private, size_t done_before)
-{
-	struct iomap_dio *dio;
-	ssize_t ret;
-
-	if (iomap_dio_simple_supported(iocb, iter, dops, dio_flags,
-				       done_before)) {
-		ret = iomap_dio_simple(iocb, iter, ops, private, dio_flags);
-		if (ret != -ENOTBLK)
-			return ret;
-	}
-
-	dio = __iomap_dio_rw(iocb, iter, ops, dops, dio_flags, private,
-			     done_before);
-	if (IS_ERR_OR_NULL(dio))
-		return PTR_ERR_OR_ZERO(dio);
-	return iomap_dio_complete(dio);
-}
-EXPORT_SYMBOL_GPL(iomap_dio_rw);
+EXPORT_SYMBOL_GPL(__iomap_dio_read_simple);
 
 static int __init iomap_dio_init(void)
 {
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 845a97c9b063..b733225d2864 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -251,8 +251,6 @@ xfs_file_dio_read(
 	struct iov_iter		*to)
 {
 	struct xfs_inode	*ip = XFS_I(file_inode(iocb->ki_filp));
-	unsigned int		dio_flags = 0;
-	const struct iomap_dio_ops *dio_ops = NULL;
 	ssize_t			ret;
 
 	trace_xfs_file_direct_read(iocb, to);
@@ -266,11 +264,15 @@ xfs_file_dio_read(
 	if (ret)
 		return ret;
 	if (mapping_stable_writes(iocb->ki_filp->f_mapping)) {
-		dio_ops = &xfs_dio_read_bounce_ops;
-		dio_flags |= IOMAP_DIO_BOUNCE;
+		ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops,
+				&xfs_dio_read_bounce_ops, IOMAP_DIO_BOUNCE,
+				NULL, 0);
+	} else {
+		ret = iomap_dio_read_simple(iocb, to, xfs_read_iomap_begin);
+		if (ret == -ENOTBLK)
+			ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, NULL,
+					0, NULL, 0);
 	}
-	ret = iomap_dio_rw(iocb, to, &xfs_read_iomap_ops, dio_ops, dio_flags,
-			NULL, 0);
 	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
 
 	return ret;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 225c3de88d03..0536e2aeddcc 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -2173,7 +2173,7 @@ const struct iomap_ops xfs_buffered_write_iomap_ops = {
 	.iomap_end		= xfs_buffered_write_iomap_end,
 };
 
-static int
+int
 xfs_read_iomap_begin(
 	struct inode		*inode,
 	loff_t			offset,
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index ebcce7d49446..cffcec532ea6 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -49,6 +49,10 @@ xfs_aligned_fsb_count(
 	return count_fsb;
 }
 
+int xfs_read_iomap_begin(struct inode *inode, loff_t offset,
+		loff_t length, unsigned flags, struct iomap *iomap,
+		struct iomap *srcmap);
+
 extern const struct iomap_ops xfs_buffered_write_iomap_ops;
 extern const struct iomap_ops xfs_direct_write_iomap_ops;
 extern const struct iomap_ops xfs_zoned_direct_write_iomap_ops;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 3df851d0f76b..345e5425f978 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -10,6 +10,7 @@
 #include <linux/mm_types.h>
 #include <linux/blkdev.h>
 #include <linux/folio_batch.h>
+#include <linux/pagemap.h>
 
 struct address_space;
 struct fiemap_extent_info;
@@ -674,6 +675,71 @@ struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 ssize_t iomap_dio_complete(struct iomap_dio *dio);
 void iomap_dio_bio_end_io(struct bio *bio);
 
+/*
+ * Fast path for small, block-aligned direct I/Os that map to a single
+ * contiguous on-disk extent.
+ *
+ * @iter must describe a non-empty READ no larger than the inode block size:
+ * writes, zero-length I/O, and larger requests need the generic iomap direct
+ * I/O path.
+ *
+ * Does not support iomap_dio_ops, dio_flags, done_before or private data.
+ * The range must also stay within i_size and encrypted inodes must use the
+ * generic iomap direct I/O path.
+ *
+ * -ENOTBLK inidicates the generic path must be used by the caller instead.
+ * Any other errno is a real result and is propagated as-is, in particular
+ * -EAGAIN for IOCB_NOWAIT must reach the caller.
+ *
+ * The caller can only provide an iomap begin handler, and the iterator
+ * is never advanced.
+ */
+ssize_t __iomap_dio_read_simple(struct kiocb *iocb, struct iov_iter *iter,
+		struct iomap_iter *iomi);
+static __always_inline ssize_t iomap_dio_read_simple(struct kiocb *iocb,
+		struct iov_iter *iter, iomap_iter_begin_fn begin)
+{
+	struct iomap_iter iomi = {
+		.inode		= file_inode(iocb->ki_filp),
+		.pos		= iocb->ki_pos,
+		.len		= iov_iter_count(iter),
+		.flags		= IOMAP_DIRECT,
+	};
+	ssize_t ret;
+
+	if (!iomi.len)
+		return 0;
+
+	/*
+	 * Simple dio is an optimization for small IO. Filter out large IO
+	 * early as it's the most common case to fail for typical direct IO
+	 * workloads.
+	 */
+	if (iomi.len > iomi.inode->i_sb->s_blocksize)
+		return -ENOTBLK;
+	if (iocb->ki_pos + iomi.len > i_size_read(iomi.inode))
+		return -ENOTBLK;
+	if (IS_ENCRYPTED(iomi.inode))
+		return -ENOTBLK;
+
+	ret = kiocb_write_and_wait(iocb, iomi.len);
+	if (ret)
+		return ret;
+
+	if (iocb->ki_flags & IOCB_NOWAIT)
+		iomi.flags |= IOMAP_NOWAIT;
+
+	inode_dio_begin(iomi.inode);
+	ret = begin(iomi.inode, iomi.pos, iomi.len, iomi.flags, &iomi.iomap,
+			&iomi.srcmap);
+	if (ret) {
+		inode_dio_end(iomi.inode);
+		return ret;
+	}
+
+	return __iomap_dio_read_simple(iocb, iter, &iomi);
+}
+
 #ifdef CONFIG_SWAP
 struct file;
 struct swap_info_struct;
-- 
2.53.0