On Wed, Jun 10, 2020 at 01:13:14PM -0700, Matthew Wilcox wrote:
> +static inline void bvec_thp_advance(const struct bio_vec *bvec,
> + struct bvec_iter_all *iter_all)
> +{
> + struct bio_vec *bv = &iter_all->bv;
> + unsigned int page_size = thp_size(bvec->bv_page);
> +
> + if (iter_all->done) {
> + bv->bv_page += thp_nr_pages(bv->bv_page);
> + bv->bv_offset = 0;
> + } else {
> + BUG_ON(bvec->bv_offset >= page_size);
> + bv->bv_page = bvec->bv_page;
> + bv->bv_offset = bvec->bv_offset & (page_size - 1);
> + }
> + bv->bv_len = min(page_size - bv->bv_offset,
> + bvec->bv_len - iter_all->done);
> + iter_all->done += bv->bv_len;
> +
> + if (iter_all->done == bvec->bv_len) {
> + iter_all->idx++;
> + iter_all->done = 0;
> + }
> +}
If, for example, we have an order-2 page followed by two order-0 pages
(thanks, generic/127!) in the bvec, we'll end up skipping the third
page because we calculate the size based on bvec->bv_page instead of
bv->bv_page.
+++ b/include/linux/bvec.h
@@ -166,15 +166,19 @@ static inline void bvec_thp_advance(const struct bio_vec *bvec,
struct bvec_iter_all *iter_all)
{
struct bio_vec *bv = &iter_all->bv;
- unsigned int page_size = thp_size(bvec->bv_page);
+ unsigned int page_size;
if (iter_all->done) {
bv->bv_page += thp_nr_pages(bv->bv_page);
+ page_size = thp_size(bv->bv_page);
bv->bv_offset = 0;
} else {
- BUG_ON(bvec->bv_offset >= page_size);
- bv->bv_page = bvec->bv_page;
- bv->bv_offset = bvec->bv_offset & (page_size - 1);
+ bv->bv_page = thp_head(bvec->bv_page +
+ (bvec->bv_offset >> PAGE_SHIFT));
+ page_size = thp_size(bv->bv_page);
+ bv->bv_offset = bvec->bv_offset -
+ (bv->bv_page - bvec->bv_page) * PAGE_SIZE;
+ BUG_ON(bv->bv_offset >= page_size);
}
bv->bv_len = min(page_size - bv->bv_offset,
bvec->bv_len - iter_all->done);
The previous code also wasn't handling the case fixed in 6bedf00e55e5
where a split bio might end up splitting a bvec. That BUG_ON can probably
come out after a few months of testing.
