Re: [PATCH v2 01/18] iomap: add ->iomap_next() and iomap_process() helper

[Joanne Koong]

On Thu, Jul 2, 2026 at 9:23 AM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Tue, Jun 30, 2026 at 05:09:16PM -0700, Joanne Koong wrote:
> > 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.
>
> Er... what is being called directly here?  The iomap_{begin,end}
> functions?  It looks to me like ->iomap_next is still an indirect call
> even at the end of the series, right?  That's still a net reduction of
> indirect calls at least.
>
> Oh, wait, it's the __always_inline iomap_process function that a smart
> compiler can use to turn the indirect calls into direct ones, isn't it?
> It might be useful to add a comment to that function saying that out
> loud so that dolts like me will pick up on why it's critical for it to
> be an inline function.

Ahh the sentence was confusingly / ambiguously worded, sorry about
that - I was trying to say that it lets callers have the ability to
inline iomap_iter() with passing a const iomap_next callback as an arg
which lets the iomap_next callback be a plain direct call. But your
interpretation of it is also true. I'll add the comment you suggested
to spell what you mentioned out and reword the commit message to make
what I was trying to say more clear.

>
> I like how this is going. :)
>
> > 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@xxxxxx>
> > Suggested-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > ---
> >  fs/iomap/iter.c       | 113 ++++++++++++++++++++++++++++++++++++------
> >  include/linux/iomap.h |  91 +++++++++++++++++++++++++++-------
> >  2 files changed, 171 insertions(+), 33 deletions(-)
> >
> > diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
> > index e4a29829591a..1062e4e34c38 100644
> > --- a/fs/iomap/iter.c
> > +++ b/fs/iomap/iter.c
> > +
> > +/**
> > + * 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
> > + *
> > + * Helper for ->iomap_next() implementations, normally called via
> > + * iomap_process().  Called after the previous mapping has been finished to
> > + * determine whether there is more of the file range left to process.
> > + *
> > + * 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_continue(const struct iomap_iter *iter, struct iomap *iomap,
> > +             struct iomap *srcmap, int ret)
> > +{
> > +     bool stale = iomap->flags & IOMAP_F_STALE;
> > +     ssize_t advanced = iter->pos - iter->iter_start_pos;
>
> These both could be const, right?
>

I'll mark these as const. Thanks for looking at these patches!

Thanks,
Joanne