"Darrick J. Wong" <djwong@xxxxxxxxxx> writes:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Convert iomap_unshare_iter to create large folios if possible, since the
> write and zeroing paths already do that. I think this got missed in the
> conversion of the write paths that landed in 6.6-rc1.
>
> Cc: ritesh.list@xxxxxxxxx, willy@xxxxxxxxxxxxx
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/iomap/buffered-io.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
Looks right to me. Feel free to add -
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
(small nit below)
>
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 0350830fc989..db889bdfd327 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -1263,7 +1263,6 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter)
> const struct iomap *srcmap = iomap_iter_srcmap(iter);
> loff_t pos = iter->pos;
> loff_t length = iomap_length(iter);
> - long status = 0;
> loff_t written = 0;
>
> /* don't bother with blocks that are not shared to start with */
> @@ -1274,9 +1273,10 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter)
> return length;
>
> do {
> - unsigned long offset = offset_in_page(pos);
> - unsigned long bytes = min_t(loff_t, PAGE_SIZE - offset, length);
> struct folio *folio;
> + int status;
> + size_t offset;
> + size_t bytes = min_t(u64, SIZE_MAX, length);
>
> status = iomap_write_begin(iter, pos, bytes, &folio);
> if (unlikely(status))
> @@ -1284,18 +1284,22 @@ static loff_t iomap_unshare_iter(struct iomap_iter *iter)
> if (iter->iomap.flags & IOMAP_F_STALE)
Just noticed that we already have "iomap = &iter->iomap" at the start of this
function. We need not dereference for iomap again here.
(Not related to this patch, but I thought I will mention while we are
still at it)
> break;
>
> - status = iomap_write_end(iter, pos, bytes, bytes, folio);
> - if (WARN_ON_ONCE(status == 0))
> + offset = offset_in_folio(folio, pos);
> + if (bytes > folio_size(folio) - offset)
> + bytes = folio_size(folio) - offset;
> +
> + bytes = iomap_write_end(iter, pos, bytes, bytes, folio);
> + if (WARN_ON_ONCE(bytes == 0))
> return -EIO;
>
> cond_resched();
>
> - pos += status;
> - written += status;
> - length -= status;
> + pos += bytes;
> + written += bytes;
> + length -= bytes;
>
> balance_dirty_pages_ratelimited(iter->inode->i_mapping);
> - } while (length);
> + } while (length > 0);
>
> return written;
> }
