Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752113AbbGaWTD (ORCPT ); Fri, 31 Jul 2015 18:19:03 -0400 Received: from mail.kernel.org ([198.145.29.136]:33835 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750904AbbGaWTB (ORCPT ); Fri, 31 Jul 2015 18:19:01 -0400 MIME-Version: 1.0 In-Reply-To: <1438380135.7974.4.camel@ssi> References: <1436168690-32102-1-git-send-email-mlin@kernel.org> <20150731192337.GA8907@redhat.com> <20150731213831.GA16464@redhat.com> <1438380135.7974.4.camel@ssi> Date: Fri, 31 Jul 2015 15:18:52 -0700 Message-ID: Subject: Re: [PATCH v5 01/11] block: make generic_make_request handle arbitrarily sized bios From: Ming Lin To: Mike Snitzer Cc: lkml Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7398 Lines: 189 On Fri, Jul 31, 2015 at 3:02 PM, Ming Lin wrote: > On Fri, 2015-07-31 at 17:38 -0400, Mike Snitzer wrote: >> On Fri, Jul 31 2015 at 5:19pm -0400, >> Ming Lin wrote: >> >> > On Fri, Jul 31, 2015 at 12:23 PM, Mike Snitzer wrote: >> > > On Mon, Jul 06 2015 at 3:44P -0400, >> > > Ming Lin wrote: >> > > >> > >> From: Kent Overstreet >> > >> >> > >> 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 >> > >> Cc: Christoph Hellwig >> > >> Cc: Al Viro >> > >> Cc: Ming Lei >> > >> Cc: Neil Brown >> > >> Cc: Alasdair Kergon >> > >> Cc: Mike Snitzer >> > >> Cc: dm-devel@redhat.com >> > >> Cc: Lars Ellenberg >> > >> Cc: drbd-user@lists.linbit.com >> > >> Cc: Jiri Kosina >> > >> Cc: Geoff Levand >> > >> Cc: Jim Paris >> > >> Cc: Joshua Morris >> > >> Cc: Philip Kelleher >> > >> Cc: Minchan Kim >> > >> Cc: Nitin Gupta >> > >> Cc: Oleg Drokin >> > >> Cc: Andreas Dilger >> > >> Acked-by: NeilBrown (for the 'md/md.c' bits) >> > >> Signed-off-by: Kent Overstreet >> > >> [dpark: skip more mq-based drivers, resolve merge conflicts, etc.] >> > >> Signed-off-by: Dongsu Park >> > >> Signed-off-by: Ming Lin >> > > ... >> > >> 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: >> > >> > I'm setting up the testsuite to debug. >> >> OK, once setup, to run the 2 tests in question directly you'd do >> something like: >> >> dmtest run --suite thin-provisioning -n discard_a_fragmented_device >> >> dmtest run --suite thin-provisioning -n discard_fully_provisioned_device_benchmark >> >> Again, these tests pass without this patchset. > > (trim CC list) > > I followed > https://github.com/jthornber/device-mapper-test-suite > https://github.com/jthornber/thin-provisioning-tools > > to setup. > > $ grep CONFIG_DM_ .config > CONFIG_DM_MQ_DEFAULT=y > CONFIG_DM_DEBUG=y > CONFIG_DM_BUFIO=y > CONFIG_DM_BIO_PRISON=y > CONFIG_DM_PERSISTENT_DATA=y > # CONFIG_DM_DEBUG_BLOCK_STACK_TRACING is not set > CONFIG_DM_CRYPT=y > CONFIG_DM_SNAPSHOT=y > CONFIG_DM_THIN_PROVISIONING=y > CONFIG_DM_CACHE=y > CONFIG_DM_CACHE_MQ=y > CONFIG_DM_CACHE_SMQ=y > CONFIG_DM_CACHE_CLEANER=y > CONFIG_DM_ERA=y > CONFIG_DM_MIRROR=y > CONFIG_DM_LOG_USERSPACE=y > CONFIG_DM_RAID=y > CONFIG_DM_ZERO=y > CONFIG_DM_MULTIPATH=y > CONFIG_DM_MULTIPATH_QL=y > CONFIG_DM_MULTIPATH_ST=y > CONFIG_DM_DELAY=y > CONFIG_DM_UEVENT=y > CONFIG_DM_FLAKEY=y > CONFIG_DM_VERITY=y > CONFIG_DM_SWITCH=y > CONFIG_DM_LOG_WRITES=y > > > # cat .dmtest/config > profile :ssd do > metadata_dev '/dev/vda' > data_dev '/dev/vdb' > end OK, virtio-blk doesn't support discard. I could reproduce it with virtio-scsi. Now debugging ... -- 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/