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;
}
