Re: [PATCH v1 07/16] iomap: iterate through entire folio in iomap_readpage_iter()

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

On Fri, Aug 29, 2025 at 04:56:18PM -0700, Joanne Koong wrote:
> Iterate through the entire folio in iomap_readpage_iter() in one go
> instead of in pieces. This will be needed for supporting user-provided
> async read folio callbacks (not yet added). This additionally makes the
> iomap_readahead_iter() logic simpler to follow.

This might be a good time to change the name since you're not otherwise
changing the function declaration, and there ought to be /some/
indication that the behavior isn't the same anymore.

Otherwise, this looks correct to me.

--D

> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
>  fs/iomap/buffered-io.c | 76 ++++++++++++++++++------------------------
>  1 file changed, 33 insertions(+), 43 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index f26544fbcb36..75bbef386b62 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -452,6 +452,7 @@ static int iomap_readpage_iter(struct iomap_iter *iter,
>  	loff_t length = iomap_length(iter);
>  	struct folio *folio = ctx->cur_folio;
>  	size_t poff, plen;
> +	loff_t count;
>  	int ret;
>  
>  	if (iomap->type == IOMAP_INLINE) {
> @@ -463,26 +464,30 @@ static int iomap_readpage_iter(struct iomap_iter *iter,
>  
>  	/* zero post-eof blocks as the page may be mapped */
>  	ifs_alloc(iter->inode, folio, iter->flags);
> -	iomap_adjust_read_range(iter->inode, folio, &pos, length, &poff, &plen);
> -	if (plen == 0)
> -		goto done;
>  
> -	if (iomap_block_needs_zeroing(iter, pos)) {
> -		folio_zero_range(folio, poff, plen);
> -		iomap_set_range_uptodate(folio, poff, plen);
> -	} else {
> -		iomap_read_folio_range_async(iter, ctx, pos, plen);
> -	}
> +	length = min_t(loff_t, length,
> +			folio_size(folio) - offset_in_folio(folio, pos));
> +	while (length) {
> +		iomap_adjust_read_range(iter->inode, folio, &pos,
> +				length, &poff, &plen);
> +		count = pos - iter->pos + plen;
> +		if (plen == 0)
> +			return iomap_iter_advance(iter, &count);
>  
> -done:
> -	/*
> -	 * Move the caller beyond our range so that it keeps making progress.
> -	 * For that, we have to include any leading non-uptodate ranges, but
> -	 * we can skip trailing ones as they will be handled in the next
> -	 * iteration.
> -	 */
> -	length = pos - iter->pos + plen;
> -	return iomap_iter_advance(iter, &length);
> +		if (iomap_block_needs_zeroing(iter, pos)) {
> +			folio_zero_range(folio, poff, plen);
> +			iomap_set_range_uptodate(folio, poff, plen);
> +		} else {
> +			iomap_read_folio_range_async(iter, ctx, pos, plen);
> +		}
> +
> +		length -= count;
> +		ret = iomap_iter_advance(iter, &count);
> +		if (ret)
> +			return ret;
> +		pos = iter->pos;
> +	}
> +	return 0;
>  }
>  
>  static void iomap_readfolio_complete(const struct iomap_iter *iter,
> @@ -494,20 +499,6 @@ static void iomap_readfolio_complete(const struct iomap_iter *iter,
>  		folio_unlock(ctx->cur_folio);
>  }
>  
> -static int iomap_read_folio_iter(struct iomap_iter *iter,
> -		struct iomap_readpage_ctx *ctx)
> -{
> -	int ret;
> -
> -	while (iomap_length(iter)) {
> -		ret = iomap_readpage_iter(iter, ctx);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	return 0;
> -}
> -
>  int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops)
>  {
>  	struct iomap_iter iter = {
> @@ -523,7 +514,7 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops)
>  	trace_iomap_readpage(iter.inode, 1);
>  
>  	while ((ret = iomap_iter(&iter, ops)) > 0)
> -		iter.status = iomap_read_folio_iter(&iter, &ctx);
> +		iter.status = iomap_readpage_iter(&iter, &ctx);
>  
>  	iomap_readfolio_complete(&iter, &ctx);
>  
> @@ -537,16 +528,15 @@ static int iomap_readahead_iter(struct iomap_iter *iter,
>  	int ret;
>  
>  	while (iomap_length(iter)) {
> -		if (ctx->cur_folio &&
> -		    offset_in_folio(ctx->cur_folio, iter->pos) == 0) {
> -			if (!ctx->folio_unlocked)
> -				folio_unlock(ctx->cur_folio);
> -			ctx->cur_folio = NULL;
> -		}
> -		if (!ctx->cur_folio) {
> -			ctx->cur_folio = readahead_folio(ctx->rac);
> -			ctx->folio_unlocked = false;
> -		}
> +		if (ctx->cur_folio && !ctx->folio_unlocked)
> +			folio_unlock(ctx->cur_folio);
> +		ctx->cur_folio = readahead_folio(ctx->rac);
> +		/*
> +		 * We should never in practice hit this case since
> +		 * the iter length matches the readahead length.
> +		 */
> +		WARN_ON(!ctx->cur_folio);
> +		ctx->folio_unlocked = false;
>  		ret = iomap_readpage_iter(iter, ctx);
>  		if (ret)
>  			return ret;
> -- 
> 2.47.3
> 
>