| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1438380135.7974.4.camel@ssi>
Date: Fri, 31 Jul 2015 15:02:15 -0700
From: Ming Lin <mlin@...nel.org>
To: Mike Snitzer <snitzer@...hat.com>
Cc: lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 01/11] block: make generic_make_request handle
arbitrarily sized bios
On Fri, 2015-07-31 at 17:38 -0400, Mike Snitzer wrote:
> On Fri, Jul 31 2015 at 5:19pm -0400,
> Ming Lin <mlin@...nel.org> wrote:
>
> > On Fri, Jul 31, 2015 at 12:23 PM, Mike Snitzer <snitzer@...hat.com> wrote:
> > > On Mon, Jul 06 2015 at 3:44P -0400,
> > > Ming Lin <mlin@...nel.org> wrote:
> > >
> > >> From: Kent Overstreet <kent.overstreet@...il.com>
> > >>
> > >> 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@...nel.dk>
> > >> Cc: Christoph Hellwig <hch@...radead.org>
> > >> Cc: Al Viro <viro@...iv.linux.org.uk>
> > >> Cc: Ming Lei <ming.lei@...onical.com>
> > >> Cc: Neil Brown <neilb@...e.de>
> > >> Cc: Alasdair Kergon <agk@...hat.com>
> > >> Cc: Mike Snitzer <snitzer@...hat.com>
> > >> Cc: dm-devel@...hat.com
> > >> Cc: Lars Ellenberg <drbd-dev@...ts.linbit.com>
> > >> Cc: drbd-user@...ts.linbit.com
> > >> Cc: Jiri Kosina <jkosina@...e.cz>
> > >> Cc: Geoff Levand <geoff@...radead.org>
> > >> Cc: Jim Paris <jim@...n.com>
> > >> Cc: Joshua Morris <josh.h.morris@...ibm.com>
> > >> Cc: Philip Kelleher <pjk1939@...ux.vnet.ibm.com>
> > >> Cc: Minchan Kim <minchan@...nel.org>
> > >> Cc: Nitin Gupta <ngupta@...are.org>
> > >> Cc: Oleg Drokin <oleg.drokin@...el.com>
> > >> Cc: Andreas Dilger <andreas.dilger@...el.com>
> > >> Acked-by: NeilBrown <neilb@...e.de> (for the 'md/md.c' bits)
> > >> Signed-off-by: Kent Overstreet <kent.overstreet@...il.com>
> > >> [dpark: skip more mq-based drivers, resolve merge conflicts, etc.]
> > >> Signed-off-by: Dongsu Park <dpark@...teo.net>
> > >> Signed-off-by: Ming Lin <ming.l@....samsung.com>
> > > ...
> > >> 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
#
# profile :spindle do
# metadata_dev '/dev/vdd'
# data_dev '/dev/vde'
# end
#
# profile :mix do
# metadata_dev '/dev/vdb'
# data_dev '/dev/vde'
# end
#
# default_profile :ssd
# default_test_scale :normal
root@bee:~/device-mapper-test-suite# dmtest run --suite thin-provisioning -n discard_a_fragmented_device
Loaded suite thin-provisioning
Started
DiscardQuickTests
discard_a_fragmented_device...restored metadata
FAIL
Finished in 1.958143844 seconds.
1) Error:
test_discard_a_fragmented_device(DiscardQuickTests):
ProcessControl::ExitError: command failed: dmsetup load test-dev-307057 /tmp/dm-table20150731-3386-1rncpsb
/root/device-mapper-test-suite/lib/dmtest/process.rb:96:in `wait'
/root/device-mapper-test-suite/lib/dmtest/process.rb:111:in `really_run'
/root/device-mapper-test-suite/lib/dmtest/process.rb:116:in `run_'
/root/device-mapper-test-suite/lib/dmtest/process.rb:138:in `run'
/root/device-mapper-test-suite/lib/dmtest/device-mapper/interface.rb:16:in `block in load'
/root/device-mapper-test-suite/lib/dmtest/utils.rb:94:in `with_temp_file'
/root/device-mapper-test-suite/lib/dmtest/device-mapper/interface.rb:12:in `load'
/root/device-mapper-test-suite/lib/dmtest/device-mapper/dev.rb:19:in `load'
/root/device-mapper-test-suite/lib/dmtest/device-mapper/lexical_operators.rb:53:in `block in create'
/root/device-mapper-test-suite/lib/dmtest/prelude.rb:39:in `protect_'
/root/device-mapper-test-suite/lib/dmtest/device-mapper/lexical_operators.rb:47:in `create'
/root/device-mapper-test-suite/lib/dmtest/device-mapper/lexical_operators.rb:10:in `with_dev'
/root/device-mapper-test-suite/lib/dmtest/thinp-mixin.rb:147:in `with_fake_discard'
/root/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:109:in `with_discardable_pool'
/root/device-mapper-test-suite/lib/dmtest/tests/thin-provisioning/discard_tests.rb:253:in `test_discard_a_fragmented_device'
1 tests, 0 assertions, 0 failures, 1 errors
Tests failed
root@bee:~/device-mapper-test-suite# dmesg
[ 83.824523] device-mapper: table: 249:0: fake-discard: unknown target type
[ 83.827539] device-mapper: ioctl: error adding target to table
Seems my setup has problem. Could you help to take a look?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists