[RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Joanne Koong]

This is submitted to get some feedback on what converting to an in-iter
->iomap_next() model would look like. It revives Matthew's previous RFC [1],
which had the same goal.

The series merges ->iomap_begin()/->iomap_end() into a single ->iomap_next()
callback (patches 1-2) and shows an example of devirtualizing the callbacks on
a hot path so the indirect call through struct iomap_ops is avoided (patch 3).
This series is on top of vfs.all (commit e7a6d06e3c3e8).

A few questions:
* is this roughly the in-iter direction you had in mind?
* is removing the indirect call still worth it? My understanding is that
  indirect calls are cheap on modern eIBRS hardware and the conversion adds
  some per-filesystem boilerplate, so I'm unsure if it carries its weight. If
  not, do you think the in-iter model is still worth having on its own?

Thanks,
Joanne

[1] https://lore.kernel.org/linux-fsdevel/20200728173216.7184-1-willy@infradead.org/T/#u

Joanne Koong (3):
  iomap: add ->iomap_next() and iomap_process() helper
  xfs: convert read and buffered write iomap ops to ->iomap_next()
  xfs: example of devirtualizing buffered write iomap callbacks

 fs/iomap/buffered-io.c |  3 +-
 fs/iomap/iter.c        | 99 ++++++++++++++++++++++++++++++++++--------
 fs/xfs/xfs_file.c      | 26 +++++++++--
 fs/xfs/xfs_iomap.c     | 29 ++++++++++---
 fs/xfs/xfs_iomap.h     |  7 +++
 include/linux/iomap.h  | 98 ++++++++++++++++++++++++++++++++++++++---
 6 files changed, 231 insertions(+), 31 deletions(-)

-- 
2.52.0

[RFC PATCH v1 1/3] iomap: add ->iomap_next() and iomap_process() helper

[Joanne Koong]

Have one ->iomap_next() callback instead of ->iomap_begin() and
->iomap_end(). ->iomap_next() finishes the previous mapping if needed,
and produces the next mapping. This lets performance-critical callers
inline the iteration with a fixed callback, which the compiler is able
to call directly instead of indirectly.

iomap_iter() uses ->iomap_next() when the filesystem provides that
callback and otherwise falls back to the ->iomap_begin()/->iomap_end()
path, so filesystems can be converted one at a time.

Add a iomap_process() inline helper that does most of the logic needed
in an ->iomap_next() implementation.

Suggested-by: Christoph Hellwig <hch@lst.de>
Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/iomap/iter.c       | 96 +++++++++++++++++++++++++++++++++++--------
 include/linux/iomap.h | 51 ++++++++++++++++++++---
 2 files changed, 126 insertions(+), 21 deletions(-)

diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
index e4a29829591a..2d5469996a51 100644
--- a/fs/iomap/iter.c
+++ b/fs/iomap/iter.c
@@ -39,22 +39,7 @@ static inline void iomap_iter_done(struct iomap_iter *iter)
 		trace_iomap_iter_srcmap(iter->inode, &iter->srcmap);
 }
 
-/**
- * iomap_iter - iterate over a ranges in a file
- * @iter: iteration structue
- * @ops: iomap ops provided by the file system
- *
- * 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)
+static int iomap_iter_legacy(struct iomap_iter *iter, const struct iomap_ops *ops)
 {
 	bool stale = iter->iomap.flags & IOMAP_F_STALE;
 	ssize_t advanced;
@@ -114,3 +99,82 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
 	iomap_iter_done(iter);
 	return 1;
 }
+
+static int iomap_iter_next(struct iomap_iter *iter, const struct iomap_ops *ops)
+{
+	int ret;
+
+	trace_iomap_iter(iter, ops, _RET_IP_);
+
+	ret = ops->iomap_next(iter, &iter->iomap, &iter->srcmap);
+	if (ret > 0) {
+		iter->status = 0;
+		iomap_iter_done(iter);
+	}
+
+	return ret;
+}
+
+/**
+ * iomap_iter - iterate over a ranges in a file
+ * @iter: iteration structue
+ * @ops: iomap ops provided by the file system
+ *
+ * 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)
+{
+	if (ops->iomap_next)
+		return iomap_iter_next(iter, ops);
+
+	return iomap_iter_legacy(iter, 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 = iter->pos - iter->iter_start_pos;
+
+	if (!iomap->length)
+		return 1;
+
+	/*
+	 * Use iter->len to determine whether to continue onto the next mapping.
+	 * Explicitly terminate on error status or if the current iter has not
+	 * 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 (ret < 0 && !advanced)
+		return ret;
+
+	if (iter->status < 0)
+		ret = iter->status;
+	else if (iter->len == 0 || (!advanced && !stale))
+		ret = 0;
+	else
+		ret = 1;
+
+	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(iomap, 0, sizeof(*iomap));
+	memset(srcmap, 0, sizeof(*srcmap));
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iomap_iter_continue);
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 3582ed1fe236..335a3858601c 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -212,15 +212,19 @@ struct iomap_write_ops {
 #define IOMAP_ATOMIC		(1 << 9) /* torn-write protection */
 #define IOMAP_DONTCACHE		(1 << 10)
 
+typedef int (*iomap_begin_fn)(struct inode *inode, loff_t pos, loff_t length,
+		unsigned flags, struct iomap *iomap, struct iomap *srcmap);
+
+typedef int (*iomap_end_fn)(struct inode *inode, loff_t pos, loff_t length,
+		ssize_t written, unsigned flags, struct iomap *iomap);
+
 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);
+	iomap_begin_fn iomap_begin;
 
 	/*
 	 * Commit and/or unreserve space previous allocated using iomap_begin.
@@ -228,8 +232,15 @@ struct iomap_ops {
 	 * 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);
+	iomap_end_fn iomap_end;
+
+	/*
+	 * Produce the next mapping (finishing the previous one if needed).
+	 * Return 1 to continue iterating, 0 if the range is fully consumed,
+	 * or a negative error on failure.
+	 */
+	int (*iomap_next)(const struct iomap_iter *iter, struct iomap *iomap,
+			struct iomap *srcmap);
 };
 
 /**
@@ -317,6 +328,9 @@ 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);
+
 /*
  * Return the file offset for the first unchanged block after a short write.
  *
@@ -648,4 +662,31 @@ static inline void iomap_bio_readahead(struct readahead_control *rac,
 }
 #endif /* CONFIG_BLOCK */
 
+static __always_inline int iomap_process(const struct iomap_iter *iter,
+		struct iomap *iomap, struct iomap *srcmap,
+		iomap_begin_fn begin, iomap_end_fn end)
+{
+	int ret = 0;
+
+	if (iomap->length && 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;
+}
+
 #endif /* LINUX_IOMAP_H */
-- 
2.52.0

Re: [RFC PATCH v1 1/3] iomap: add ->iomap_next() and iomap_process() helper

[Christoph Hellwig]

> +	/*
> +	 * Produce the next mapping (finishing the previous one if needed).
> +	 * Return 1 to continue iterating, 0 if the range is fully consumed,
> +	 * or a negative error on failure.
> +	 */
> +	int (*iomap_next)(const struct iomap_iter *iter, struct iomap *iomap,
> +			struct iomap *srcmap);

If you add function typedefs for all the other two methods I'd one one here
as well for consistency.  We'll need it anywone once the legacy ops are
entirely gone we can kill of the ops and just pass this function pointer.

As for the arguments:  I don't really see much of a point in passing
the iomap/scrmap separately vs just the iter.  Or am I missing something
here?

Re: [RFC PATCH v1 1/3] iomap: add ->iomap_next() and iomap_process() helper

[Joanne Koong]

On Thu, Jun 25, 2026 at 5:56 AM Christoph Hellwig <hch@lst.de> wrote:
>
> > +     /*
> > +      * Produce the next mapping (finishing the previous one if needed).
> > +      * Return 1 to continue iterating, 0 if the range is fully consumed,
> > +      * or a negative error on failure.
> > +      */
> > +     int (*iomap_next)(const struct iomap_iter *iter, struct iomap *iomap,
> > +                     struct iomap *srcmap);
>
> If you add function typedefs for all the other two methods I'd one one here
> as well for consistency.  We'll need it anywone once the legacy ops are
> entirely gone we can kill of the ops and just pass this function pointer.

Sounds good, I'll do this in v2.

>
> As for the arguments:  I don't really see much of a point in passing
> the iomap/scrmap separately vs just the iter.  Or am I missing something
> here?

It was meant to safeguard against a callback modifying iomap-internal
non-mapping fields (iter->pos/len/status/iter_start_pos) but maybe
this is unnecessary. I'll drop the 2 args and make it non-const.

Thanks,
Joanne

Re: [RFC PATCH v1 1/3] iomap: add ->iomap_next() and iomap_process() helper

[Christoph Hellwig]

On Thu, Jun 25, 2026 at 02:03:13PM -0700, Joanne Koong wrote:
> > As for the arguments:  I don't really see much of a point in passing
> > the iomap/scrmap separately vs just the iter.  Or am I missing something
> > here?
> 
> It was meant to safeguard against a callback modifying iomap-internal
> non-mapping fields (iter->pos/len/status/iter_start_pos) but maybe
> this is unnecessary. I'll drop the 2 args and make it non-const.

As in my reply to Gao I missed the constness point.  So I think keeping
the current version should be ok.  I'm a little worried that we might
end up with something not beeing const in the end like the iter private
data, though.

[RFC PATCH v1 2/3] xfs: convert read and buffered write iomap ops to ->iomap_next()

[Joanne Koong]

Convert two xfs iomap_ops to the new ->iomap_next() callback as
examples (xfs_read_iomap_ops (no ->iomap_end()) and
xfs_buffered_write_iomap_ops (with ->iomap_end()). Both use the
iomap_process() helper, which finishes the previous mapping if needed
and produces the next one. No functional changes are intended.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/xfs/xfs_iomap.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index f20a02f49ed9..3b212bfe04d7 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -2168,9 +2168,19 @@ xfs_buffered_write_iomap_end(
 	return 0;
 }
 
+static int
+xfs_buffered_write_iomap_next(
+	const struct iomap_iter *iter,
+	struct iomap		*iomap,
+	struct iomap		*srcmap)
+{
+	return iomap_process(iter, iomap, srcmap,
+			xfs_buffered_write_iomap_begin,
+			xfs_buffered_write_iomap_end);
+}
+
 const struct iomap_ops xfs_buffered_write_iomap_ops = {
-	.iomap_begin		= xfs_buffered_write_iomap_begin,
-	.iomap_end		= xfs_buffered_write_iomap_end,
+	.iomap_next		= xfs_buffered_write_iomap_next,
 };
 
 static int
@@ -2214,8 +2224,17 @@ xfs_read_iomap_begin(
 				 shared ? IOMAP_F_SHARED : 0, seq);
 }
 
+static int
+xfs_read_iomap_next(
+	const struct iomap_iter *iter,
+	struct iomap		*iomap,
+	struct iomap		*srcmap)
+{
+	return iomap_process(iter, iomap, srcmap, xfs_read_iomap_begin, NULL);
+}
+
 const struct iomap_ops xfs_read_iomap_ops = {
-	.iomap_begin		= xfs_read_iomap_begin,
+	.iomap_next		= xfs_read_iomap_next,
 };
 
 static int
-- 
2.52.0

[RFC PATCH v1 3/3] xfs: example of devirtualizing buffered write iomap callbacks

[Joanne Koong]

This is an example of what removing the indirect call would look like.
With the inlined iomap_buffered_write_inline() that gets passed
xfs_buffered_write_iomap_begin() and xfs_buffered_write_iomap_end() as
constant callbacks, the compiler is able to call the callbacks directly
and drop the indirect call through struct iomap_ops:

before:
callq  iomap_file_buffered_write
R_X86_64_32S   xfs_buffered_write_iomap_ops

after (direct calls, R_X86_64_PLT32 == direct):
callq  ...  R_X86_64_PLT32  xfs_buffered_write_iomap_begin-0x4
callq  ...  R_X86_64_PLT32  xfs_buffered_write_iomap_end-0x4

Suggested-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/iomap/buffered-io.c |  3 ++-
 fs/iomap/iter.c        |  3 ++-
 fs/xfs/xfs_file.c      | 26 ++++++++++++++++++++---
 fs/xfs/xfs_iomap.c     |  4 ++--
 fs/xfs/xfs_iomap.h     |  7 +++++++
 include/linux/iomap.h  | 47 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 5a107d59ae27..12aa42f6497e 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1153,7 +1153,7 @@ static bool iomap_write_end(struct iomap_iter *iter, size_t len, size_t copied,
 	return __iomap_write_end(iter->inode, pos, len, copied, folio);
 }
 
-static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i,
+int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i,
 		const struct iomap_write_ops *write_ops)
 {
 	ssize_t total_written = 0;
@@ -1259,6 +1259,7 @@ static int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i,
 
 	return total_written ? 0 : status;
 }
+EXPORT_SYMBOL_GPL(iomap_write_iter);
 
 ssize_t
 iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i,
diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
index 2d5469996a51..a3edb16d3488 100644
--- a/fs/iomap/iter.c
+++ b/fs/iomap/iter.c
@@ -25,7 +25,7 @@ int iomap_iter_advance(struct iomap_iter *iter, u64 count)
 	return 0;
 }
 
-static inline void iomap_iter_done(struct iomap_iter *iter)
+void iomap_iter_done(struct iomap_iter *iter)
 {
 	WARN_ON_ONCE(iter->iomap.offset > iter->pos);
 	WARN_ON_ONCE(iter->iomap.length == 0);
@@ -38,6 +38,7 @@ static inline void iomap_iter_done(struct iomap_iter *iter)
 	if (iter->srcmap.type != IOMAP_HOLE)
 		trace_iomap_iter_srcmap(iter->inode, &iter->srcmap);
 }
+EXPORT_SYMBOL_GPL(iomap_iter_done);
 
 static int iomap_iter_legacy(struct iomap_iter *iter, const struct iomap_ops *ops)
 {
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 845a97c9b063..49f5d7f495dd 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1038,6 +1038,7 @@ xfs_file_buffered_write(
 {
 	struct inode		*inode = iocb->ki_filp->f_mapping->host;
 	struct xfs_inode	*ip = XFS_I(inode);
+	struct iomap_iter	iter;
 	ssize_t			ret;
 	bool			cleared_space = false;
 	unsigned int		iolock;
@@ -1053,9 +1054,28 @@ xfs_file_buffered_write(
 		goto out;
 
 	trace_xfs_file_buffered_write(iocb, from);
-	ret = iomap_file_buffered_write(iocb, from,
-			&xfs_buffered_write_iomap_ops, &xfs_iomap_write_ops,
-			NULL);
+
+	/*
+	 * Call inlined iomap buffered write and pass constant begin/end
+	 * function pointers so the compiler can devirtualize them and avoid the
+	 * indirect call through struct iomap_ops.
+	 */
+	iter = (struct iomap_iter){
+		.inode		= inode,
+		.pos		= iocb->ki_pos,
+		.len		= iov_iter_count(from),
+		.flags		= IOMAP_WRITE,
+	};
+	if (iocb->ki_flags & IOCB_NOWAIT)
+		iter.flags |= IOMAP_NOWAIT;
+	if (iocb->ki_flags & IOCB_DONTCACHE)
+		iter.flags |= IOMAP_DONTCACHE;
+
+	ret = iomap_buffered_write_inline(&iter, from,
+			xfs_buffered_write_iomap_begin,
+			xfs_buffered_write_iomap_end, &xfs_iomap_write_ops);
+	if (ret > 0)
+		iocb->ki_pos = iter.pos;
 
 	/*
 	 * If we hit a space limit, try to free up some lingering preallocated
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 3b212bfe04d7..23b1885a1123 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1773,7 +1773,7 @@ xfs_zoned_buffered_write_iomap_begin(
 	return error;
 }
 
-static int
+int
 xfs_buffered_write_iomap_begin(
 	struct inode		*inode,
 	loff_t			offset,
@@ -2124,7 +2124,7 @@ xfs_buffered_write_delalloc_punch(
 			offset, offset + length, iter->private);
 }
 
-static int
+int
 xfs_buffered_write_iomap_end(
 	struct inode		*inode,
 	loff_t			offset,
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index ebcce7d49446..42183e4c461b 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -19,6 +19,13 @@ int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
 xfs_fileoff_t xfs_iomap_eof_align_last_fsb(struct xfs_inode *ip,
 		xfs_fileoff_t end_fsb);
 
+int xfs_buffered_write_iomap_begin(struct inode *inode, loff_t offset,
+		loff_t count, unsigned flags, struct iomap *iomap,
+		struct iomap *srcmap);
+int xfs_buffered_write_iomap_end(struct inode *inode, loff_t offset,
+		loff_t length, ssize_t written, unsigned flags,
+		struct iomap *iomap);
+
 u64 xfs_iomap_inode_sequence(struct xfs_inode *ip, u16 iomap_flags);
 int xfs_bmbt_to_iomap(struct xfs_inode *ip, struct iomap *iomap,
 		struct xfs_bmbt_irec *imap, unsigned int mapping_flags,
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 335a3858601c..60dc977dca55 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -367,6 +367,8 @@ static inline bool iomap_want_unshare_iter(const struct iomap_iter *iter)
 ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
 		const struct iomap_ops *ops,
 		const struct iomap_write_ops *write_ops, void *private);
+int iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i,
+		const struct iomap_write_ops *write_ops);
 int iomap_fsverity_write(struct file *file, loff_t pos, size_t length,
 		const void *buf, const struct iomap_ops *ops,
 		const struct iomap_write_ops *write_ops);
@@ -689,4 +691,49 @@ static __always_inline int iomap_process(const struct iomap_iter *iter,
 	return ret < 0 ? ret : 1;
 }
 
+void iomap_iter_done(struct iomap_iter *iter);
+
+/*
+ * Inline version of the ->iomap_next call for performance-critical callers.
+ * When inlined at a call site that passes constant begin / end function
+ * pointers, the begin and end calls are called directly and avoid the indirect
+ * call through struct iomap_ops.
+ */
+static __always_inline int iomap_iter_inline(struct iomap_iter *iter,
+		iomap_begin_fn begin, iomap_end_fn end)
+{
+	int ret = iomap_process(iter, &iter->iomap, &iter->srcmap, begin, end);
+
+	if (ret > 0) {
+		iter->status = 0;
+		iomap_iter_done(iter);
+	}
+	return ret;
+}
+
+/*
+ * Inline buffered write loop for callers that can supply constant begin and end
+ * callbacks. This lets the compiler devirtualize the mapping calls and drop the
+ * indirect call through struct iomap_ops. The passed-in iter should be
+ * initialized by the caller. This will advance the iter by the number of bytes
+ * that get successfully written.
+ *
+ * This returns the number of bytes written or a negative error.
+ */
+static __always_inline ssize_t iomap_buffered_write_inline(
+		struct iomap_iter *iter, struct iov_iter *i,
+		iomap_begin_fn begin, iomap_end_fn end,
+		const struct iomap_write_ops *write_ops)
+{
+	loff_t start_pos = iter->pos;
+	ssize_t ret;
+
+	while ((ret = iomap_iter_inline(iter, begin, end)) > 0)
+		iter->status = iomap_write_iter(iter, i, write_ops);
+
+	if (iter->pos == start_pos)
+		return ret;
+	return iter->pos - start_pos;
+}
+
 #endif /* LINUX_IOMAP_H */
-- 
2.52.0

Re: [RFC PATCH v1 3/3] xfs: example of devirtualizing buffered write iomap callbacks

[Christoph Hellwig]

On Wed, Jun 24, 2026 at 07:47:23PM -0700, Joanne Koong wrote:
> This is an example of what removing the indirect call would look like.
> With the inlined iomap_buffered_write_inline() that gets passed
> xfs_buffered_write_iomap_begin() and xfs_buffered_write_iomap_end() as
> constant callbacks, the compiler is able to call the callbacks directly
> and drop the indirect call through struct iomap_ops:

Nice.  Btw, a more critical path would be direct I/O.

Fengnan, maybe you can play around with this series and your direct read
fast path?

> +	/*
> +	 * Call inlined iomap buffered write and pass constant begin/end
> +	 * function pointers so the compiler can devirtualize them and avoid the
> +	 * indirect call through struct iomap_ops.
> +	 */
> +	iter = (struct iomap_iter){
> +		.inode		= inode,
> +		.pos		= iocb->ki_pos,
> +		.len		= iov_iter_count(from),
> +		.flags		= IOMAP_WRITE,
> +	};
> +	if (iocb->ki_flags & IOCB_NOWAIT)
> +		iter.flags |= IOMAP_NOWAIT;
> +	if (iocb->ki_flags & IOCB_DONTCACHE)
> +		iter.flags |= IOMAP_DONTCACHE;

We'll probably want a helper for this initializion boilerplate.
Or make it part of iomap_buffered_write_inline somehow?

> +	ret = iomap_buffered_write_inline(&iter, from,
> +			xfs_buffered_write_iomap_begin,
> +			xfs_buffered_write_iomap_end,

So we're not even doing the _next scheme here and already get
results, nice!

> +	if (ret > 0)
> +		iocb->ki_pos = iter.pos;

Similarly here, it would be great if we could somehow avoid
too much boilerplate.

Re: [RFC PATCH v1 3/3] xfs: example of devirtualizing buffered write iomap callbacks

[Joanne Koong]

On Thu, Jun 25, 2026 at 6:00 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Jun 24, 2026 at 07:47:23PM -0700, Joanne Koong wrote:
> > This is an example of what removing the indirect call would look like.
> > With the inlined iomap_buffered_write_inline() that gets passed
> > xfs_buffered_write_iomap_begin() and xfs_buffered_write_iomap_end() as
> > constant callbacks, the compiler is able to call the callbacks directly
> > and drop the indirect call through struct iomap_ops:
>
> Nice.  Btw, a more critical path would be direct I/O.
>
> Fengnan, maybe you can play around with this series and your direct read
> fast path?
>
> > +     /*
> > +      * Call inlined iomap buffered write and pass constant begin/end
> > +      * function pointers so the compiler can devirtualize them and avoid the
> > +      * indirect call through struct iomap_ops.
> > +      */
> > +     iter = (struct iomap_iter){
> > +             .inode          = inode,
> > +             .pos            = iocb->ki_pos,
> > +             .len            = iov_iter_count(from),
> > +             .flags          = IOMAP_WRITE,
> > +     };
> > +     if (iocb->ki_flags & IOCB_NOWAIT)
> > +             iter.flags |= IOMAP_NOWAIT;
> > +     if (iocb->ki_flags & IOCB_DONTCACHE)
> > +             iter.flags |= IOMAP_DONTCACHE;
>
> We'll probably want a helper for this initializion boilerplate.
> Or make it part of iomap_buffered_write_inline somehow?
>
> > +     ret = iomap_buffered_write_inline(&iter, from,
> > +                     xfs_buffered_write_iomap_begin,
> > +                     xfs_buffered_write_iomap_end,
>
> So we're not even doing the _next scheme here and already get
> results, nice!
>
> > +     if (ret > 0)
> > +             iocb->ki_pos = iter.pos;
>
> Similarly here, it would be great if we could somehow avoid
> too much boilerplate.
>

Agreed, I think this can all just be moved into
iomap_buffered_write_inline() and then the call would just be:

  ret = iomap_buffered_write_inline(iocb, from,
                xfs_buffered_write_iomap_begin, xfs_buffered_write_iomap_end,
                &xfs_iomap_write_ops, NULL);

I'll clean this up for v2.

Thanks,
Joanne

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Gao Xiang]

On 2026/6/25 10:47, Joanne Koong wrote:
> This is submitted to get some feedback on what converting to an in-iter
> ->iomap_next() model would look like. It revives Matthew's previous RFC [1],
> which had the same goal.
> 
> The series merges ->iomap_begin()/->iomap_end() into a single ->iomap_next()
> callback (patches 1-2) and shows an example of devirtualizing the callbacks on
> a hot path so the indirect call through struct iomap_ops is avoided (patch 3).
> This series is on top of vfs.all (commit e7a6d06e3c3e8).
> 
> A few questions:
> * is this roughly the in-iter direction you had in mind?
> * is removing the indirect call still worth it? My understanding is that
>    indirect calls are cheap on modern eIBRS hardware and the conversion adds
>    some per-filesystem boilerplate, so I'm unsure if it carries its weight. If
>    not, do you think the in-iter model is still worth having on its own?


As I mentioned a year ago, I really hope this way can be proceed
to avoid iomap iter-callback models:

https://lore.kernel.org/r/20250905152118.GE1587915@frogsfrogsfrogs

And in that way, iomap can be more flexible as a fs IO library.

Thanks,
Gao Xiang

> 
> Thanks,
> Joanne
> 
> [1] https://lore.kernel.org/linux-fsdevel/20200728173216.7184-1-willy@infradead.org/T/#u

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Gao Xiang]

On 2026/6/25 11:25, Gao Xiang wrote:
> 
> 
> On 2026/6/25 10:47, Joanne Koong wrote:
>> This is submitted to get some feedback on what converting to an in-iter
>> ->iomap_next() model would look like. It revives Matthew's previous RFC [1],
>> which had the same goal.
>>
>> The series merges ->iomap_begin()/->iomap_end() into a single ->iomap_next()
>> callback (patches 1-2) and shows an example of devirtualizing the callbacks on
>> a hot path so the indirect call through struct iomap_ops is avoided (patch 3).
>> This series is on top of vfs.all (commit e7a6d06e3c3e8).
>>
>> A few questions:
>> * is this roughly the in-iter direction you had in mind?
>> * is removing the indirect call still worth it? My understanding is that
>>    indirect calls are cheap on modern eIBRS hardware and the conversion adds
>>    some per-filesystem boilerplate, so I'm unsure if it carries its weight. If
>>    not, do you think the in-iter model is still worth having on its own?
> 
> 
> As I mentioned a year ago, I really hope this way can be proceed
> to avoid iomap iter-callback models:
> 
> https://lore.kernel.org/r/20250905152118.GE1587915@frogsfrogsfrogs
> 
> And in that way, iomap can be more flexible as a fs IO library.

IMHO, hhe main point is not to avoid the indirect call to make it
noticeably faster or to avoid security issues due to
Spectre/Meltdown/...

It just gives more flexibility to specific filesystems to control
the IO path better (since each filesystem might have its own
slightly different IO flow), rather than adding various flags/code
in the iomap core itself to fulfill the current callback model.

Thanks,
Gao Xiang

> 
> Thanks,
> Gao Xiang
> 
>>
>> Thanks,
>> Joanne
>>
>> [1] https://lore.kernel.org/linux-fsdevel/20200728173216.7184-1-willy@infradead.org/T/#u

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Christoph Hellwig]

On Thu, Jun 25, 2026 at 11:25:54AM +0800, Gao Xiang wrote:
> As I mentioned a year ago, I really hope this way can be proceed
> to avoid iomap iter-callback models:
>
> https://lore.kernel.org/r/20250905152118.GE1587915@frogsfrogsfrogs

Oh, I guess this is where passing the iomap and srcmap separately
from the iter come from.  But for this to make sense we'd have to
be able to pass a const iter.

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Gao Xiang]

On 2026/6/25 21:02, Christoph Hellwig wrote:
> On Thu, Jun 25, 2026 at 11:25:54AM +0800, Gao Xiang wrote:
>> As I mentioned a year ago, I really hope this way can be proceed
>> to avoid iomap iter-callback models:
>>
>> https://lore.kernel.org/r/20250905152118.GE1587915@frogsfrogsfrogs
> 
> Oh, I guess this is where passing the iomap and srcmap separately
> from the iter come from.  But for this to make sense we'd have to
> be able to pass a const iter.

... anyway, I hope iomap folks could consider my humble
suggestion, since I'm afraid that this callback model in
the long term won't scale well in very slightly different
details (along with more and more iomap users), so rather
than just converting more callers, I guess it might be
better to consider non-callback model as a direction
anyway.

Thanks,
Gao Xiang

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Joanne Koong]

On Thu, Jun 25, 2026 at 6:21 AM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> On 2026/6/25 21:02, Christoph Hellwig wrote:
> > On Thu, Jun 25, 2026 at 11:25:54AM +0800, Gao Xiang wrote:
> >> As I mentioned a year ago, I really hope this way can be proceed
> >> to avoid iomap iter-callback models:
> >>
> >> https://lore.kernel.org/r/20250905152118.GE1587915@frogsfrogsfrogs
> >
> > Oh, I guess this is where passing the iomap and srcmap separately
> > from the iter come from.  But for this to make sense we'd have to
> > be able to pass a const iter.
>
> ... anyway, I hope iomap folks could consider my humble
> suggestion, since I'm afraid that this callback model in
> the long term won't scale well in very slightly different
> details (along with more and more iomap users), so rather
> than just converting more callers, I guess it might be
> better to consider non-callback model as a direction
> anyway.

Thanks Gao. I don't think the callback and helper models are as far
apart as it might look though after the ->iomap_next() change. Once
all the callers have been converted, there'll be no iomap_ops
indirection left and callers can just pass in the direct function
pointer for iomap_next(). You had mentioned something about having
callers own the iter and drive the loop to get rid of indirect calls -
I think the inline patch in this series does what you had in mind.

I think the difference is that under your proposal, the loop body
logic for pre/postprocessing the read/write/etc ranges is also moved
out into helper functions, which each caller would then have to put
together / reassemble. It seems like a lot of duplicate code and I'm
not sure I see the benefit of open-coding that when none of the
callers right now need special logic interjected between the
fine-grained helpers and and their callback. imo it seems more
advantageous to only add that in the future if there's a caller that
has that special need, where only that caller would need to do that,
instead of making all callers have to do that.

Thanks,
Joanne

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Gao Xiang]

Hi Joanne,

On 2026/6/26 05:33, Joanne Koong wrote:
> On Thu, Jun 25, 2026 at 6:21 AM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>>
>> On 2026/6/25 21:02, Christoph Hellwig wrote:
>>> On Thu, Jun 25, 2026 at 11:25:54AM +0800, Gao Xiang wrote:
>>>> As I mentioned a year ago, I really hope this way can be proceed
>>>> to avoid iomap iter-callback models:
>>>>
>>>> https://lore.kernel.org/r/20250905152118.GE1587915@frogsfrogsfrogs
>>>
>>> Oh, I guess this is where passing the iomap and srcmap separately
>>> from the iter come from.  But for this to make sense we'd have to
>>> be able to pass a const iter.
>>
>> ... anyway, I hope iomap folks could consider my humble
>> suggestion, since I'm afraid that this callback model in
>> the long term won't scale well in very slightly different
>> details (along with more and more iomap users), so rather
>> than just converting more callers, I guess it might be
>> better to consider non-callback model as a direction
>> anyway.
> 
> Thanks Gao. I don't think the callback and helper models are as far
> apart as it might look though after the ->iomap_next() change. Once
> all the callers have been converted, there'll be no iomap_ops
> indirection left and callers can just pass in the direct function
> pointer for iomap_next(). You had mentioned something about having
> callers own the iter and drive the loop to get rid of indirect calls -
> I think the inline patch in this series does what you had in mind.

Thanks for the reply.

I partially agree since it's much closer, but what I meant is
the design model difference as below.

> 
> I think the difference is that under your proposal, the loop body
> logic for pre/postprocessing the read/write/etc ranges is also moved
> out into helper functions, which each caller would then have to put
> together / reassemble. It seems like a lot of duplicate code and I'm
> not sure I see the benefit of open-coding that when none of the
> callers right now need special logic interjected between the
> fine-grained helpers and and their callback. imo it seems more
> advantageous to only add that in the future if there's a caller that
> has that special need, where only that caller would need to do that,
> instead of making all callers have to do that.

If you take a look at the initial commit of iomap ae259a9c8593
("fs: introduce iomap infrastructure"), the starting point was
to provide `struct iomap_ops` (two callbacks) to get the filesystem
extent mapping and unique entrypoints: iomap_file_buffered_write(),
iomap_page_mkwrite() for buffer writes (write and mmap write).

And then iomap applies to DAX, direct I/O, writeback, buffer read,
etc.  basically for block-based filesystems, and that is why
`struct iomap_ops` is always _one important part of iomap core_
until FUSE adapted.


But if we take a look at the overall requirement, I think what we
need is to provide an easy-use high-level fs helper library to
wrap up the following stuffs:

  1. page cache management (mainly sub-page tracking) and DAX pages;

  2. IO management (buffer I/O, direct I/O, DAX access);

  3. others (like fiemap, bmap, lseek, etc.)


For example, most block filesystems could use them of all, and for
FUSE and networking filesystem, basically they only need 1 and
maybe partial 3.

The recent implementation (including ->iomap_next()), is still
a callback-based model, the main loop is inside `fs/iomap` itself,
and you have to:

  - still arrange the mapping logic into an unique callback, which
is .iomap_next() now, if the mapping logic needs more information
rather than `struct inode *`, they have to add private pointer
somewhere.

  - Take a recent example (buffered read refactoring), need
    `struct iomap_read_folio_ctx` for FUSE and maybe other future
    networking filesystems to callback into their filesystem to
    provide new I/O contexts and submit I/Os.

But actually take a simple example of iomap_read_folio, you:
could change into (just a very rough sketch):

  xxxfs_read_folio {
    struct xxxxfs_private priv;

    iomap_iter(&iter) {   /// it could be a macro
        if (iomap_iter_outofrange(&iter))
		xxxfs_map_extent(&iter, &priv);

        // to maintain the page cache <-> struct iomap_folio_state
        // and handle some type of mapping
        if (iomap_read_folio_iter(&iter) == NEED_NEW_IO_CONTEXT) {
            xxxfs_new_io (&iter, &priv);
        }
    }
    if (xxxfs_if_any_available_io(&iter, &priv))
	xxxfs_submit_io(&iter, &priv);
}

In this model, I really think `struct iomap_read_folio_ctx` and
similar doesn't need any more (another set of callback avoidness),
and both xxxfs_map_extent() and xxxfs_new_io()/ xxxfs_submit_io()
can pass in their per-fs private without need to bother
the iomap-core to add various `private` pointers.

and in the iomap_iter() loop you can omit/change/add any part if
it doesn't fulfill to the filesystem timing.

As for "a lot of duplicate code", I think with proper new designs,
the common part could be wrapped up into macros (for example,
for bdev filesystems, xxxfs_new_io() and xxxfs_submit_io() calls
can be wrapped up into macros, and only leave:

	if (iomap_iter_outofrange(&iter))
		xxxfs_map_extent(&iter, &priv);"

can be customized for each fs as the common macro helpers.

It's just my rough thought, as I said in the previous year, I
still don't have a strong tendency but it's my suggestion over
these years.

Anyway since you're actively working/changing on iomap, so
it's fine to just respect your ideas/efforts if you don't
think it's worth, but as a record of my thoughts here again.

Thanks,
Gao Xiang

> 
> Thanks,
> Joanne

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Christoph Hellwig]

On Fri, Jun 26, 2026 at 10:22:17AM +0800, Gao Xiang wrote:
> It's just my rough thought, as I said in the previous year, I
> still don't have a strong tendency but it's my suggestion over
> these years.
>
> Anyway since you're actively working/changing on iomap, so
> it's fine to just respect your ideas/efforts if you don't
> think it's worth, but as a record of my thoughts here again.

This does sound like a lot of work, and I'm not a 100% sold on
the benefits, although getting rid of callbacks would be nice.

I'd be tempted to say that as a first step we should move the the
single callback, as a second stop (after converting everyone to
the single callback) to inline for performance critical path, and
then see if we can massage it into a pure iterator without much
downside instead of trying to do everything at once.

As you seem to have the strongest opinion here, I'd love to see
a proof of concept from you to get everyone excited.

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Gao Xiang]

Hi Christoph,

On 2026/6/26 13:10, Christoph Hellwig wrote:
> On Fri, Jun 26, 2026 at 10:22:17AM +0800, Gao Xiang wrote:
>> It's just my rough thought, as I said in the previous year, I
>> still don't have a strong tendency but it's my suggestion over
>> these years.
>>
>> Anyway since you're actively working/changing on iomap, so
>> it's fine to just respect your ideas/efforts if you don't
>> think it's worth, but as a record of my thoughts here again.
> 
> This does sound like a lot of work, and I'm not a 100% sold on
> the benefits, although getting rid of callbacks would be nice.
> 
> I'd be tempted to say that as a first step we should move the the
> single callback, as a second stop (after converting everyone to
> the single callback) to inline for performance critical path, and
> then see if we can massage it into a pure iterator without much
> downside instead of trying to do everything at once.
> 
> As you seem to have the strongest opinion here, I'd love to see
> a proof of concept from you to get everyone excited.

I know it's much better to "show the code" but I also think it's
useful to comment ideas now rather than later in case that anyone
is interested so that it could be useful for their adaption.
I hope I could arrange it in my formal todo list, but I've already
had a bunch of other things (erofs fsdax dedupe support as I told
to you since it impacts workloads, and some overlayfs improvement
and more) in my scheduling list.

Although I tend to play with this idea, it's not a blocker on
my main interested use cases for now.  Also see if some
complicated conversion like btrfs and f2fs can make more related
developers interested in following this way.

Thanks,
Gao Xiang

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Christoph Hellwig]

On Fri, Jun 26, 2026 at 01:35:33PM +0800, Gao Xiang wrote:
> I know it's much better to "show the code" but I also think it's
> useful to comment ideas now rather than later in case that anyone
> is interested so that it could be useful for their adaption.

Sure.

> I hope I could arrange it in my formal todo list, but I've already
> had a bunch of other things (erofs fsdax dedupe support as I told
> to you since it impacts workloads, and some overlayfs improvement
> and more) in my scheduling list.
>
> Although I tend to play with this idea, it's not a blocker on
> my main interested use cases for now.  Also see if some
> complicated conversion like btrfs and f2fs can make more related
> developers interested in following this way.

Yeah.  But for now I'd like to get the relatively low hanging fruit
done.  And Joanne showed that moving from begin/end to an iter is
actually way simpler than I thought by having the iomap_process
helper that still does get callbacks, but which get inlined.  So
I'd be really happy to get this done ASAP before we get a lot more
conversions, and also to avoid having a partial transitition where
we need to keep the old ops around for too long.

After that doing the inlining as in patch 3 becomes pretty easy,
so we can do it where it matter like direct I/O.  And also once
we are down to a single callback I think we have a better baseline
for your idea.

>
> Thanks,
> Gao Xiang
---end quoted text---

Re: [RFC PATCH v1 0/3] iomap: convert to in-iter ->iomap_next() model

[Christoph Hellwig]

On Wed, Jun 24, 2026 at 07:47:20PM -0700, Joanne Koong wrote:
> A few questions:
> * is this roughly the in-iter direction you had in mind?

I like it.

> * is removing the indirect call still worth it? My understanding is that
>   indirect calls are cheap on modern eIBRS hardware and the conversion adds
>   some per-filesystem boilerplate, so I'm unsure if it carries its weight. If
>   not, do you think the in-iter model is still worth having on its own?

Indirect calls have always been relatively slow, even without spectre.
So if we can easily avoid them that's always a win.  That doesn't mean we
should do stupid things just to avoid them, but I think we have a win/win
here.