block: remove the same_page output argument from bvec_try_merge_page

bvec_try_merge_page currently returns if the added page fragment is
within the same page as the last page in the last current bio_vec.

This information is used by __bio_iov_iter_get_pages so that we always
have a single folio pin per page even when the page is split over
multiple __bio_iov_iter_get_pages calls.

Threading this through the entire lowlevel add page to bio logic is
annoying and inefficient and leads to less code sharing than otherwise
possible.  Instead add code to __bio_iov_iter_get_pages that checks
if the the bio_vecs did not change and thus a merge into the last
segment must have happened, and if there is an offset into the page
for the currently added fragment, because if yes we must have already
had a previous fragment of the same page in the last bio_vec.  While
this is still a bit ugly, it keeps the logic in the one place that
needs it and allows for more code sharing.

Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 608594a154a5b9ce73456c0139be2708de337932..075578d9b03910a8d7186f38cc2bdc8df96fdd8e 100644 (file)
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -132,10 +132,8 @@ int bio_integrity_add_page(struct bio *bio, struct page *page,
 
        if (bip->bip_vcnt > 0) {
                struct bio_vec *bv = &bip->bip_vec[bip->bip_vcnt - 1];
-               bool same_page = false;
 
-               if (bvec_try_merge_hw_page(q, bv, page, len, offset,
-                                          &same_page)) {
+               if (bvec_try_merge_hw_page(q, bv, page, len, offset)) {
                        bip->bip_iter.bi_size += len;
                        return len;
                }

diff --git a/block/bio.c b/block/bio.c
index 4e6c85a33d74dbc1f025c5dbc1965ad8f40aa4c1..18dd2e4b8bdbd48c6a4c9d2e4ba21ad0ccb9f948 100644 (file)
--- a/block/bio.c
+++ b/block/bio.c
@@ -918,7 +918,7 @@ static inline bool bio_full(struct bio *bio, unsigned len)
 }
 
 static bool bvec_try_merge_page(struct bio_vec *bv, struct page *page,
-               unsigned int len, unsigned int off, bool *same_page)
+               unsigned int len, unsigned int off)
 {
        size_t bv_end = bv->bv_offset + bv->bv_len;
        phys_addr_t vec_end_addr = page_to_phys(bv->bv_page) + bv_end - 1;
@@ -931,9 +931,7 @@ 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 + off) &
-                    PAGE_MASK));
-       if (!*same_page) {
+       if ((vec_end_addr & PAGE_MASK) != ((page_addr + off) & PAGE_MASK)) {
                if (IS_ENABLED(CONFIG_KMSAN))
                        return false;
                if (bv->bv_page + bv_end / PAGE_SIZE != page + off / PAGE_SIZE)
@@ -953,8 +951,7 @@ static bool bvec_try_merge_page(struct bio_vec *bv, struct page *page,
  * helpers to split.  Hopefully this will go away soon.
  */
 bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv,
-               struct page *page, unsigned len, unsigned offset,
-               bool *same_page)
+               struct page *page, unsigned len, unsigned offset)
 {
        unsigned long mask = queue_segment_boundary(q);
        phys_addr_t addr1 = bvec_phys(bv);
@@ -964,7 +961,7 @@ bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv,
                return false;
        if (len > queue_max_segment_size(q) - bv->bv_len)
                return false;
-       return bvec_try_merge_page(bv, page, len, offset, same_page);
+       return bvec_try_merge_page(bv, page, len, offset);
 }
 
 /**
@@ -1002,8 +999,6 @@ EXPORT_SYMBOL_GPL(__bio_add_page);
 int bio_add_page(struct bio *bio, struct page *page,
                 unsigned int len, unsigned int offset)
 {
-       bool same_page = false;
-
        if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
                return 0;
        if (bio->bi_iter.bi_size > UINT_MAX - len)
@@ -1011,7 +1006,7 @@ int bio_add_page(struct bio *bio, struct page *page,
 
        if (bio->bi_vcnt > 0 &&
            bvec_try_merge_page(&bio->bi_io_vec[bio->bi_vcnt - 1],
-                               page, len, offset, &same_page)) {
+                               page, len, offset)) {
                bio->bi_iter.bi_size += len;
                return len;
        }
@@ -1088,27 +1083,6 @@ void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)
        bio_set_flag(bio, BIO_CLONED);
 }
 
-static int bio_iov_add_folio(struct bio *bio, struct folio *folio, size_t len,
-                            size_t offset)
-{
-       bool same_page = false;
-
-       if (WARN_ON_ONCE(bio->bi_iter.bi_size > UINT_MAX - len))
-               return -EIO;
-
-       if (bio->bi_vcnt > 0 &&
-           bvec_try_merge_page(&bio->bi_io_vec[bio->bi_vcnt - 1],
-                               folio_page(folio, 0), len, offset,
-                               &same_page)) {
-               bio->bi_iter.bi_size += len;
-               if (same_page && bio_flagged(bio, BIO_PAGE_PINNED))
-                       unpin_user_folio(folio, 1);
-               return 0;
-       }
-       bio_add_folio_nofail(bio, folio, len, offset);
-       return 0;
-}
-
 static unsigned int get_contig_folio_len(unsigned int *num_pages,
                                         struct page **pages, unsigned int i,
                                         struct folio *folio, size_t left,
@@ -1203,6 +1177,7 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
        for (left = size, i = 0; left > 0; left -= len, i += num_pages) {
                struct page *page = pages[i];
                struct folio *folio = page_folio(page);
+               unsigned int old_vcnt = bio->bi_vcnt;
 
                folio_offset = ((size_t)folio_page_idx(folio, page) <<
                               PAGE_SHIFT) + offset;
@@ -1215,7 +1190,23 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
                        len = get_contig_folio_len(&num_pages, pages, i,
                                                   folio, left, offset);
 
-               bio_iov_add_folio(bio, folio, len, folio_offset);
+               if (!bio_add_folio(bio, folio, len, folio_offset)) {
+                       WARN_ON_ONCE(1);
+                       ret = -EINVAL;
+                       goto out;
+               }
+
+               if (bio_flagged(bio, BIO_PAGE_PINNED)) {
+                       /*
+                        * We're adding another fragment of a page that already
+                        * was part of the last segment.  Undo our pin as the
+                        * page was pinned when an earlier fragment of it was
+                        * added to the bio and __bio_release_pages expects a
+                        * single pin per page.
+                        */
+                       if (offset && bio->bi_vcnt == old_vcnt)
+                               unpin_user_folio(folio, 1);
+               }
                offset = 0;
        }
 

diff --git a/block/blk.h b/block/blk.h
index 006e3be433d287215f51688daf53cc57cf225242..595d6f677d4599d51dc269ac14bc26a96bf1b3d3 100644 (file)
--- a/block/blk.h
+++ b/block/blk.h
@@ -100,8 +100,7 @@ struct bio_vec *bvec_alloc(mempool_t *pool, unsigned short *nr_vecs,
 void bvec_free(mempool_t *pool, struct bio_vec *bv, unsigned short nr_vecs);
 
 bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv,
-               struct page *page, unsigned len, unsigned offset,
-               bool *same_page);
+               struct page *page, unsigned len, unsigned offset);
 
 static inline bool biovec_phys_mergeable(struct request_queue *q,
                struct bio_vec *vec1, struct bio_vec *vec2)