Re: [PATCH v1] iomap: simplify iomap_iter_advance()

[Brian Foster <bfoster@redhat.com>]

On Wed, Sep 17, 2025 at 02:54:09PM -0700, Joanne Koong wrote:
> On Wed, Sep 17, 2025 at 6:08 AM Brian Foster <bfoster@redhat.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 05:40:01PM -0700, Joanne Koong wrote:
> > > Most callers of iomap_iter_advance() do not need the remaining length
> > > returned. Get rid of the extra iomap_length() call that
> > > iomap_iter_advance() does. If the caller wants the remaining length,
> > > they can make the call to iomap_length().
> > >
> > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > > ---
> >
> > Indeed this does clean up some of the calls that create a local var
> > purely to work around the interface. OTOH, the reason I made the advance
> > update the length was so it was clear the remaining length would be used
> > correctly in those looping callers. Otherwise I'm not sure it's clear
> > the bytes/length value needs to be updated and that felt like an easy
> > mistake to make to me.
> >
> > I don't really have a strong preference so I'll defer to whatever the
> > majority opinion is. I wonder though if something else worth considering
> > is to rename the current helper to __iomap_iter_advance() (or whatever)
> > and make your updated variant of iomap_iter_advance() into a wrapper of
> > it.
> 
> That idea sounds good to me too, though I wonder for the naming of it
> if iomap_iter_advance() should remain the current helper and
> __iomap_iter_advance() should be for this more-minimal version of it.
> From what I've seen elsewhere, it seems like the __ prefix is used for
> minimum behavior and then the non-__ version of it is for that
> behavior + extra stuff.
> 

IME the __iomap_iter_advance() would be the most low level and flexible
version, whereas the wrappers simplify things. There's also the point
that the wrapper seems the more common case, so maybe that makes things
cleaner if that one is used more often.

But TBH I'm not sure there is strong precedent. I'm content if we can
retain the current variant for the callers that take advantage of it.
Another idea is you could rename the current function to
iomap_iter_advance_and_update_length_for_loopy_callers() and see what
alternative suggestions come up. ;)

Brian

> 
> Thanks,
> Joanne
> >
> > Brian
>