linux-kernel - Re: [PATCH v6 20/51] block: Add bio_for_each_thp_segment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Jun 2020 11:20:29 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     linux-fsdevel@...r.kernel.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 20/51] block: Add bio_for_each_thp_segment_all

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.