Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1276019 > unrolled thread

[PATCH 0/3] blk-merge: fix sg merge regression

Started byMing Lei <ming.lei@canonical.com>
First post2015-11-24 03:40 +0100
Last post2015-11-24 04:20 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] blk-merge: fix sg merge regression Ming Lei <ming.lei@canonical.com> - 2015-11-24 03:40 +0100
    [PATCH 2/3] blk-merge: fix blk_bio_segment_split Ming Lei <ming.lei@canonical.com> - 2015-11-24 03:40 +0100
    [PATCH 1/3] block: fix segment split Ming Lei <ming.lei@canonical.com> - 2015-11-24 03:40 +0100
    [PATCH 3/3] blk-merge: warn if figured out segment number is bigger than nr_phys_segments Ming Lei <ming.lei@canonical.com> - 2015-11-24 03:40 +0100
    Re: [PATCH 0/3] blk-merge: fix sg merge regression Jens Axboe <axboe@kernel.dk> - 2015-11-24 04:20 +0100

#1276019 — [PATCH 0/3] blk-merge: fix sg merge regression

FromMing Lei <ming.lei@canonical.com>
Date2015-11-24 03:40 +0100
Subject[PATCH 0/3] blk-merge: fix sg merge regression
Message-ID<qybGp-1Y4-1@gated-at.bofh.it>
The 1st two patches fix regressions on SG merge, and the 3rd one
adds one warning in blk_rq_map_sg() so that this kind of issue
can be reported a bit easier.

Thanks,
Ming


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1276021 — [PATCH 2/3] blk-merge: fix blk_bio_segment_split

FromMing Lei <ming.lei@canonical.com>
Date2015-11-24 03:40 +0100
Subject[PATCH 2/3] blk-merge: fix blk_bio_segment_split
Message-ID<qybGp-1Y4-9@gated-at.bofh.it>
In reply to#1276019
Commit bdced438acd83a(block: setup bi_phys_segments after
splitting) introduces function of computing bio->bi_phys_segments
during bio splitting.

Unfortunately both bio->bi_seg_front_size and bio->bi_seg_back_size
arn't computed, so too many physical segments may be obtained
for one request since both the two are used to check if one segment
across two bios can be possible.

This patch fixes the issue by computing the two variables in
blk_bio_segment_split().

Fixes: bdced438acd83a(block: setup bi_phys_segments after splitting)
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Mark Salter <msalter@redhat.com>
Tested-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Tested-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-merge.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index f2efe8a..50793cd 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -76,6 +76,9 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 	struct bio_vec bv, bvprv, *bvprvp = NULL;
 	struct bvec_iter iter;
 	unsigned seg_size = 0, nsegs = 0, sectors = 0;
+	unsigned front_seg_size = bio->bi_seg_front_size;
+	bool do_split = true;
+	struct bio *new = NULL;
 
 	bio_for_each_segment(bv, bio, iter) {
 		if (sectors + (bv.bv_len >> 9) > queue_max_sectors(q))
@@ -111,13 +114,26 @@ new_segment:
 		bvprvp = &bvprv;
 		seg_size = bv.bv_len;
 		sectors += bv.bv_len >> 9;
+
+		if (nsegs == 1 && seg_size > front_seg_size)
+			front_seg_size = seg_size;
 	}
 
-	*segs = nsegs;
-	return NULL;
+	do_split = false;
 split:
 	*segs = nsegs;
-	return bio_split(bio, sectors, GFP_NOIO, bs);
+
+	if (do_split) {
+		new = bio_split(bio, sectors, GFP_NOIO, bs);
+		if (new)
+			bio = new;
+	}
+
+	bio->bi_seg_front_size = front_seg_size;
+	if (seg_size > bio->bi_seg_back_size)
+		bio->bi_seg_back_size = seg_size;
+
+	return do_split ? new : NULL;
 }
 
 void blk_queue_split(struct request_queue *q, struct bio **bio,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276024 — [PATCH 1/3] block: fix segment split

FromMing Lei <ming.lei@canonical.com>
Date2015-11-24 03:40 +0100
Subject[PATCH 1/3] block: fix segment split
Message-ID<qybGp-1Y4-13@gated-at.bofh.it>
In reply to#1276019
Inside blk_bio_segment_split(), previous bvec pointer(bvprvp)
always points to the iterator local variable, which is obviously
wrong, so fix it by pointing to the local variable of 'bvprv'.

Fixes: 5014c311baa2b(block: fix bogus compiler warnings in blk-merge.c)
Cc: stable@kernel.org #4.3
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Mark Salter <msalter@redhat.com>
Tested-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Tested-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-merge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index de5716d8..f2efe8a 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -98,7 +98,7 @@ static struct bio *blk_bio_segment_split(struct request_queue *q,
 
 			seg_size += bv.bv_len;
 			bvprv = bv;
-			bvprvp = &bv;
+			bvprvp = &bvprv;
 			sectors += bv.bv_len >> 9;
 			continue;
 		}
@@ -108,7 +108,7 @@ new_segment:
 
 		nsegs++;
 		bvprv = bv;
-		bvprvp = &bv;
+		bvprvp = &bvprv;
 		seg_size = bv.bv_len;
 		sectors += bv.bv_len >> 9;
 	}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276028 — [PATCH 3/3] blk-merge: warn if figured out segment number is bigger than nr_phys_segments

FromMing Lei <ming.lei@canonical.com>
Date2015-11-24 03:40 +0100
Subject[PATCH 3/3] blk-merge: warn if figured out segment number is bigger than nr_phys_segments
Message-ID<qybGq-1Y4-15@gated-at.bofh.it>
In reply to#1276019
We had seen lots of reports of this kind issue, so add one
warnning in blk-merge, then it can be triggered easily and
avoid to depend on warning/bug from drivers.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-merge.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 50793cd..41a55ba 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -428,6 +428,12 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 	if (sg)
 		sg_mark_end(sg);
 
+	/*
+	 * Something must have been wrong if the figured number of
+	 * segment is bigger than number of req's physical segments
+	 */
+	WARN_ON(nsegs > rq->nr_phys_segments);
+
 	return nsegs;
 }
 EXPORT_SYMBOL(blk_rq_map_sg);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1276034

FromJens Axboe <axboe@kernel.dk>
Date2015-11-24 04:20 +0100
Message-ID<qycj8-2xN-17@gated-at.bofh.it>
In reply to#1276019
On 11/23/2015 07:35 PM, Ming Lei wrote:
> The 1st two patches fix regressions on SG merge, and the 3rd one
> adds one warning in blk_rq_map_sg() so that this kind of issue
> can be reported a bit easier.

Thanks Ming, applied all 3.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web