On Fri, Aug 30, 2024 at 01:22:55PM +0530, Kundan Kumar wrote:
> +++ b/block/bio.c
> @@ -931,7 +931,8 @@ static bool bvec_try_merge_page(struct bio_vec *bv, struct page *page,
> if (!zone_device_pages_have_same_pgmap(bv->bv_page, page))
> return false;
>
> - *same_page = ((vec_end_addr & PAGE_MASK) == page_addr);
> + *same_page = ((vec_end_addr & PAGE_MASK) == ((page_addr + off) &
> + PAGE_MASK));
> if (!*same_page) {
> if (IS_ENABLED(CONFIG_KMSAN))
> return false;
This seems like a completely independent change, which has presumably
only now been noticed as a problem, but really should be in a separate
commit and marked for backporting?
> @@ -1280,9 +1312,9 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> unsigned short entries_left = bio->bi_max_vecs - bio->bi_vcnt;
> struct bio_vec *bv = bio->bi_io_vec + bio->bi_vcnt;
> struct page **pages = (struct page **)bv;
> - ssize_t size, left;
> - unsigned len, i = 0;
> - size_t offset;
> + ssize_t size;
> + unsigned int i = 0, num_pages;
I prefer
unsigned int num_pages, i = 0;
but that's a mild preference.
> @@ -1322,17 +1354,28 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> goto out;
> }
>
> - for (left = size, i = 0; left > 0; left -= len, i++) {
> + for (left = size, i = 0; left > 0; left -= len, i += num_pages) {
> struct page *page = pages[i];
> + struct folio *folio = page_folio(page);
> +
> + folio_offset = ((size_t)folio_page_idx(folio, page) <<
> + PAGE_SHIFT) + offset;
> +
> + len = min_t(size_t, (folio_size(folio) - folio_offset), left);
Does this need to be min_t? afaict these are all already size_t.
Other than that last one, looks good.
Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
