On Wed, Feb 05, 2025 at 11:00:20AM -0800, Darrick J. Wong wrote:
> On Wed, Feb 05, 2025 at 08:58:16AM -0500, Brian Foster wrote:
> > The iter termination logic in iomap_iter_advance() is only needed by
> > iomap_iter() to determine whether to proceed with the next mapping
> > for an ongoing operation. The old logic sets ret to 1 and then
> > terminates if the operation is complete (iter->len == 0) or the
> > previous iteration performed no work and the mapping has not been
> > marked stale. The stale check exists to allow operations to
> > retry the current mapping if an inconsistency has been detected.
> >
> > To further genericize iomap_iter_advance(), lift the termination
> > logic into iomap_iter() and update the former to return success (0)
> > or an error code. iomap_iter() continues on successful advance and
> > non-zero iter->len or otherwise terminates in the no progress (and
> > not stale) or error cases.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> > ---
> > fs/iomap/iter.c | 21 +++++++++++++--------
> > 1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
> > index 1db16be7b9f0..8e0746ad80bd 100644
> > --- a/fs/iomap/iter.c
> > +++ b/fs/iomap/iter.c
...
> > @@ -91,8 +86,18 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
> > return processed;
> > }
> >
> > - /* advance and clear state from the previous iteration */
> > + /*
> > + * Advance the iter and clear state from the previous iteration. Use
> > + * iter->len to determine whether to continue onto the next mapping.
> > + * Explicitly terminate in the case where the current iter has not
> > + * advanced at all (i.e. no work was done for some reason) unless the
> > + * mapping has been marked stale and needs to be reprocessed.
> > + */
> > ret = iomap_iter_advance(iter, processed);
> > + if (!ret && iter->len > 0)
> > + ret = 1;
> > + if (ret > 0 && !iter->processed && !stale)
> > + ret = 0;
>
> I guess I'll wait to see what the rest of the conversion series looks
> like...
>
It's not fully tested yet, but FWIW here's what I currently have in
iomap_iter() after the remaining conversions:
...
ret = (iter->len > 0) ? 1 : 0;
if (iter->processed < 0)
ret = iter->processed;
else if (!advanced && !stale)
ret = 0;
iomap_iter_reset_iomap(iter);
if (ret <= 0)
return ret;
...
Note that iter.processed should only be success (0) or error here and
hence I was planning to subsequently rename it to iter.status.
> Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
>
Thanks.
Brian
> --D
>
> > iomap_iter_reset_iomap(iter);
> > if (ret <= 0)
> > return ret;
> > --
> > 2.48.1
> >
> >
>
