[PATCH 2/5] block: refactor __bio_iov_bvec_add_pages

[PATCH 2/5] block: refactor __bio_iov_bvec_add_pages

[Christoph Hellwig]

Return early on error, and add an unlikely annotation for that case.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index c2592c5d70b9..ad346082a971 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -873,20 +873,19 @@ static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
 	len = min_t(size_t, bv->bv_len - iter->iov_offset, iter->count);
 	size = bio_add_page(bio, bv->bv_page, len,
 				bv->bv_offset + iter->iov_offset);
-	if (size == len) {
-		if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
-			struct page *page;
-			int i;
+	if (unlikely(size != len))
+		return -EINVAL;
 
-			mp_bvec_for_each_page(page, bv, i)
-				get_page(page);
-		}
+	if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
+		struct page *page;
+		int i;
 
-		iov_iter_advance(iter, size);
-		return 0;
+		mp_bvec_for_each_page(page, bv, i)
+			get_page(page);
 	}
 
-	return -EINVAL;
+	iov_iter_advance(iter, size);
+	return 0;
 }
 
 #define PAGE_PTRS_PER_BVEC     (sizeof(struct bio_vec) / sizeof(struct page *))
-- 
2.20.1

Re: [PATCH 2/5] block: refactor __bio_iov_bvec_add_pages

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 2/5] block: refactor __bio_iov_bvec_add_pages

[Bart Van Assche]

On Mon, 2019-04-08 at 12:46 +0200, Christoph Hellwig wrote:
> Return early on error, and add an unlikely annotation for that case.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

avoid calling nth_page in the block I/O path v2

[Christoph Hellwig]

Hi Jens,

you complained about the overhead of calling nth_page in the multipage
bio_vec enabled I/O path a while ago.  While I can't really reproduce
the numbers on my (slower) hardware we could avoid the nth_page calls
pretty easily with a few tweaks.  Can you take a look at this series?

Changes since v1:
 - remove a spurious variable initialization

[PATCH 1/5] block: rewrite blk_bvec_map_sg to avoid a nth_page call

[Christoph Hellwig]

The offset in scatterlists is allowed to be larger than the page size,
so don't go to great length to avoid that case and simplify the
arithmetics.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/blk-merge.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 895795cdb145..247b17f2a0f6 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -469,26 +469,17 @@ static unsigned blk_bvec_map_sg(struct request_queue *q,
 		struct scatterlist **sg)
 {
 	unsigned nbytes = bvec->bv_len;
-	unsigned nsegs = 0, total = 0, offset = 0;
+	unsigned nsegs = 0, total = 0;
 
 	while (nbytes > 0) {
-		unsigned seg_size;
-		struct page *pg;
-		unsigned idx;
+		unsigned offset = bvec->bv_offset + total;
+		unsigned len = min(get_max_segment_size(q, offset), nbytes);
 
 		*sg = blk_next_sg(sg, sglist);
+		sg_set_page(*sg, bvec->bv_page, len, offset);
 
-		seg_size = get_max_segment_size(q, bvec->bv_offset + total);
-		seg_size = min(nbytes, seg_size);
-
-		offset = (total + bvec->bv_offset) % PAGE_SIZE;
-		idx = (total + bvec->bv_offset) / PAGE_SIZE;
-		pg = bvec_nth_page(bvec->bv_page, idx);
-
-		sg_set_page(*sg, pg, seg_size, offset);
-
-		total += seg_size;
-		nbytes -= seg_size;
+		total += len;
+		nbytes -= len;
 		nsegs++;
 	}
 
-- 
2.20.1

[PATCH 2/5] block: refactor __bio_iov_bvec_add_pages

[Christoph Hellwig]

Return early on error, and add an unlikely annotation for that case.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/bio.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index c2592c5d70b9..ad346082a971 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -873,20 +873,19 @@ static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
 	len = min_t(size_t, bv->bv_len - iter->iov_offset, iter->count);
 	size = bio_add_page(bio, bv->bv_page, len,
 				bv->bv_offset + iter->iov_offset);
-	if (size == len) {
-		if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
-			struct page *page;
-			int i;
+	if (unlikely(size != len))
+		return -EINVAL;
 
-			mp_bvec_for_each_page(page, bv, i)
-				get_page(page);
-		}
+	if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
+		struct page *page;
+		int i;
 
-		iov_iter_advance(iter, size);
-		return 0;
+		mp_bvec_for_each_page(page, bv, i)
+			get_page(page);
 	}
 
-	return -EINVAL;
+	iov_iter_advance(iter, size);
+	return 0;
 }
 
 #define PAGE_PTRS_PER_BVEC     (sizeof(struct bio_vec) / sizeof(struct page *))
-- 
2.20.1

[PATCH 3/5] block: don't allow multiple bio_iov_iter_get_pages calls per bio

[Christoph Hellwig]

No caller uses bio_iov_iter_get_pages multiple times on a given bio,
and that funtionality isn't all that useful.  Removing it will make
some future changes a little easier and also simplifies the function
a bit.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 block/bio.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index ad346082a971..c2a389b1509a 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -958,7 +958,10 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 {
 	const bool is_bvec = iov_iter_is_bvec(iter);
-	unsigned short orig_vcnt = bio->bi_vcnt;
+	int ret;
+
+	if (WARN_ON_ONCE(bio->bi_vcnt))
+		return -EINVAL;
 
 	/*
 	 * If this is a BVEC iter, then the pages are kernel pages. Don't
@@ -968,19 +971,13 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 		bio_set_flag(bio, BIO_NO_PAGE_REF);
 
 	do {
-		int ret;
-
 		if (is_bvec)
 			ret = __bio_iov_bvec_add_pages(bio, iter);
 		else
 			ret = __bio_iov_iter_get_pages(bio, iter);
+	} while (!ret && iov_iter_count(iter) && !bio_full(bio));
 
-		if (unlikely(ret))
-			return bio->bi_vcnt > orig_vcnt ? 0 : ret;
-
-	} while (iov_iter_count(iter) && !bio_full(bio));
-
-	return 0;
+	return bio->bi_vcnt ? 0 : ret;
 }
 
 static void submit_bio_wait_endio(struct bio *bio)
-- 
2.20.1

[PATCH 4/5] block: change how we get page references in bio_iov_iter_get_pages

[Christoph Hellwig]

Instead of needing a special macro to iterate over all pages in
a bvec just do a second passs over the whole bio.  This also matches
what we do on the release side.  The release side helper is moved
up to where we need the get helper to clearly express the symmetry.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c          | 51 ++++++++++++++++++++++----------------------
 include/linux/bvec.h |  5 -----
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index c2a389b1509a..d3490aeb1a7e 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -861,6 +861,26 @@ int bio_add_page(struct bio *bio, struct page *page,
 }
 EXPORT_SYMBOL(bio_add_page);
 
+static void bio_get_pages(struct bio *bio)
+{
+	struct bvec_iter_all iter_all;
+	struct bio_vec *bvec;
+	int i;
+
+	bio_for_each_segment_all(bvec, bio, i, iter_all)
+		get_page(bvec->bv_page);
+}
+
+static void bio_release_pages(struct bio *bio)
+{
+	struct bvec_iter_all iter_all;
+	struct bio_vec *bvec;
+	int i;
+
+	bio_for_each_segment_all(bvec, bio, i, iter_all)
+		put_page(bvec->bv_page);
+}
+
 static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
 {
 	const struct bio_vec *bv = iter->bvec;
@@ -875,15 +895,6 @@ static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
 				bv->bv_offset + iter->iov_offset);
 	if (unlikely(size != len))
 		return -EINVAL;
-
-	if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
-		struct page *page;
-		int i;
-
-		mp_bvec_for_each_page(page, bv, i)
-			get_page(page);
-	}
-
 	iov_iter_advance(iter, size);
 	return 0;
 }
@@ -963,13 +974,6 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 	if (WARN_ON_ONCE(bio->bi_vcnt))
 		return -EINVAL;
 
-	/*
-	 * If this is a BVEC iter, then the pages are kernel pages. Don't
-	 * release them on IO completion, if the caller asked us to.
-	 */
-	if (is_bvec && iov_iter_bvec_no_ref(iter))
-		bio_set_flag(bio, BIO_NO_PAGE_REF);
-
 	do {
 		if (is_bvec)
 			ret = __bio_iov_bvec_add_pages(bio, iter);
@@ -977,6 +981,11 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
 			ret = __bio_iov_iter_get_pages(bio, iter);
 	} while (!ret && iov_iter_count(iter) && !bio_full(bio));
 
+	if (iov_iter_bvec_no_ref(iter))
+		bio_set_flag(bio, BIO_NO_PAGE_REF);
+	else
+		bio_get_pages(bio);
+
 	return bio->bi_vcnt ? 0 : ret;
 }
 
@@ -1670,16 +1679,6 @@ void bio_set_pages_dirty(struct bio *bio)
 	}
 }
 
-static void bio_release_pages(struct bio *bio)
-{
-	struct bio_vec *bvec;
-	int i;
-	struct bvec_iter_all iter_all;
-
-	bio_for_each_segment_all(bvec, bio, i, iter_all)
-		put_page(bvec->bv_page);
-}
-
 /*
  * bio_check_pages_dirty() will check that all the BIO's pages are still dirty.
  * If they are, then fine.  If, however, some pages are clean then they must
diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index f6275c4da13a..307bbda62b7b 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -189,9 +189,4 @@ static inline void mp_bvec_last_segment(const struct bio_vec *bvec,
 	}
 }
 
-#define mp_bvec_for_each_page(pg, bv, i)				\
-	for (i = (bv)->bv_offset / PAGE_SIZE;				\
-		(i <= (((bv)->bv_offset + (bv)->bv_len - 1) / PAGE_SIZE)) && \
-		(pg = bvec_nth_page((bv)->bv_page, i)); i += 1)
-
 #endif /* __LINUX_BVEC_ITER_H */
-- 
2.20.1

[PATCH 5/5] block: only allow contiguous page structs in a bio_vec

[Christoph Hellwig]

We currently have to call nth_page when iterating over pages inside a
bio_vec.  Jens complained a while ago that this is fairly expensive.
To mitigate this we can check that that the actual page structures
are contiguous when adding them to the bio, and just do check pointer
arithmetics later on.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c          |  9 +++++++--
 include/linux/bvec.h | 13 ++++---------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index d3490aeb1a7e..8adc2a20d57d 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -659,8 +659,13 @@ static inline bool page_is_mergeable(const struct bio_vec *bv,
 		return false;
 	if (xen_domain() && !xen_biovec_phys_mergeable(bv, page))
 		return false;
-	if (same_page && (vec_end_addr & PAGE_MASK) != page_addr)
-		return false;
+
+	if ((vec_end_addr & PAGE_MASK) != page_addr) {
+		if (same_page)
+			return false;
+		if (pfn_to_page(PFN_DOWN(vec_end_addr)) + 1 != page)
+			return false;
+	}
 
 	return true;
 }
diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index 307bbda62b7b..44b0f4684190 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -51,11 +51,6 @@ struct bvec_iter_all {
 	unsigned	done;
 };
 
-static inline struct page *bvec_nth_page(struct page *page, int idx)
-{
-	return idx == 0 ? page : nth_page(page, idx);
-}
-
 /*
  * various member access, note that bio_data should of course not be used
  * on highmem page vectors
@@ -92,8 +87,8 @@ static inline struct page *bvec_nth_page(struct page *page, int idx)
 	      PAGE_SIZE - bvec_iter_offset((bvec), (iter)))
 
 #define bvec_iter_page(bvec, iter)				\
-	bvec_nth_page(mp_bvec_iter_page((bvec), (iter)),		\
-		      mp_bvec_iter_page_idx((bvec), (iter)))
+	(mp_bvec_iter_page((bvec), (iter)) +			\
+	 mp_bvec_iter_page_idx((bvec), (iter)))
 
 #define bvec_iter_bvec(bvec, iter)				\
 ((struct bio_vec) {						\
@@ -157,7 +152,7 @@ static inline void mp_bvec_next_segment(const struct bio_vec *bvec,
 	struct bio_vec *bv = &iter_all->bv;
 
 	if (bv->bv_page) {
-		bv->bv_page = nth_page(bv->bv_page, 1);
+		bv->bv_page++;
 		bv->bv_offset = 0;
 	} else {
 		bv->bv_page = bvec->bv_page;
@@ -177,7 +172,7 @@ static inline void mp_bvec_last_segment(const struct bio_vec *bvec,
 	unsigned total = bvec->bv_offset + bvec->bv_len;
 	unsigned last_page = (total - 1) / PAGE_SIZE;
 
-	seg->bv_page = bvec_nth_page(bvec->bv_page, last_page);
+	seg->bv_page = bvec->bv_page + last_page;
 
 	/* the whole segment is inside the last page */
 	if (bvec->bv_offset >= last_page * PAGE_SIZE) {
-- 
2.20.1

Re: [PATCH 2/5] block: refactor __bio_iov_bvec_add_pages

[Ming Lei]

On Thu, Apr 11, 2019 at 08:23:28AM +0200, Christoph Hellwig wrote:
> Return early on error, and add an unlikely annotation for that case.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  block/bio.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index c2592c5d70b9..ad346082a971 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -873,20 +873,19 @@ static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
>  	len = min_t(size_t, bv->bv_len - iter->iov_offset, iter->count);
>  	size = bio_add_page(bio, bv->bv_page, len,
>  				bv->bv_offset + iter->iov_offset);
> -	if (size == len) {
> -		if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
> -			struct page *page;
> -			int i;
> +	if (unlikely(size != len))
> +		return -EINVAL;
>  
> -			mp_bvec_for_each_page(page, bv, i)
> -				get_page(page);
> -		}
> +	if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
> +		struct page *page;
> +		int i;
>  
> -		iov_iter_advance(iter, size);
> -		return 0;
> +		mp_bvec_for_each_page(page, bv, i)
> +			get_page(page);
>  	}
>  
> -	return -EINVAL;
> +	iov_iter_advance(iter, size);
> +	return 0;
>  }
>  
>  #define PAGE_PTRS_PER_BVEC     (sizeof(struct bio_vec) / sizeof(struct page *))
> -- 
> 2.20.1
> 

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

Re: [PATCH 3/5] block: don't allow multiple bio_iov_iter_get_pages calls per bio

[Ming Lei]

On Thu, Apr 11, 2019 at 08:23:29AM +0200, Christoph Hellwig wrote:
> No caller uses bio_iov_iter_get_pages multiple times on a given bio,
> and that funtionality isn't all that useful.  Removing it will make
> some future changes a little easier and also simplifies the function
> a bit.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  block/bio.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index ad346082a971..c2a389b1509a 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -958,7 +958,10 @@ static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>  int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>  {
>  	const bool is_bvec = iov_iter_is_bvec(iter);
> -	unsigned short orig_vcnt = bio->bi_vcnt;
> +	int ret;
> +
> +	if (WARN_ON_ONCE(bio->bi_vcnt))
> +		return -EINVAL;
>  
>  	/*
>  	 * If this is a BVEC iter, then the pages are kernel pages. Don't
> @@ -968,19 +971,13 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>  		bio_set_flag(bio, BIO_NO_PAGE_REF);
>  
>  	do {
> -		int ret;
> -
>  		if (is_bvec)
>  			ret = __bio_iov_bvec_add_pages(bio, iter);
>  		else
>  			ret = __bio_iov_iter_get_pages(bio, iter);
> +	} while (!ret && iov_iter_count(iter) && !bio_full(bio));
>  
> -		if (unlikely(ret))
> -			return bio->bi_vcnt > orig_vcnt ? 0 : ret;
> -
> -	} while (iov_iter_count(iter) && !bio_full(bio));
> -
> -	return 0;
> +	return bio->bi_vcnt ? 0 : ret;
>  }
>  
>  static void submit_bio_wait_endio(struct bio *bio)
> -- 
> 2.20.1
> 

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

Re: [PATCH 4/5] block: change how we get page references in bio_iov_iter_get_pages

[Ming Lei]

On Thu, Apr 11, 2019 at 08:23:30AM +0200, Christoph Hellwig wrote:
> Instead of needing a special macro to iterate over all pages in
> a bvec just do a second passs over the whole bio.  This also matches
> what we do on the release side.  The release side helper is moved
> up to where we need the get helper to clearly express the symmetry.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/bio.c          | 51 ++++++++++++++++++++++----------------------
>  include/linux/bvec.h |  5 -----
>  2 files changed, 25 insertions(+), 31 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index c2a389b1509a..d3490aeb1a7e 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -861,6 +861,26 @@ int bio_add_page(struct bio *bio, struct page *page,
>  }
>  EXPORT_SYMBOL(bio_add_page);
>  
> +static void bio_get_pages(struct bio *bio)
> +{
> +	struct bvec_iter_all iter_all;
> +	struct bio_vec *bvec;
> +	int i;
> +
> +	bio_for_each_segment_all(bvec, bio, i, iter_all)
> +		get_page(bvec->bv_page);
> +}
> +
> +static void bio_release_pages(struct bio *bio)
> +{
> +	struct bvec_iter_all iter_all;
> +	struct bio_vec *bvec;
> +	int i;
> +
> +	bio_for_each_segment_all(bvec, bio, i, iter_all)
> +		put_page(bvec->bv_page);
> +}
> +
>  static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
>  {
>  	const struct bio_vec *bv = iter->bvec;
> @@ -875,15 +895,6 @@ static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
>  				bv->bv_offset + iter->iov_offset);
>  	if (unlikely(size != len))
>  		return -EINVAL;
> -
> -	if (!bio_flagged(bio, BIO_NO_PAGE_REF)) {
> -		struct page *page;
> -		int i;
> -
> -		mp_bvec_for_each_page(page, bv, i)
> -			get_page(page);
> -	}
> -
>  	iov_iter_advance(iter, size);
>  	return 0;
>  }
> @@ -963,13 +974,6 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>  	if (WARN_ON_ONCE(bio->bi_vcnt))
>  		return -EINVAL;
>  
> -	/*
> -	 * If this is a BVEC iter, then the pages are kernel pages. Don't
> -	 * release them on IO completion, if the caller asked us to.
> -	 */
> -	if (is_bvec && iov_iter_bvec_no_ref(iter))
> -		bio_set_flag(bio, BIO_NO_PAGE_REF);
> -
>  	do {
>  		if (is_bvec)
>  			ret = __bio_iov_bvec_add_pages(bio, iter);
> @@ -977,6 +981,11 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
>  			ret = __bio_iov_iter_get_pages(bio, iter);
>  	} while (!ret && iov_iter_count(iter) && !bio_full(bio));
>  
> +	if (iov_iter_bvec_no_ref(iter))
> +		bio_set_flag(bio, BIO_NO_PAGE_REF);
> +	else
> +		bio_get_pages(bio);
> +
>  	return bio->bi_vcnt ? 0 : ret;
>  }
>  
> @@ -1670,16 +1679,6 @@ void bio_set_pages_dirty(struct bio *bio)
>  	}
>  }
>  
> -static void bio_release_pages(struct bio *bio)
> -{
> -	struct bio_vec *bvec;
> -	int i;
> -	struct bvec_iter_all iter_all;
> -
> -	bio_for_each_segment_all(bvec, bio, i, iter_all)
> -		put_page(bvec->bv_page);
> -}
> -
>  /*
>   * bio_check_pages_dirty() will check that all the BIO's pages are still dirty.
>   * If they are, then fine.  If, however, some pages are clean then they must
> diff --git a/include/linux/bvec.h b/include/linux/bvec.h
> index f6275c4da13a..307bbda62b7b 100644
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -189,9 +189,4 @@ static inline void mp_bvec_last_segment(const struct bio_vec *bvec,
>  	}
>  }
>  
> -#define mp_bvec_for_each_page(pg, bv, i)				\
> -	for (i = (bv)->bv_offset / PAGE_SIZE;				\
> -		(i <= (((bv)->bv_offset + (bv)->bv_len - 1) / PAGE_SIZE)) && \
> -		(pg = bvec_nth_page((bv)->bv_page, i)); i += 1)
> -
>  #endif /* __LINUX_BVEC_ITER_H */
> -- 
> 2.20.1
> 

Reviewed-by: Ming Lei <ming.lei@redhat.com>

-- 
Ming

Re: [PATCH 3/5] block: don't allow multiple bio_iov_iter_get_pages calls per bio

[Bart Van Assche]

On Thu, 2019-04-11 at 08:23 +0200, Christoph Hellwig wrote:
> No caller uses bio_iov_iter_get_pages multiple times on a given bio,
> and that funtionality isn't all that useful.  Removing it will make
> some future changes a little easier and also simplifies the function
> a bit.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: avoid calling nth_page in the block I/O path v2

[Jens Axboe]

On 4/11/19 12:23 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> you complained about the overhead of calling nth_page in the multipage
> bio_vec enabled I/O path a while ago.  While I can't really reproduce
> the numbers on my (slower) hardware we could avoid the nth_page calls
> pretty easily with a few tweaks.  Can you take a look at this series?
> 
> Changes since v1:
>  - remove a spurious variable initialization

Applied for 5.2, thanks.

-- 
Jens Axboe