On Wed, Feb 05, 2025 at 10:58:01AM -0800, Darrick J. Wong wrote:
> On Wed, Feb 05, 2025 at 08:58:17AM -0500, Brian Foster wrote:
> > As a final step for generic iter advance, export the helper and
> > update it to return the remaining length of the current iteration
> > after the advance. This will usually be 0 in the iomap_iter() case,
> > but will be useful for the various operations that iterate on their
> > own and will be updated to advance as they progress.
> >
> > Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> > ---
> > fs/iomap/iter.c | 22 ++++++++--------------
> > include/linux/iomap.h | 1 +
> > 2 files changed, 9 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
> > index 8e0746ad80bd..cdba24dbbfd7 100644
> > --- a/fs/iomap/iter.c
> > +++ b/fs/iomap/iter.c
> > @@ -15,22 +15,16 @@ static inline void iomap_iter_reset_iomap(struct iomap_iter *iter)
> > }
> >
> > /*
> > - * Advance to the next range we need to map.
> > - *
> > - * If the iomap is marked IOMAP_F_STALE, it means the existing map was not fully
> > - * processed - it was aborted because the extent the iomap spanned may have been
> > - * changed during the operation. In this case, the iteration behaviour is to
> > - * remap the unprocessed range of the iter, and that means we may need to remap
> > - * even when we've made no progress (i.e. count = 0). Hence the "finished
> > - * iterating" case needs to distinguish between (count = 0) meaning we are done
> > - * and (count = 0 && stale) meaning we need to remap the entire remaining range.
> > + * Advance the current iterator position and return the length remaining for the
> > + * current mapping.
>
> This last sentence should state that the remaining length is returned
> via @count as an outparam and not through the function's return value.
>
Ok.
Brian
> Otherwise looks ok to me.
>
> --D
>
> > */
> > -static inline int iomap_iter_advance(struct iomap_iter *iter, s64 count)
> > +int iomap_iter_advance(struct iomap_iter *iter, u64 *count)
> > {
> > - if (WARN_ON_ONCE(count > iomap_length(iter)))
> > + if (WARN_ON_ONCE(*count > iomap_length(iter)))
> > return -EIO;
> > - iter->pos += count;
> > - iter->len -= count;
> > + iter->pos += *count;
> > + iter->len -= *count;
> > + *count = iomap_length(iter);
> > return 0;
> > }
> >
> > @@ -93,7 +87,7 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
> > * 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);
> > + ret = iomap_iter_advance(iter, &processed);
> > if (!ret && iter->len > 0)
> > ret = 1;
> > if (ret > 0 && !iter->processed && !stale)
> > diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> > index f5ca71ac2fa2..f304c602e5fe 100644
> > --- a/include/linux/iomap.h
> > +++ b/include/linux/iomap.h
> > @@ -229,6 +229,7 @@ struct iomap_iter {
> > };
> >
> > int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops);
> > +int iomap_iter_advance(struct iomap_iter *iter, u64 *count);
> >
> > /**
> > * iomap_length_trim - trimmed length of the current iomap iteration
> > --
> > 2.48.1
> >
> >
>
