Re: [PATCH v6 3/3] iomap: add simple dio path for small direct I/O

From: changfengnan

Date: Mon Jul 06 2026 - 07:21:11 EST



> From: "Christoph Hellwig"<hch@xxxxxxxxxxxxx>
> Date:  Fri, Jul 3, 2026, 20:40
> Subject:  Re: [PATCH v6 3/3] iomap: add simple dio path for small direct I/O
> To: "changfengnan"<changfengnan@xxxxxxxxxxxxx>
> Cc: "Joanne Koong"<joannelkoong@xxxxxxxxx>, "Christoph Hellwig"<hch@xxxxxxxxxxxxx>, "brauner"<brauner@xxxxxxxxxx>, "djwong"<djwong@xxxxxxxxxx>, "ojaswin"<ojaswin@xxxxxxxxxxxxx>, "dgc"<dgc@xxxxxxxxxx>, "linux-xfs"<linux-xfs@xxxxxxxxxxxxxxx>, "linux-fsdevel"<linux-fsdevel@xxxxxxxxxxxxxxx>, "linux-ext4"<linux-ext4@xxxxxxxxxxxxxxx>, "linux-kernel"<linux-kernel@xxxxxxxxxxxxxxx>, <lidiangang@xxxxxxxxxxxxx>, "pankaj.raghav"<pankaj.raghav@xxxxxxxxx>, "Brian Foster"<bfoster@xxxxxxxxxx>
> On Fri, Jul 03, 2026 at 05:13:12PM +0800, changfengnan wrote:
> > taken, which introduced some overhead. 
> > I implemented some optimizations based on Christoph’s suggestions and
> > made some modifications to `iomap_process`; 
> > Now, XFS performance is essentially unchanged, while ext4 still shows a
> > 1.5% drop; I will continue to investigate the cause..   I’ve attached the patch.
> > Actually, I had expected that switching to `iomap_next` would improve
> > performance, since it avoids an indirect call.
> 
> I don't think it will for you - the direct I/O readers don't have
> iomap_end methods, so you still have the same number of indirect
> calls.  What could help is to inline the iterator as seen in Joannes'
> fist demo series.  We could even look into doing that only for simple
> dio in a first step, shifting the burden to use the simple method
> to the callers (at least for the POC).

I'm not sure if this is what you had in mind, but I made some
adjustments referencing Joannes' first patch, and using this approach, 
there was no performance degradation.

diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index c05849d305ae4..fd2aeabca892a 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -809,8 +809,8 @@ static ssize_t exfat_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
 
         if (iocb->ki_flags & IOCB_DIRECT) {
                 file_accessed(iocb->ki_filp);
-                ret = iomap_dio_rw(iocb, iter, exfat_iomap_next, NULL, 0,
-                                NULL, 0);
+                ret = iomap_dio_rw(iocb, iter, exfat_iomap_next, NULL,
+                                   IOMAP_DIO_NO_IOMAP_END, NULL, 0);
         } else {
                 ret = generic_file_read_iter(iocb, iter);
         }
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 755fde1baf036..cf1be36fdb177 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4004,6 +4004,9 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
                 io_end->flag &= ~EXT4_IO_END_UNWRITTEN;
 }
 
+int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+                     unsigned int flags, struct iomap *iomap,
+                     struct iomap *srcmap);
 int ext4_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
                 struct iomap *srcmap);
 int ext4_iomap_next_report(const struct iomap_iter *iter, struct iomap *iomap,
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index dbe073e181a7a..e08edabcdeaa0 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -91,7 +91,8 @@ 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_next, NULL, 0, NULL, 0);
+        ret = iomap_dio_rw_begin(iocb, to, ext4_iomap_begin, ext4_iomap_next,
+                                 NULL, IOMAP_DIO_NO_IOMAP_END, NULL, 0);
         inode_unlock_shared(inode);
 
         file_accessed(iocb->ki_filp);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 4c30dd8dbec7a..479406c84ff61 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3770,9 +3770,9 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
         return ret;
 }
 
-
-static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
-                unsigned flags, struct iomap *iomap, struct iomap *srcmap)
+int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+                     unsigned int flags, struct iomap *iomap,
+                     struct iomap *srcmap)
 {
         int ret;
         struct ext4_map_blocks map;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index d08b7d3c52710..fa17e249b0b03 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -1000,6 +1000,40 @@ iomap_dio_simple_supported(struct kiocb *iocb, struct iov_iter *iter,
         return true;
 }
 
+static inline void
+iomap_dio_simple_iter_done(struct iomap_iter *iomi)
+{
+        WARN_ON_ONCE(iomi->iomap.offset > iomi->pos);
+        WARN_ON_ONCE(iomi->iomap.length == 0);
+        WARN_ON_ONCE(iomi->iomap.offset + iomi->iomap.length <= iomi->pos);
+        WARN_ON_ONCE(iomi->iomap.flags & IOMAP_F_STALE);
+
+        iomi->iter_start_pos = iomi->pos;
+
+        trace_iomap_iter_dstmap(iomi->inode, &iomi->iomap);
+        if (iomi->srcmap.type != IOMAP_HOLE)
+                trace_iomap_iter_srcmap(iomi->inode, &iomi->srcmap);
+}
+
+static inline int
+iomap_dio_simple_begin_iter(struct iomap_iter *iomi,
+                            iomap_begin_fn iomap_begin,
+                            iomap_next_fn iomap_next)
+{
+        int ret;
+
+        trace_iomap_iter(iomi, iomap_next, _RET_IP_);
+
+        ret = iomap_begin(iomi->inode, iomi->pos, iomi->len, iomi->flags,
+                          &iomi->iomap, &iomi->srcmap);
+        iomi->status = 0;
+        if (ret < 0)
+                return ret;
+
+        iomap_dio_simple_iter_done(iomi);
+        return 1;
+}
+
 static int
 iomap_dio_simple_finish_iter(struct iomap_iter *iomi,
                 iomap_next_fn iomap_next, size_t done)
@@ -1053,8 +1087,8 @@ iomap_dio_simple_finish_iter(struct iomap_iter *iomi,
  */
 static ssize_t
 iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
-                 iomap_next_fn iomap_next, void *private,
-                 unsigned int dio_flags)
+                 iomap_next_fn iomap_next, iomap_begin_fn iomap_begin,
+                 void *private, unsigned int dio_flags)
 {
         struct inode *inode = file_inode(iocb->ki_filp);
         size_t count = iov_iter_count(iter);
@@ -1080,7 +1114,11 @@ iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
 
         inode_dio_begin(inode);
 
-        ret = iomap_iter(&iomi, iomap_next);
+        if (iomap_begin)
+                ret = iomap_dio_simple_begin_iter(&iomi, iomap_begin,
+                                                  iomap_next);
+        else
+                ret = iomap_iter(&iomi, iomap_next);
         if (ret <= 0) {
                 inode_dio_end(inode);
                 return ret ?: -ENOTBLK;
@@ -1142,9 +1180,18 @@ iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
                 WRITE_ONCE(iocb->private, bio);
         }
 
-        ret = iomap_dio_simple_finish_iter(&iomi, iomap_next, count);
-        if (ret)
-                goto out_bio_release_pages_no_end;
+        /*
+         * The finishing iomap_iter() only runs the filesystem's ->iomap_end()
+         * work and releases any attached folio batch.  When the caller promised
+         * neither applies (IOMAP_DIO_NO_IOMAP_END), skip it to avoid an extra
+         * indirect ->iomap_next() call and the iomap_iter() frames on the hot
+         * read path.
+         */
+        if (!(dio_flags & IOMAP_DIO_NO_IOMAP_END)) {
+                ret = iomap_dio_simple_finish_iter(&iomi, iomap_next, count);
+                if (ret)
+                        goto out_bio_release_pages_no_end;
+        }
 
         if (!wait_for_completion) {
                 bio->bi_end_io = iomap_dio_simple_end_io;
@@ -1161,7 +1208,8 @@ iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
 out_bio_put:
         bio_put(bio);
 out_iomap_end:
-        iomap_dio_simple_finish_iter(&iomi, iomap_next, 0);
+        if (!(dio_flags & IOMAP_DIO_NO_IOMAP_END))
+                iomap_dio_simple_finish_iter(&iomi, iomap_next, 0);
         inode_dio_end(inode);
         return ret;
 out_bio_release_pages_no_end:
@@ -1181,8 +1229,8 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 
         if (iomap_dio_simple_supported(iocb, iter, dops, dio_flags,
                                        done_before)) {
-                ret = iomap_dio_simple(iocb, iter, iomap_next, private,
-                                dio_flags);
+                ret = iomap_dio_simple(iocb, iter, iomap_next, NULL, private,
+                                       dio_flags);
                 if (ret != -ENOTBLK)
                         return ret;
         }
@@ -1195,6 +1243,32 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 }
 EXPORT_SYMBOL_GPL(iomap_dio_rw);
 
+ssize_t
+iomap_dio_rw_begin(struct kiocb *iocb, struct iov_iter *iter,
+                   iomap_begin_fn iomap_begin, iomap_next_fn iomap_next,
+                   const struct iomap_dio_ops *dops, unsigned int dio_flags,
+                   void *private, size_t done_before)
+{
+        struct iomap_dio *dio;
+        ssize_t ret;
+
+        if ((dio_flags & IOMAP_DIO_NO_IOMAP_END) &&
+            iomap_dio_simple_supported(iocb, iter, dops, dio_flags,
+                                       done_before)) {
+                ret = iomap_dio_simple(iocb, iter, iomap_next, iomap_begin,
+                                       private, dio_flags);
+                if (ret != -ENOTBLK)
+                        return ret;
+        }
+
+        dio = __iomap_dio_rw(iocb, iter, iomap_next, 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_begin);
+
 static int __init iomap_dio_init(void)
 {
         return bioset_init(&iomap_dio_simple_pool, 4,
diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
index 984045af310a1..7dee4a7944071 100644
--- a/fs/iomap/iter.c
+++ b/fs/iomap/iter.c
@@ -15,16 +15,6 @@ static inline void iomap_iter_clean_fbatch(struct iomap_iter *iter)
         }
 }
 
-/* Advance the current iterator position and decrement the remaining length */
-int iomap_iter_advance(struct iomap_iter *iter, u64 count)
-{
-        if (WARN_ON_ONCE(count > iomap_length(iter)))
-                return -EIO;
-        iter->pos += count;
-        iter->len -= count;
-        return 0;
-}
-
 static inline void iomap_iter_done(struct iomap_iter *iter)
 {
         WARN_ON_ONCE(iter->iomap.offset > iter->pos);
diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h
index e1ea8392cf47d..e43ca419b13a8 100644
--- a/fs/iomap/trace.h
+++ b/fs/iomap/trace.h
@@ -127,7 +127,8 @@ DEFINE_RANGE_EVENT(iomap_zero_iter);
         {IOMAP_DIO_FORCE_WAIT,                "DIO_FORCE_WAIT" }, \
         {IOMAP_DIO_OVERWRITE_ONLY,        "DIO_OVERWRITE_ONLY" }, \
         {IOMAP_DIO_PARTIAL,                "DIO_PARTIAL" }, \
-        {IOMAP_DIO_FSBLOCK_ALIGNED,        "DIO_FSBLOCK_ALIGNED" }
+        {IOMAP_DIO_FSBLOCK_ALIGNED,        "DIO_FSBLOCK_ALIGNED" }, \
+        {IOMAP_DIO_NO_IOMAP_END,        "DIO_NO_IOMAP_END" }
 
 DECLARE_EVENT_CLASS(iomap_class,
         TP_PROTO(struct inode *inode, struct iomap *iomap),
diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index a4f99128b46c5..babdfd0eff4f9 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -458,8 +458,8 @@ static ssize_t ntfs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
                 }
 
                 file_accessed(iocb->ki_filp);
-                ret = iomap_dio_rw(iocb, to, ntfs_read_iomap_next, NULL, 0,
-                                NULL, 0);
+                ret = iomap_dio_rw(iocb, to, ntfs_read_iomap_next, NULL,
+                                   IOMAP_DIO_NO_IOMAP_END, NULL, 0);
         } else {
                 ret = generic_file_read_iter(iocb, to);
         }
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index a987ffbf3c023..2d4dbd9fac7b4 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -251,7 +251,7 @@ xfs_file_dio_read(
         struct iov_iter                *to)
 {
         struct xfs_inode        *ip = XFS_I(file_inode(iocb->ki_filp));
-        unsigned int                dio_flags = 0;
+        unsigned int                dio_flags = IOMAP_DIO_NO_IOMAP_END;
         const struct iomap_dio_ops *dio_ops = NULL;
         ssize_t                        ret;
 
@@ -269,8 +269,9 @@ xfs_file_dio_read(
                 dio_ops = &xfs_dio_read_bounce_ops;
                 dio_flags |= IOMAP_DIO_BOUNCE;
         }
-        ret = iomap_dio_rw(iocb, to, xfs_read_iomap_next, dio_ops, dio_flags,
-                        NULL, 0);
+        ret = iomap_dio_rw_begin(iocb, to, xfs_read_iomap_begin,
+                                 xfs_read_iomap_next, 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 71c4bb024f042..dcc7f1fe2d52b 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -2203,7 +2203,7 @@ xfs_buffered_write_iomap_next(
                         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 01875d20fb669..3db533e977e09 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -55,6 +55,9 @@ int xfs_direct_write_iomap_next(const struct iomap_iter *iter,
                 struct iomap *iomap, struct iomap *srcmap);
 int xfs_zoned_direct_write_iomap_next(const struct iomap_iter *iter,
                 struct iomap *iomap, struct iomap *srcmap);
+int xfs_read_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
+                         unsigned int flags, struct iomap *iomap,
+                         struct iomap *srcmap);
 int xfs_read_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
                 struct iomap *srcmap);
 int xfs_seek_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 3b41f123a92d4..060051fc79b1f 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -266,7 +266,6 @@ struct iomap_iter {
 };
 
 int iomap_iter(struct iomap_iter *iter, iomap_next_fn iomap_next);
-int iomap_iter_advance(struct iomap_iter *iter, u64 count);
 
 /**
  * iomap_length_trim - trimmed length of the current iomap iteration
@@ -298,6 +297,16 @@ static inline u64 iomap_length(const struct iomap_iter *iter)
         return iomap_length_trim(iter, iter->pos, iter->len);
 }
 
+/* Advance the current iterator position and decrement the remaining length */
+static inline int iomap_iter_advance(struct iomap_iter *iter, u64 count)
+{
+        if (WARN_ON_ONCE(count > iomap_length(iter)))
+                return -EIO;
+        iter->pos += count;
+        iter->len -= count;
+        return 0;
+}
+
 /**
  * iomap_iter_advance_full - advance by the full length of current map
  */
@@ -605,9 +614,23 @@ struct iomap_dio_ops {
  */
 #define IOMAP_DIO_BOUNCE                (1 << 4)
 
+/*
+ * The caller's ->iomap_next() has no ->iomap_end() work to run and this I/O
+ * does not use a folio batch, so the iomap_dio_simple() fast path may skip
+ * the finishing iomap_iter() call entirely.  Only meaningful on the simple
+ * read fast path; ignored by the generic __iomap_dio_rw() slow path.
+ */
+#define IOMAP_DIO_NO_IOMAP_END                BIT(5)
+
 ssize_t iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
                 iomap_next_fn iomap_next, const struct iomap_dio_ops *dops,
                 unsigned int dio_flags, void *private, size_t done_before);
+ssize_t iomap_dio_rw_begin(struct kiocb *iocb, struct iov_iter *iter,
+                           iomap_begin_fn iomap_begin,
+                           iomap_next_fn iomap_next,
+                           const struct iomap_dio_ops *dops,
+                           unsigned int dio_flags, void *private,
+                           size_t done_before);
 struct iomap_dio *__iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
                 iomap_next_fn iomap_next, const struct iomap_dio_ops *dops,
                 unsigned int dio_flags, void *private, size_t done_before);
>