On Wed, Feb 26, 2025 at 10:02:34AM -0800, Darrick J. Wong wrote:
> > if (!bp->b_error && is_vmalloc_addr(bp->b_addr))
> > invalidate_kernel_vmap_range(bp->b_addr,
> > - xfs_buf_vmap_len(bp));
> > + DIV_ROUND_UP(BBTOB(bp->b_length),
> > + PAGE_SIZE));
>
> The second argument to invalidate_kernel_vmap_range is the number of
> bytes, right?
Yes.
> Isn't this BBTOB() without the DIV_ROUND_UP? Or do you
> actually want roundup(BBTOB(b_length), PAGE_SIZE) here?
Yes.
> > - if (bp->b_page_count == 1) {
> > - __bio_add_page(bio, virt_to_page(bp->b_addr), size,
> > - offset_in_page(bp->b_addr));
> > - } else {
> > - for (p = 0; p < bp->b_page_count; p++)
> > - __bio_add_page(bio, bp->b_pages[p], PAGE_SIZE, 0);
> > - bio->bi_iter.bi_size = size; /* limit to the actual size used */
> > + bio = bio_alloc(bp->b_target->bt_bdev, size >> PAGE_SHIFT,
>
> Is the second argument (size >> PAGE_SHIFT) supposed to be the number of
> pages that we're going to __bio_add_page to the bio?
Yes.
> In which case, shouldn't it be alloc_size ?
Yes.
> > + } while (size);
> >
> > - if (is_vmalloc_addr(bp->b_addr))
> > - flush_kernel_vmap_range(bp->b_addr,
> > - xfs_buf_vmap_len(bp));
> > + flush_kernel_vmap_range(bp->b_addr, alloc_size);
>
> ...and this one is roundup(size, PAGE_SIZE) isn't it?
Yes.
