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

[Joanne Koong]

On Wed, Jul 1, 2026 at 4:23 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>
> We'll need to sort out the interaction with the iomap_next work, though.
>
> Joanne, is it ok to get this in first and have Fengnan help your with
> the interactions due to the open coded iomap_begin/end calls?
>

That sounds good, I'll rebase the next version onto these changes.

I think it ends up being pretty simple to get it integrated since
->iomap_next already contains the logic to only begin the mapping at
the start and only finish the mapping when the iter has been fully
consumed, eg

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 3ef5e9adca87..e654ec135d54 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -1035,7 +1035,7 @@ iomap_dio_simple_supported(struct kiocb *iocb,
struct iov_iter *iter,
  */
 static ssize_t
 iomap_dio_simple(struct kiocb *iocb, struct iov_iter *iter,
-                const struct iomap_ops *ops, void *private,
+                iomap_next_fn iomap_next, void *private,
                 unsigned int dio_flags)
 {
        struct inode *inode = file_inode(iocb->ki_filp);
@@ -1062,11 +1062,11 @@ iomap_dio_simple(struct kiocb *iocb, struct
iov_iter *iter,

        inode_dio_begin(inode);

-       ret = ops->iomap_begin(inode, iomi.pos, count, iomi.flags,
-                              &iomi.iomap, &iomi.srcmap);
-       if (ret) {
+       ret = iomap_iter(&iomi, iomap_next);
+       if (ret <= 0) {
                inode_dio_end(inode);
-               return ret;
+               /* ret of 0 means no mapping, which should never happen */
+               return ret ? ret : -EFAULT;
        }

        if (iomi.iomap.type != IOMAP_MAPPED ||
@@ -1125,14 +1125,17 @@ iomap_dio_simple(struct kiocb *iocb, struct
iov_iter *iter,
                WRITE_ONCE(iocb->private, bio);
        }

-       if (ops->iomap_end)
-               ops->iomap_end(inode, iomi.pos, count, count, iomi.flags,
-                              &iomi.iomap);
+       iomap_iter_advance(&iomi, count);
+       /*
+        * ends the mapping if the mapping needs to be ended. no new mappings
+        * are fetched since the entire range of the iter has been consumed
+        */
+       iomap_iter(&iomi, iomap_next);

        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);
+               trace_iomap_dio_rw_queued(inode, iocb->ki_pos, count);
                return -EIOCBQUEUED;
        }

@@ -1144,9 +1147,12 @@ iomap_dio_simple(struct kiocb *iocb, struct
iov_iter *iter,
 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);
+       /*
+        * on failure, iter was never advanced. ->iomap_next() will know that 0
+        * bytes have been processed and the mapping will be ended with no next
+        * mapping fetched
+        */
+       iomap_iter(&iomi, iomap_next);
        inode_dio_end(inode);
        return ret;
 }

Thanks,
Joanne