Re: [PATCH 03/10] iomap: convert misc simple ops to incremental advance

Brian Foster
2025-02-13T15:24:36Z

On Wed, Feb 12, 2025 at 10:54:42PM -0800, Christoph Hellwig wrote:
> > + u64 length = iomap_length(iter);
> > +
> > switch (iomap->type) {
> > case IOMAP_MAPPED:
> > case IOMAP_UNWRITTEN:
> > @@ -132,7 +134,8 @@ static loff_t iomap_swapfile_iter(const struct iomap_iter *iter,
> > return error;
> > memcpy(&isi->iomap, iomap, sizeof(isi->iomap));
> > }
> > - return iomap_length(iter);
> > +
> > + return iomap_iter_advance(iter, &length);
>
> I notice that for many of these callers passing in the length as in/out
> argument isn't all that great. Not sure that's worth changing now (or
> adding another helper?). The conversion itself looks good.
>

Yes, I noticed the same when I changed it to an in/out param in the
previous series. Several callers end up needing a dummy local to
represent iomap_length(). I left it as is because I think this is the
lesser of two evils compared to the overloaded return situation, at
least for now, but I could also tack on a patch to introduce an
iomap_iter_advance_full(iter) (name?) or some such inline helper to
clean up these cases..

Brian