On Wed, Feb 05, 2025 at 08:58:21AM -0500, Brian Foster wrote:
> Modify zero 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@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/iomap/buffered-io.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index f953bf66beb1..ec227b45f3aa 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1345,17 +1345,16 @@ static inline int iomap_zero_iter_flush_and_stale(struct iomap_iter *i)
>
> static loff_t iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
> {
> - loff_t pos = iter->pos;
> - loff_t length = iomap_length(iter);
> - loff_t written = 0;
> + u64 bytes = iomap_length(iter);
> + int status;
>
> do {
> struct folio *folio;
> - int status;
> size_t offset;
> - size_t bytes = min_t(u64, SIZE_MAX, length);
> + loff_t pos = iter->pos;
I wonder, is there any need for the local variable here?
Looks fine to me though.
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> bool ret;
>
> + bytes = min_t(u64, SIZE_MAX, bytes);
> status = iomap_write_begin(iter, pos, bytes, &folio);
> if (status)
> return status;
> @@ -1376,14 +1375,14 @@ static loff_t iomap_zero_iter(struct iomap_iter *iter, bool *did_zero)
> if (WARN_ON_ONCE(!ret))
> return -EIO;
>
> - pos += bytes;
> - length -= bytes;
> - written += bytes;
> - } while (length > 0);
> + status = iomap_iter_advance(iter, &bytes);
> + if (status)
> + break;
> + } while (bytes > 0);
>
> if (did_zero)
> *did_zero = true;
> - return written;
> + return status;
> }
>
> int
> @@ -1436,11 +1435,14 @@ iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero,
>
> if (srcmap->type == IOMAP_HOLE ||
> srcmap->type == IOMAP_UNWRITTEN) {
> - loff_t proc = iomap_length(&iter);
> + s64 proc;
>
> if (range_dirty) {
> range_dirty = false;
> proc = iomap_zero_iter_flush_and_stale(&iter);
> + } else {
> + u64 length = iomap_length(&iter);
> + proc = iomap_iter_advance(&iter, &length);
> }
> iter.processed = proc;
> continue;
> --
> 2.48.1
>
>
