Re: [PATCH v5 06/10] iomap: export iomap_iter_advance() and return remaining length

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

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@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  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.

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
> 
>