From 9dc112e2daf87b40607fd8d357e2d7de32290d45 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sat, 30 Sep 2017 19:46:48 +0800 Subject: [PATCH 1/5] dm mpath: remove annoying message of 'blk_get_request() returned -11' It is very normal to see allocation failure, especially with blk-mq request_queues, so it's unnecessary to report this error and annoy people. In practice this 'blk_get_request() returned -11' error gets logged quite frequently when a blk-mq DM multipath device sees heavy IO. This change is marked for stable@ because the annoying message in question was included in stable@ commit 7083abbbf. Fixes: 7083abbbf ("dm mpath: avoid that path removal can trigger an infinite loop") Cc: stable@vger.kernel.org Signed-off-by: Ming Lei Signed-off-by: Mike Snitzer --- drivers/md/dm-mpath.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 11f273d2f018..e8094d8fbe0d 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -499,8 +499,6 @@ static int multipath_clone_and_map(struct dm_target *ti, struct request *rq, if (IS_ERR(clone)) { /* EBUSY, ENODEV or EWOULDBLOCK: requeue */ bool queue_dying = blk_queue_dying(q); - DMERR_LIMIT("blk_get_request() returned %ld%s - requeuing", - PTR_ERR(clone), queue_dying ? " (path offline)" : ""); if (queue_dying) { atomic_inc(&m->pg_init_in_progress); activate_or_offline_path(pgpath); From 8a74d29d541cd86569139c6f3f44b2d210458071 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Tue, 14 Nov 2017 15:40:52 -0500 Subject: [PATCH 2/5] dm: discard support requires all targets in a table support discards A DM device with a mix of discard capabilities (due to some underlying devices not having discard support) _should_ just return -EOPNOTSUPP for the region of the device that doesn't support discards (even if only by way of the underlying driver formally not supporting discards). BUT, that does ask the underlying driver to handle something that it never advertised support for. In doing so we're exposing users to the potential for a underlying disk driver hanging if/when a discard is issued a the device that is incapable and never claimed to support discards. Fix this by requiring that each DM target in a DM table provide discard support as a prereq for a DM device to advertise support for discards. This may cause some configurations that were happily supporting discards (even in the face of a mix of discard support) to stop supporting discards -- but the risk of users hitting driver hangs, and forced reboots, outweighs supporting those fringe mixed discard configurations. Cc: stable@vger.kernel.org Signed-off-by: Mike Snitzer --- drivers/md/dm-table.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index fc7d240cbf05..6f1ec12c287c 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -1758,13 +1758,12 @@ static bool dm_table_supports_write_zeroes(struct dm_table *t) return true; } - -static int device_discard_capable(struct dm_target *ti, struct dm_dev *dev, - sector_t start, sector_t len, void *data) +static int device_not_discard_capable(struct dm_target *ti, struct dm_dev *dev, + sector_t start, sector_t len, void *data) { struct request_queue *q = bdev_get_queue(dev->bdev); - return q && blk_queue_discard(q); + return q && !blk_queue_discard(q); } static bool dm_table_supports_discards(struct dm_table *t) @@ -1772,28 +1771,24 @@ static bool dm_table_supports_discards(struct dm_table *t) struct dm_target *ti; unsigned i; - /* - * Unless any target used by the table set discards_supported, - * require at least one underlying device to support discards. - * t->devices includes internal dm devices such as mirror logs - * so we need to use iterate_devices here, which targets - * supporting discard selectively must provide. - */ for (i = 0; i < dm_table_get_num_targets(t); i++) { ti = dm_table_get_target(t, i); if (!ti->num_discard_bios) - continue; + return false; - if (ti->discards_supported) - return true; - - if (ti->type->iterate_devices && - ti->type->iterate_devices(ti, device_discard_capable, NULL)) - return true; + /* + * Either the target provides discard support (as implied by setting + * 'discards_supported') or it relies on _all_ data devices having + * discard support. + */ + if (!ti->discards_supported && + (!ti->type->iterate_devices || + ti->type->iterate_devices(ti, device_not_discard_capable, NULL))) + return false; } - return false; + return true; } void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, From 7dea378b23fdc5003379615fc579dbf987ed8128 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Thu, 16 Nov 2017 10:22:42 -0500 Subject: [PATCH 3/5] dm: do not set 'discards_supported' in targets that do not need it The DM target's 'discards_supported' flag is intended to act as an override. Meaning, even if the underlying storage doesn't support discards the DM target will. Signed-off-by: Mike Snitzer --- drivers/md/dm-era-target.c | 1 - drivers/md/dm-raid.c | 6 ------ 2 files changed, 7 deletions(-) diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c index ba84b8d62cd0..73a5c198113a 100644 --- a/drivers/md/dm-era-target.c +++ b/drivers/md/dm-era-target.c @@ -1513,7 +1513,6 @@ static int era_ctr(struct dm_target *ti, unsigned argc, char **argv) ti->flush_supported = true; ti->num_discard_bios = 1; - ti->discards_supported = true; era->callbacks.congested_fn = era_is_congested; dm_table_add_target_callbacks(ti->table, &era->callbacks); diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index a25eebd98996..ea6eb96b5012 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -2887,9 +2887,6 @@ static void configure_discard_support(struct raid_set *rs) bool raid456; struct dm_target *ti = rs->ti; - /* Assume discards not supported until after checks below. */ - ti->discards_supported = false; - /* * XXX: RAID level 4,5,6 require zeroing for safety. */ @@ -2914,9 +2911,6 @@ static void configure_discard_support(struct raid_set *rs) } } - /* All RAID members properly support discards */ - ti->discards_supported = true; - /* * RAID1 and RAID10 personalities require bio splitting, * RAID0/4/5/6 don't and process large discard bios properly. From 5d47c89f29eab6e9e224d7ac8c0baf4da72c9493 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Thu, 16 Nov 2017 15:42:26 -0500 Subject: [PATCH 4/5] dm: clear all discard attributes in queue_limits when discards are disabled Otherwise, it can happen that the QUEUE_FLAG_DISCARD isn't set but the various discard attributes (which get exposed via sysfs) may be set. Signed-off-by: Mike Snitzer --- drivers/md/dm-table.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index 6f1ec12c287c..3acf8f56eb64 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -1801,9 +1801,15 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q, */ q->limits = *limits; - if (!dm_table_supports_discards(t)) + if (!dm_table_supports_discards(t)) { queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, q); - else + /* Must also clear discard limits... */ + q->limits.max_discard_sectors = 0; + q->limits.max_hw_discard_sectors = 0; + q->limits.discard_granularity = 0; + q->limits.discard_alignment = 0; + q->limits.discard_misaligned = 0; + } else queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q); if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) { From 74d4108d9e681dbbe4a2940ed8fdff1f6868184c Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 15 Nov 2017 16:38:09 -0800 Subject: [PATCH 5/5] dm bufio: fix integer overflow when limiting maximum cache size The default max_cache_size_bytes for dm-bufio is meant to be the lesser of 25% of the size of the vmalloc area and 2% of the size of lowmem. However, on 32-bit systems the intermediate result in the expression (VMALLOC_END - VMALLOC_START) * DM_BUFIO_VMALLOC_PERCENT / 100 overflows, causing the wrong result to be computed. For example, on a 32-bit system where the vmalloc area is 520093696 bytes, the result is 1174405 rather than the expected 130023424, which makes the maximum cache size much too small (far less than 2% of lowmem). This causes severe performance problems for dm-verity users on affected systems. Fix this by using mult_frac() to correctly multiply by a percentage. Do this for all places in dm-bufio that multiply by a percentage. Also replace (VMALLOC_END - VMALLOC_START) with VMALLOC_TOTAL, which contrary to the comment is now defined in include/linux/vmalloc.h. Depends-on: 9993bc635 ("sched/x86: Fix overflow in cyc2ns_offset") Fixes: 95d402f057f2 ("dm: add bufio") Cc: # v3.2+ Signed-off-by: Eric Biggers Signed-off-by: Mike Snitzer --- drivers/md/dm-bufio.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index d216a8f7bc22..8e3adcb46851 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -974,7 +974,8 @@ static void __get_memory_limit(struct dm_bufio_client *c, buffers = c->minimum_buffers; *limit_buffers = buffers; - *threshold_buffers = buffers * DM_BUFIO_WRITEBACK_PERCENT / 100; + *threshold_buffers = mult_frac(buffers, + DM_BUFIO_WRITEBACK_PERCENT, 100); } /* @@ -1910,19 +1911,15 @@ static int __init dm_bufio_init(void) memset(&dm_bufio_caches, 0, sizeof dm_bufio_caches); memset(&dm_bufio_cache_names, 0, sizeof dm_bufio_cache_names); - mem = (__u64)((totalram_pages - totalhigh_pages) * - DM_BUFIO_MEMORY_PERCENT / 100) << PAGE_SHIFT; + mem = (__u64)mult_frac(totalram_pages - totalhigh_pages, + DM_BUFIO_MEMORY_PERCENT, 100) << PAGE_SHIFT; if (mem > ULONG_MAX) mem = ULONG_MAX; #ifdef CONFIG_MMU - /* - * Get the size of vmalloc space the same way as VMALLOC_TOTAL - * in fs/proc/internal.h - */ - if (mem > (VMALLOC_END - VMALLOC_START) * DM_BUFIO_VMALLOC_PERCENT / 100) - mem = (VMALLOC_END - VMALLOC_START) * DM_BUFIO_VMALLOC_PERCENT / 100; + if (mem > mult_frac(VMALLOC_TOTAL, DM_BUFIO_VMALLOC_PERCENT, 100)) + mem = mult_frac(VMALLOC_TOTAL, DM_BUFIO_VMALLOC_PERCENT, 100); #endif dm_bufio_default_cache_size = mem;