On Thu, Oct 9, 2014 at 10:26 PM, Jeff Mahoney <[email protected]> wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 10/9/14, 9:53 AM, Jeff Moyer wrote: >> Jeff Mahoney <[email protected]> writes: >> >>> Commit 05f1dd53152173 (block: add queue flag for disabling SG >>> merging) uses bi_vcnt to assign bio->bi_phys_segments if sg >>> merging is disabled. When using device mapper on top of a blk-mq >>> device (virtio_blk in my test), we'd end up overflowing the >>> scatterlist in __blk_bios_map_sg. >>> >>> __bio_clone_fast copies bi_iter and bi_io_vec but not bi_vcnt, >>> so blk_recount_segments would report bi_phys_segments as 0. >>> Since rq->nr_phys_segments is 0 as well, the checks to ensure >>> that we don't exceed the queue's segment limit end up allowing >>> more bios (and segments) to attach the a request until we finally >>> map it. That also means we pass the BUG_ON at the beginning of >>> virtio_queue_rq, ultimately causing memory corruption and a >>> crash. >>> >>> If we copy bi_vcnt in __bio_clone_fast, the bios and requests >>> properly report the number of segments and everything works as >>> expected. >>> >>> Originally reported at >>> http://bugzilla.opensuse.org/show_bug.cgi?id=888259 >> >> Hi, Jeff, >> >> Did you manage to reproduce this problem with commit 0738854 >> (blk-merge: fix blk_recount_segments) applied? Or perhaps with >> commit 200612e (dm table: propagate QUEUE_FLAG_NO_SG_MERGE)? > > Yep. I was able to reproduce it with 3.17. I did try 0738854 when I > was still using 3.16 since it looked like a good candidate. Neither of > those patches affect the problem here. bio->bi_phys_segments never > gets a value set in the fast clone case and that translates to > req->nr_phys_segments never getting properly accumulated. That might > not be a problem except that the NO_SG_MERGE behavior bypasses the > iteration that would come up with the correct value. In either case,
This patch may get incorrect rq->nr_phys_segments because bio cloning is often used in case of I/O splitting, so could you test if the attached patch fixes your problem? Thanks, -- Ming Lei
From d3946db93c8eb1da4a0c32249d603f415febf09e Mon Sep 17 00:00:00 2001 From: Ming Lei <[email protected]> Date: Thu, 9 Oct 2014 23:17:35 +0800 Subject: [PATCH] blk-merge: don't compute bi_phys_segments from bi_vcnt for cloned bio It isn't correct to figure out req->bi_phys_segments from bio->bi_vcnt if the bio is cloned. Signed-off-by: Ming Lei <[email protected]> --- block/blk-merge.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/blk-merge.c b/block/blk-merge.c index f71bad3..d5cad1a 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -98,6 +98,7 @@ void blk_recalc_rq_segments(struct request *rq) void blk_recount_segments(struct request_queue *q, struct bio *bio) { if (test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags) && + !bio_flagged(bio, BIO_CLONED) && bio->bi_vcnt < queue_max_segments(q)) bio->bi_phys_segments = bio->bi_vcnt; else { -- 1.7.9.5
