Re: [PATCH v2 18/18] Documentation: iomap: update docs to reflect iomap_next model

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jul 02, 2026 at 06:36:55PM -0700, Joanne Koong wrote:
> On Thu, Jul 2, 2026 at 12:27 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Tue, Jun 30, 2026 at 05:09:33PM -0700, Joanne Koong wrote:
> > > Filesystems no longer pass a struct iomap_ops with separate
> > > ->iomap_begin() and ->iomap_end() callbacks.  Instead, every iomap
> > > operation takes a single iomap_next() callback directly. iomap_next()
> > > finishes the previous mapping (if any) and produces the next one. Most
> > > filesystems build it from begin and end helpers via the iomap_process()
> > > helper.
> > >
> > > Update the iomap documentation to match this change.
> > >
> > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > > ---
> > >  Documentation/filesystems/iomap/design.rst    | 115 +++++++++++++-----
> > >  .../filesystems/iomap/operations.rst          |  60 +++++----
> > >  Documentation/filesystems/iomap/porting.rst   |  22 +++-
> > >  3 files changed, 132 insertions(+), 65 deletions(-)
> > >
> > > diff --git a/Documentation/filesystems/iomap/design.rst b/Documentation/filesystems/iomap/design.rst
> > > index 0f7672676c0b..7a37e303eea8 100644
> > > --- a/Documentation/filesystems/iomap/design.rst
> > > +++ b/Documentation/filesystems/iomap/design.rst
> > > @@ -75,7 +75,10 @@ At a high level, an iomap operation `looks like this
> > >
> > >  1. For each byte in the operation range...
> > >
> > > -   1. Obtain a space mapping via ``->iomap_begin``
> > > +   1. Obtain the next space mapping via the ``iomap_next`` callback.
> > > +      From the second iteration onwards this same callback first finishes
> > > +      the previous mapping (committing or unreserving space as needed)
> > > +      and then produces the next one.
> > >
> > >     2. For each sub-unit of work...
> > >
> > > @@ -86,7 +89,13 @@ At a high level, an iomap operation `looks like this
> > >
> > >     3. Increment operation cursor
> > >
> > > -   4. Release the mapping via ``->iomap_end``, if necessary
> > > +iomap repeats this until the range is fully consumed.  The ``iomap_next``
> > > +callback returns ``1`` while there is more of the range left to process,
> > > +``0`` once it is fully consumed, and a negative errno on error.
> >
> > s/and/or/
> >
> > > +Filesystems rarely implement ``->iomap_next`` by hand. The ``iomap_process``
> > > +helper implements the finish-then-produce sequence in +terms of two smaller
> > > +callbacks, ``begin`` and ``end``. See `The Mapping Callback`_ below for more
> > > +info.
> >
> > I wonder, under what circumstances would a filesystem /not/ use
> > iomap_process()?
> 
> Hmm, maybe a situation where they need to share or carry some
> filesystem-specific state / info between finishing the mapping and
> producing the next one?

The ->begin method can still set iomap::private and the ->end method can
dispose of it, right?  Oh, wait, no, that doesn't work because you're
talking about ->begin/->end passing something to the next ->begin.

I suppose you could make iomap_iter_continue preserve the iomap/srcmap
private pointer across the memset(0) calls.  But all current users
either rely on private being zeroed by iomap_iter after ->iomap_end
returns, so that would be quite the change.

Hm.  I was thinking that the signature for iomap_process could be
cleaner if you didn't have to pass iomap/srcmap explicitly.
iomap_process could do the (dangerous) casting from the (const struct
iomap_iter *) to the (struct iomap *) pointers before calling ->begin
and ->end.

But if you do have an ->iomap_next function that doesn't use
iomap_process, then it has to do the pointer extraction itself.  We
already expose the innards of struct iomap_iter to callers, so maybe
it's ok to have some gross helpers like:

static inline struct iomap *iomap_iter_iomap(const struct iomap_iter *i)
{
	return (struct iomap *)&i->iomap;
}

static inline struct iomap *iomap_iter_srcmap(const struct iomap_iter *i)
{
	return (struct iomap *)&i->srcmap;
}

I'm not sure *that*'s any cleaner.  And I think it gets confusing with
the other srcmap extraction function.

Eh never mind, I've talked myself out of this. ;)

> > >  Each iomap operation will be covered in more detail below.
> > >  This library was covered previously by an `LWN article
> > > @@ -189,7 +198,7 @@ The fields are as follows:
> > >     * **IOMAP_DELALLOC**: A promise to allocate space at a later time
> > >       ("delayed allocation").
> > >       If the filesystem returns IOMAP_F_NEW here and the write fails, the
> > > -     ``->iomap_end`` function must delete the reservation.
> > > +     ``end`` function must delete the reservation.
> > >       The ``addr`` field must be set to ``IOMAP_NULL_ADDR``.
> > >
> > >     * **IOMAP_MAPPED**: The file range maps to specific space on the
> > > @@ -208,12 +217,12 @@ The fields are as follows:
> > >
> > >     * **IOMAP_INLINE**: The file range maps to the memory buffer
> > >       specified by ``inline_data``.
> > > -     For write operation, the ``->iomap_end`` function presumably
> > > -     handles persisting the data.
> > > +     For write operation, the ``end`` function presumably handles
> > > +     persisting the data.
> >
> > Unrelated to this patch, but this should say "For write operations, the
> > end function must persist the data" because iomap_writepages doesn't
> > handle inline data.
> >
> > >       The ``addr`` field must be set to ``IOMAP_NULL_ADDR``.
> > >
> > >   * ``flags`` describe the status of the space mapping.
> > > -   These flags should be set by the filesystem in ``->iomap_begin``:
> > > +   These flags should be set by the filesystem in ``begin``:
> >>
> > > +``->iomap_next``
> > > +~~~~~~~~~~~~~~~~
> > > +
> > > +Each call must finish the previous mapping, if any, and then produce the
> >
> > Each call?  Oh, each implementation of ->iomap_next must finish the
> > previous mapping.
> >
> > > +next mapping for the current iteration position described by ``iter``.
> > > +The mapping is returned through ``iomap`` (and through ``srcmap`` for
> > > +operations that read from one mapping while writing to another; see
> > > +``begin`` below).
> > >
> > > -``->iomap_begin``
> > > +The callback returns ``1`` to continue iterating, ``0`` once the file
> > > +range has been fully consumed, and a negative errno on error.
> >
> > s/and/or/
> >
> > I think there should be a transition sentence here along the lines of
> >
> > "Most filesystems are not expected to implement all of these behaviors
> > in ->iomap_next themselves.  They should instead call iomap_process as
> > described below."
> >
> > Or demote the next section so it's more obvious that the "iomap_process"
> > and "->iomap_next" sections aren't independent?
> >
> 
> Sounds good, I'll incorporate all the suggestions you recommended.

<nod>

--D

> Thanks,
> Joanne
>