Re: [PATCH v5 09/10] iomap: advance the iter directly on unshare range

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

On Wed, Feb 05, 2025 at 08:58:20AM -0500, Brian Foster wrote:
> Modify unshare range to advance the iter directly. Replace the local
> pos and length calculations with direct advances and loop based on
> iter state instead.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/iomap/buffered-io.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 678c189faa58..f953bf66beb1 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1267,20 +1267,19 @@ EXPORT_SYMBOL_GPL(iomap_write_delalloc_release);
>  static loff_t iomap_unshare_iter(struct iomap_iter *iter)
>  {
>  	struct iomap *iomap = &iter->iomap;
> -	loff_t pos = iter->pos;
> -	loff_t length = iomap_length(iter);
> -	loff_t written = 0;
> +	u64 bytes = iomap_length(iter);
> +	int status;
>  
>  	if (!iomap_want_unshare_iter(iter))
> -		return length;
> +		return iomap_iter_advance(iter, &bytes);
>  
>  	do {
>  		struct folio *folio;
> -		int status;
>  		size_t offset;
> -		size_t bytes = min_t(u64, SIZE_MAX, length);
> +		loff_t pos = iter->pos;

Do we still need the local variable here?

Otherwise looks right to me, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  		bool ret;
>  
> +		bytes = min_t(u64, SIZE_MAX, bytes);
>  		status = iomap_write_begin(iter, pos, bytes, &folio);
>  		if (unlikely(status))
>  			return status;
> @@ -1298,14 +1297,14 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter)
>  
>  		cond_resched();
>  
> -		pos += bytes;
> -		written += bytes;
> -		length -= bytes;
> -
>  		balance_dirty_pages_ratelimited(iter->inode->i_mapping);
> -	} while (length > 0);
>  
> -	return written;
> +		status = iomap_iter_advance(iter, &bytes);
> +		if (status)
> +			break;
> +	} while (bytes > 0);
> +
> +	return status;
>  }
>  
>  int
> -- 
> 2.48.1
> 
>