On Mon, Jul 06 2015 at 3:44P -0400, Ming Lin <[email protected]> wrote:
> From: Kent Overstreet <[email protected]> > > 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 <[email protected]> > Cc: Christoph Hellwig <[email protected]> > Cc: Al Viro <[email protected]> > Cc: Ming Lei <[email protected]> > Cc: Neil Brown <[email protected]> > Cc: Alasdair Kergon <[email protected]> > Cc: Mike Snitzer <[email protected]> > Cc: [email protected] > Cc: Lars Ellenberg <[email protected]> > Cc: [email protected] > Cc: Jiri Kosina <[email protected]> > Cc: Geoff Levand <[email protected]> > Cc: Jim Paris <[email protected]> > Cc: Joshua Morris <[email protected]> > Cc: Philip Kelleher <[email protected]> > Cc: Minchan Kim <[email protected]> > Cc: Nitin Gupta <[email protected]> > Cc: Oleg Drokin <[email protected]> > Cc: Andreas Dilger <[email protected]> > Acked-by: NeilBrown <[email protected]> (for the 'md/md.c' bits) > Signed-off-by: Kent Overstreet <[email protected]> > [dpark: skip more mq-based drivers, resolve merge conflicts, etc.] > Signed-off-by: Dongsu Park <[email protected]> > Signed-off-by: Ming Lin <[email protected]> ... > diff --git a/block/blk-merge.c b/block/blk-merge.c > index 30a0d9f..3707f30 100644 > --- a/block/blk-merge.c > +++ b/block/blk-merge.c > @@ -9,12 +9,158 @@ > > #include "blk.h" > > +static struct bio *blk_bio_discard_split(struct request_queue *q, > + struct bio *bio, > + struct bio_set *bs) > +{ > + unsigned int max_discard_sectors, granularity; > + int alignment; > + sector_t tmp; > + unsigned split_sectors; > + > + /* Zero-sector (unknown) and one-sector granularities are the same. */ > + granularity = max(q->limits.discard_granularity >> 9, 1U); > + > + max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9); > + max_discard_sectors -= max_discard_sectors % granularity; > + > + if (unlikely(!max_discard_sectors)) { > + /* XXX: warn */ > + return NULL; > + } > + > + if (bio_sectors(bio) <= max_discard_sectors) > + return NULL; > + > + split_sectors = max_discard_sectors; > + > + /* > + * If the next starting sector would be misaligned, stop the discard at > + * the previous aligned sector. > + */ > + alignment = (q->limits.discard_alignment >> 9) % granularity; > + > + tmp = bio->bi_iter.bi_sector + split_sectors - alignment; > + tmp = sector_div(tmp, granularity); > + > + if (split_sectors > tmp) > + split_sectors -= tmp; > + > + return bio_split(bio, split_sectors, GFP_NOIO, bs); > +} This code to stop the discard at the previous aligned sector could be the reason why I have 2 device-mapper-test-suite tests in the 'thin-provisioning' testsuite failing due to this patchset: 4) Failure: test_discard_a_fragmented_device(DiscardQuickTests) [/root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:49:in `block in assert_no_mappings' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:43:in `call' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:43:in `block in with_dev_md' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:40:in `each' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:40:in `with_dev_md' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:48:in `assert_no_mappings' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:263:in `test_discard_a_fragmented_device']: <0> expected but was <131072>. 5) Failure: test_discard_fully_provisioned_device_benchmark(DiscardQuickTests) [/root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:49:in `block in assert_no_mappings' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:43:in `call' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:43:in `block in with_dev_md' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:40:in `each' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:40:in `with_dev_md' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:48:in `assert_no_mappings' /root/snitm/git/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:225:in `test_discard_fully_provisioned_device_benchmark']: <0> expected but was <262144>. But I haven't yet figured out where the blk_bio_discard_split code, or elsewhere, is going wrong. Mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
