On Fri, Jul 01, 2022 at 06:40:40PM +0100, Al Viro wrote:
> -static void bio_put_pages(struct page **pages, size_t size, size_t off)
> -{
> - size_t i, nr = DIV_ROUND_UP(size + (off & ~PAGE_MASK), PAGE_SIZE);
> -
> - for (i = 0; i < nr; i++)
> - put_page(pages[i]);
> -}
> -
> static int bio_iov_add_page(struct bio *bio, struct page *page,
> unsigned int len, unsigned int offset)
> {
> @@ -1228,11 +1220,11 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
> * the iov data will be picked up in the next bio iteration.
> */
> size = iov_iter_get_pages(iter, pages, LONG_MAX, nr_pages, &offset);
> - if (size > 0)
> - size = ALIGN_DOWN(size, bdev_logical_block_size(bio->bi_bdev));
> if (unlikely(size <= 0))
> return size ? size : -EFAULT;
> + nr_pages = DIV_ROUND_UP(offset + size, PAGE_SIZE);
>
> + size = ALIGN_DOWN(size, bdev_logical_block_size(bio->bi_bdev));
This isn't quite right. The result of the ALIGN_DOWN could be 0, so whatever
page we got before would be leaked since unused pages are only released on an
add_page error. I was about to reply with a patch that fixes this, but here's
the one that I'm currently testing:
---
diff --git a/block/bio.c b/block/bio.c
index 933ea3210954..c4a1ce39c65c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1151,14 +1151,6 @@ void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter)
bio_set_flag(bio, BIO_CLONED);
}
-static void bio_put_pages(struct page **pages, size_t size, size_t off)
-{
- size_t i, nr = DIV_ROUND_UP(size + (off & ~PAGE_MASK), PAGE_SIZE);
-
- for (i = 0; i < nr; i++)
- put_page(pages[i]);
-}
-
static int bio_iov_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
@@ -1208,9 +1200,10 @@ 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;
+ unsigned len, i = 0;
ssize_t size, left;
- unsigned len, i;
size_t offset;
+ int ret;
/*
* Move page array up in the allocated memory for the bio vecs as far as
@@ -1228,14 +1221,19 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
* the iov data will be picked up in the next bio iteration.
*/
size = iov_iter_get_pages(iter, pages, LONG_MAX, nr_pages, &offset);
- if (size > 0)
+ if (size > 0) {
+ nr_pages = DIV_ROUND_UP(size + offset, PAGE_SIZE);
size = ALIGN_DOWN(size, bdev_logical_block_size(bio->bi_bdev));
- if (unlikely(size <= 0))
- return size ? size : -EFAULT;
+ } else
+ nr_pages = 0;
+
+ if (unlikely(size <= 0)) {
+ ret = size ? size : -EFAULT;
+ goto out;
+ }
- for (left = size, i = 0; left > 0; left -= len, i++) {
+ for (left = size; left > 0; left -= len, i++) {
struct page *page = pages[i];
- int ret;
len = min_t(size_t, PAGE_SIZE - offset, left);
if (bio_op(bio) == REQ_OP_ZONE_APPEND)
@@ -1244,15 +1242,19 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
else
ret = bio_iov_add_page(bio, page, len, offset);
- if (ret) {
- bio_put_pages(pages + i, left, offset);
- return ret;
- }
+ if (ret)
+ goto out;
offset = 0;
}
iov_iter_advance(iter, size);
- return 0;
+out:
+ while (i < nr_pages) {
+ put_page(pages[i]);
+ i++;
+ }
+
+ return ret;
}
/**
--
