Re: [BUG] "block: make generic_make_request handle arbitrarily sized bios" breaks boot on parisc-linux

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 24, 2016 at 10:28 AM, John David Anglin
<dave.anglin@xxxxxxxx> wrote:
> The following block change breaks boot on parisc-linux:
>
> commit 54efd50bfd873e2dbf784e0b21a8027ba4299a3e
> Author: Kent Overstreet <kent.overstreet@xxxxxxxxx>
> Date:   Thu Apr 23 22:37:18 2015 -0700
>
>    block: make generic_make_request handle arbitrarily sized bios
>
>    The way the block layer is currently written, it goes to great lengths
>    to avoid having to split bios; upper layer code (such as bio_add_page())
>    checks what the underlying device can handle and tries to always create
>    bios that don't need to be split.
>
>    But this approach becomes unwieldy and eventually breaks down with
>    stacked devices and devices with dynamic limits, and it adds a lot of
>    complexity. If the block layer could split bios as needed, we could
>    eliminate a lot of complexity elsewhere - particularly in stacked
>    drivers. Code that creates bios can then create whatever size bios are
>    convenient, and more importantly stacked drivers don't have to deal with
>    both their own bio size limitations and the limitations of the
>    (potentially multiple) devices underneath them.  In the future this will
>    let us delete merge_bvec_fn and a bunch of other code.
>
>    We do this by adding calls to blk_queue_split() to the various
>    make_request functions that need it - a few can already handle arbitrary
>    size bios. Note that we add the call _after_ any call to
>    blk_queue_bounce(); this means that blk_queue_split() and
>    blk_recalc_rq_segments() don't need to be concerned with bouncing
>    affecting segment merging.
>
>    Some make_request_fn() callbacks were simple enough to audit and verify
>    they don't need blk_queue_split() calls. The skipped ones are:
>
>     * nfhd_make_request (arch/m68k/emu/nfblock.c)
>     * axon_ram_make_request (arch/powerpc/sysdev/axonram.c)
>     * simdisk_make_request (arch/xtensa/platforms/iss/simdisk.c)
>     * brd_make_request (ramdisk - drivers/block/brd.c)
>     * mtip_submit_request (drivers/block/mtip32xx/mtip32xx.c)
>     * loop_make_request
>     * null_queue_bio
>     * bcache's make_request fns
>
>    Some others are almost certainly safe to remove now, but will be left
>    for future patches.
>
>    Cc: Jens Axboe <axboe@xxxxxxxxx>
>    Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
>    Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
>    Cc: Ming Lei <ming.lei@xxxxxxxxxxxxx>
>    Cc: Neil Brown <neilb@xxxxxxx>
>    Cc: Alasdair Kergon <agk@xxxxxxxxxx>
>    Cc: Mike Snitzer <snitzer@xxxxxxxxxx>
>    Cc: dm-devel@xxxxxxxxxx
>    Cc: Lars Ellenberg <drbd-dev@xxxxxxxxxxxxxxxx>
>    Cc: drbd-user@xxxxxxxxxxxxxxxx
>    Cc: Jiri Kosina <jkosina@xxxxxxx>
>    Cc: Geoff Levand <geoff@xxxxxxxxxxxxx>
>    Cc: Jim Paris <jim@xxxxxxxx>
>    Cc: Philip Kelleher <pjk1939@xxxxxxxxxxxxxxxxxx>
>    Cc: Minchan Kim <minchan@xxxxxxxxxx>
>    Cc: Nitin Gupta <ngupta@xxxxxxxxxx>
>    Cc: Oleg Drokin <oleg.drokin@xxxxxxxxx>
>    Cc: Andreas Dilger <andreas.dilger@xxxxxxxxx>
>    Acked-by: NeilBrown <neilb@xxxxxxx> (for the 'md/md.c' bits)
>    Acked-by: Mike Snitzer <snitzer@xxxxxxxxxx>
>    Reviewed-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
>    Signed-off-by: Kent Overstreet <kent.overstreet@xxxxxxxxx>
>    [dpark: skip more mq-based drivers, resolve merge conflicts, etc.]
>    Signed-off-by: Dongsu Park <dpark@xxxxxxxxxx>
>    Signed-off-by: Ming Lin <ming.l@xxxxxxxxxxxxxxx>
>    Signed-off-by: Jens Axboe <axboe@xxxxxx>
>
> This thread on the linux-parisc has most of the discussion and analysis:
> http://www.spinics.net/lists/linux-parisc/msg06710.html
>
> Essentially, the SCSI layer underestimates the number of sg segments needed and we run off the end of the sg list and crash.
> This happens because the protect bit is ignored.  As a result 4.3 and later kernels fail to boot.  This includes the current Debian
> kernel for hppa.
>
> Hopefully, the block group can help resolve this issue.  We can help with testing if needed.
>

We fixed several similar bugs, but maybe there is another one, :-(

Could you apply the attached debug patch and post the log after the issue is
triggered?

Thanks,
Ming Lei
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 2613531..1a73eb6 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -416,6 +416,24 @@ single_segment:
 	return nsegs;
 }
 
+static void check_bvec(struct request *req)
+{
+	struct bio_vec bvec;
+	struct req_iterator iter;
+	int i = 0;
+
+	printk("%s: dump bvec for %p(f:%llx, t:%d)\n",
+			__func__, req, req->cmd_flags,
+			req->cmd_type);
+	rq_for_each_segment(bvec, req, iter) {
+		printk("\t %4d: %u %u %lu %p\n",
+			     i, bvec.bv_offset, bvec.bv_len,
+			     (unsigned long)page_to_pfn(bvec.bv_page),
+			     iter.bio);
+		i++;
+	}
+}
+
 /*
  * map a request to scatterlist, return number of sg entries setup. Caller
  * must make sure sg can hold rq->nr_phys_segments entries
@@ -425,6 +443,7 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 {
 	struct scatterlist *sg = NULL;
 	int nsegs = 0;
+	bool extra_len = false, dma_drain = false;
 
 	if (rq->bio)
 		nsegs = __blk_bios_map_sg(q, rq->bio, sglist, &sg);
@@ -436,6 +455,7 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 
 		sg->length += pad_len;
 		rq->extra_len += pad_len;
+		extra_len = true;
 	}
 
 	if (q->dma_drain_size && q->dma_drain_needed(rq)) {
@@ -450,6 +470,7 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 			    (PAGE_SIZE - 1));
 		nsegs++;
 		rq->extra_len += q->dma_drain_size;
+		dma_drain = true;
 	}
 
 	if (sg)
@@ -460,6 +481,12 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
 	 * segment is bigger than number of req's physical segments
 	 */
 	WARN_ON(nsegs > rq->nr_phys_segments);
+	if (nsegs > rq->nr_phys_segments) {
+		printk("%s: merge bug: %d %d, extra_len %d, dma_drain %d\n",
+			__func__, nsegs, rq->nr_phys_segments,
+			extra_len, dma_drain);
+		check_bvec(rq);
+	}
 
 	return nsegs;
 }

[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux