[SRU,Xenial,Yakkety,1/1] block: relax check on sg gap

Submitted by Joseph Salisbury on April 13, 2017, 10:45 a.m.

Details

Message ID 29eb85e07efa8d2d360aa0ab952bff050a85ead5.1492026266.git.joseph.salisbury@canonical.com
State New
Headers show

Commit Message

Joseph Salisbury April 13, 2017, 10:45 a.m.
From: Ming Lei <ming.lei@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/1682215

If the last bvec of the 1st bio and the 1st bvec of the next
bio are physically contigious, and the latter can be merged
to last segment of the 1st bio, we should think they don't
violate sg gap(or virt boundary) limit.

Both Vitaly and Dexuan reported lots of unmergeable small bios
are observed when running mkfs on Hyper-V virtual storage, and
performance becomes quite low. This patch fixes that performance
issue.

The same issue should exist on NVMe, since it sets virt boundary too.

Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reported-by: Dexuan Cui <decui@microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>
Cc: Keith Busch <keith.busch@intel.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
(cherry picked from commit 729204ef49ec00b788ce23deb9eb922a5769f55d)
Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
---
 include/linux/blkdev.h | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Colin King April 13, 2017, 11:45 a.m.
On 13/04/17 11:45, Joseph Salisbury wrote:
> From: Ming Lei <ming.lei@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1682215
> 
> If the last bvec of the 1st bio and the 1st bvec of the next
> bio are physically contigious, and the latter can be merged
> to last segment of the 1st bio, we should think they don't
> violate sg gap(or virt boundary) limit.
> 
> Both Vitaly and Dexuan reported lots of unmergeable small bios
> are observed when running mkfs on Hyper-V virtual storage, and
> performance becomes quite low. This patch fixes that performance
> issue.
> 
> The same issue should exist on NVMe, since it sets virt boundary too.
> 
> Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Tested-by: Dexuan Cui <decui@microsoft.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> Signed-off-by: Jens Axboe <axboe@fb.com>
> (cherry picked from commit 729204ef49ec00b788ce23deb9eb922a5769f55d)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> ---
>  include/linux/blkdev.h | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index f28a95a..a0b6aae 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1387,6 +1387,25 @@ static inline bool bvec_gap_to_prev(struct request_queue *q,
>  	return __bvec_gap_to_prev(q, bprv, offset);
>  }
>  
> +/*
> + * Check if the two bvecs from two bios can be merged to one segment.
> + * If yes, no need to check gap between the two bios since the 1st bio
> + * and the 1st bvec in the 2nd bio can be handled in one segment.
> + */
> +static inline bool bios_segs_mergeable(struct request_queue *q,
> +		struct bio *prev, struct bio_vec *prev_last_bv,
> +		struct bio_vec *next_first_bv)
> +{
> +	if (!BIOVEC_PHYS_MERGEABLE(prev_last_bv, next_first_bv))
> +		return false;
> +	if (!BIOVEC_SEG_BOUNDARY(q, prev_last_bv, next_first_bv))
> +		return false;
> +	if (prev->bi_seg_back_size + next_first_bv->bv_len >
> +			queue_max_segment_size(q))
> +		return false;
> +	return true;
> +}
> +
>  static inline bool bio_will_gap(struct request_queue *q, struct bio *prev,
>  			 struct bio *next)
>  {
> @@ -1396,7 +1415,8 @@ static inline bool bio_will_gap(struct request_queue *q, struct bio *prev,
>  		bio_get_last_bvec(prev, &pb);
>  		bio_get_first_bvec(next, &nb);
>  
> -		return __bvec_gap_to_prev(q, &pb, nb.bv_offset);
> +		if (!bios_segs_mergeable(q, prev, &pb, &nb))
> +			return __bvec_gap_to_prev(q, &pb, nb.bv_offset);
>  	}
>  
>  	return false;
> 

Good test results, it's a clean cherry pick from upstream that has been
around for a few months and hence given some testing upstream.

Acked-by: Colin Ian King <colin.king@canonical.com>
Seth Forshee April 13, 2017, 3:47 p.m.
On Thu, Apr 13, 2017 at 06:45:15AM -0400, Joseph Salisbury wrote:
> From: Ming Lei <ming.lei@canonical.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1682215
> 
> If the last bvec of the 1st bio and the 1st bvec of the next
> bio are physically contigious, and the latter can be merged
> to last segment of the 1st bio, we should think they don't
> violate sg gap(or virt boundary) limit.
> 
> Both Vitaly and Dexuan reported lots of unmergeable small bios
> are observed when running mkfs on Hyper-V virtual storage, and
> performance becomes quite low. This patch fixes that performance
> issue.
> 
> The same issue should exist on NVMe, since it sets virt boundary too.
> 
> Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Tested-by: Dexuan Cui <decui@microsoft.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> Signed-off-by: Jens Axboe <axboe@fb.com>
> (cherry picked from commit 729204ef49ec00b788ce23deb9eb922a5769f55d)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>

Clean cherry pick, positive test results.

Acked-by: Seth Forshee <seth.forshee@canonical.com>
Stefan Bader April 13, 2017, 4:03 p.m.

Patch hide | download patch | download mbox

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f28a95a..a0b6aae 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1387,6 +1387,25 @@  static inline bool bvec_gap_to_prev(struct request_queue *q,
 	return __bvec_gap_to_prev(q, bprv, offset);
 }
 
+/*
+ * Check if the two bvecs from two bios can be merged to one segment.
+ * If yes, no need to check gap between the two bios since the 1st bio
+ * and the 1st bvec in the 2nd bio can be handled in one segment.
+ */
+static inline bool bios_segs_mergeable(struct request_queue *q,
+		struct bio *prev, struct bio_vec *prev_last_bv,
+		struct bio_vec *next_first_bv)
+{
+	if (!BIOVEC_PHYS_MERGEABLE(prev_last_bv, next_first_bv))
+		return false;
+	if (!BIOVEC_SEG_BOUNDARY(q, prev_last_bv, next_first_bv))
+		return false;
+	if (prev->bi_seg_back_size + next_first_bv->bv_len >
+			queue_max_segment_size(q))
+		return false;
+	return true;
+}
+
 static inline bool bio_will_gap(struct request_queue *q, struct bio *prev,
 			 struct bio *next)
 {
@@ -1396,7 +1415,8 @@  static inline bool bio_will_gap(struct request_queue *q, struct bio *prev,
 		bio_get_last_bvec(prev, &pb);
 		bio_get_first_bvec(next, &nb);
 
-		return __bvec_gap_to_prev(q, &pb, nb.bv_offset);
+		if (!bios_segs_mergeable(q, prev, &pb, &nb))
+			return __bvec_gap_to_prev(q, &pb, nb.bv_offset);
 	}
 
 	return false;