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

On Thu, Jul 23, 2026 at 9:47 AM Darrick J. Wong <[email protected]> wrote:
>
> On Thu, Jul 23, 2026 at 07:01:40AM +0200, Christoph Hellwig 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.
> >
> > 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 <[email protected]>
> > ---
> >  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);
>
> I was kinda wondering if you can pass ext4_iomap_ops.iomap_begin here
> to avoid the declaration in ext4.h?

I think it ends up being a bit cleaner to do it here, otherwise the
subsequent ext4 patch that replaces .iomap_begin()/.iomap_end() with
.iomap_next() will have to do it.

Thanks,
Joanne