From 99f3c90d0d85708e7401a81ce3314e50bf7f2819 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Fri, 29 Jul 2016 13:19:55 -0400 Subject: dm flakey: error READ bios during the down_interval When the corrupt_bio_byte feature was introduced it caused READ bios to no longer be errored with -EIO during the down_interval. This had to do with the complexity of needing to submit READs if the corrupt_bio_byte feature was used. Fix it so READ bios are properly errored with -EIO; doing so early in flakey_map() as long as there isn't a match for the corrupt_bio_byte feature. Fixes: a3998799fb4df ("dm flakey: add corrupt_bio_byte feature") Reported-by: Akira Hayakawa Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org --- drivers/md/dm-flakey.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c index 29b99fb6a16a..19db13e99466 100644 --- a/drivers/md/dm-flakey.c +++ b/drivers/md/dm-flakey.c @@ -289,10 +289,16 @@ static int flakey_map(struct dm_target *ti, struct bio *bio) pb->bio_submitted = true; /* - * Map reads as normal. + * Map reads as normal only if corrupt_bio_byte set. */ - if (bio_data_dir(bio) == READ) - goto map_bio; + if (bio_data_dir(bio) == READ) { + /* If flags were specified, only corrupt those that match. */ + if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw == READ) && + all_corrupt_bio_flags_match(bio, fc)) + goto map_bio; + else + return -EIO; + } /* * Drop writes? @@ -330,12 +336,13 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio, int error) /* * Corrupt successful READs while in down state. - * If flags were specified, only corrupt those that match. */ - if (fc->corrupt_bio_byte && !error && pb->bio_submitted && - (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw == READ) && - all_corrupt_bio_flags_match(bio, fc)) - corrupt_bio_data(bio, fc); + if (!error && pb->bio_submitted && (bio_data_dir(bio) == READ)) { + if (fc->corrupt_bio_byte) + corrupt_bio_data(bio, fc); + else + return -EIO; + } return error; } -- cgit v1.2.3-58-ga151 From 1814f2e3fb95b58490e56a38fefe462ffe8fb9ad Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Mon, 25 Jul 2016 21:08:51 -0400 Subject: dm mpath: add locking to multipath_resume and must_push_back Multiple flags were being tested without locking. Protect against non-atomic bit changes in m->flags by holding m->lock (while testing or setting the queue_if_no_path related flags). Signed-off-by: Mike Snitzer --- drivers/md/dm-mpath.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 7eac080fcb18..d7107d23b897 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -507,13 +507,27 @@ static bool __must_push_back(struct multipath *m) static bool must_push_back_rq(struct multipath *m) { - return (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags) || - __must_push_back(m)); + bool r; + unsigned long flags; + + spin_lock_irqsave(&m->lock, flags); + r = (test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags) || + __must_push_back(m)); + spin_unlock_irqrestore(&m->lock, flags); + + return r; } static bool must_push_back_bio(struct multipath *m) { - return __must_push_back(m); + bool r; + unsigned long flags; + + spin_lock_irqsave(&m->lock, flags); + r = __must_push_back(m); + spin_unlock_irqrestore(&m->lock, flags); + + return r; } /* @@ -1680,12 +1694,14 @@ static void multipath_postsuspend(struct dm_target *ti) static void multipath_resume(struct dm_target *ti) { struct multipath *m = ti->private; + unsigned long flags; + spin_lock_irqsave(&m->lock, flags); if (test_bit(MPATHF_SAVED_QUEUE_IF_NO_PATH, &m->flags)) set_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags); else clear_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags); - smp_mb__after_atomic(); + spin_unlock_irqrestore(&m->lock, flags); } /* -- cgit v1.2.3-58-ga151 From 7d9595d848cdff5c7939f68eec39e0c5d36a1d67 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Tue, 2 Aug 2016 12:51:11 -0400 Subject: dm rq: fix the starting and stopping of blk-mq queues Improve dm_stop_queue() to cancel any requeue_work. Also, have dm_start_queue() and dm_stop_queue() clear/set the QUEUE_FLAG_STOPPED for the blk-mq request_queue. On suspend dm_stop_queue() handles stopping the blk-mq request_queue BUT: even though the hw_queues are marked BLK_MQ_S_STOPPED at that point there is still a race that is allowing block/blk-mq.c to call ->queue_rq against a hctx that it really shouldn't. Add a check to dm_mq_queue_rq() that guards against this rarity (albeit _not_ race-free). Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org # must patch dm.c on < 4.8 kernels --- drivers/md/dm-rq.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c index 7a9661868496..1ca7463e8bb2 100644 --- a/drivers/md/dm-rq.c +++ b/drivers/md/dm-rq.c @@ -78,6 +78,7 @@ void dm_start_queue(struct request_queue *q) if (!q->mq_ops) dm_old_start_queue(q); else { + queue_flag_clear_unlocked(QUEUE_FLAG_STOPPED, q); blk_mq_start_stopped_hw_queues(q, true); blk_mq_kick_requeue_list(q); } @@ -101,8 +102,14 @@ void dm_stop_queue(struct request_queue *q) { if (!q->mq_ops) dm_old_stop_queue(q); - else + else { + spin_lock_irq(q->queue_lock); + queue_flag_set(QUEUE_FLAG_STOPPED, q); + spin_unlock_irq(q->queue_lock); + + blk_mq_cancel_requeue_work(q); blk_mq_stop_hw_queues(q); + } } static struct dm_rq_target_io *alloc_old_rq_tio(struct mapped_device *md, @@ -864,6 +871,17 @@ static int dm_mq_queue_rq(struct blk_mq_hw_ctx *hctx, dm_put_live_table(md, srcu_idx); } + /* + * On suspend dm_stop_queue() handles stopping the blk-mq + * request_queue BUT: even though the hw_queues are marked + * BLK_MQ_S_STOPPED at that point there is still a race that + * is allowing block/blk-mq.c to call ->queue_rq against a + * hctx that it really shouldn't. The following check guards + * against this rarity (albeit _not_ race-free). + */ + if (unlikely(test_bit(BLK_MQ_S_STOPPED, &hctx->state))) + return BLK_MQ_RQ_QUEUE_BUSY; + if (ti->type->busy && ti->type->busy(ti)) return BLK_MQ_RQ_QUEUE_BUSY; -- cgit v1.2.3-58-ga151 From eaf9a7361f47727b166688a9f2096854eef60fbe Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Tue, 2 Aug 2016 13:07:20 -0400 Subject: dm: set DMF_SUSPENDED* _before_ clearing DMF_NOFLUSH_SUSPENDING Otherwise, there is potential for both DMF_SUSPENDED* and DMF_NOFLUSH_SUSPENDING to not be set during dm_suspend() -- which is definitely _not_ a valid state. This fix, in conjuction with "dm rq: fix the starting and stopping of blk-mq queues", addresses the potential for request-based DM multipath's __multipath_map() to see !dm_noflush_suspending() during suspend. Reported-by: Bart Van Assche Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org --- drivers/md/dm.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 25d1d97154a8..dfa09e14e847 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -2082,7 +2082,8 @@ static void unlock_fs(struct mapped_device *md) * Caller must hold md->suspend_lock */ static int __dm_suspend(struct mapped_device *md, struct dm_table *map, - unsigned suspend_flags, int interruptible) + unsigned suspend_flags, int interruptible, + int dmf_suspended_flag) { bool do_lockfs = suspend_flags & DM_SUSPEND_LOCKFS_FLAG; bool noflush = suspend_flags & DM_SUSPEND_NOFLUSH_FLAG; @@ -2149,6 +2150,8 @@ static int __dm_suspend(struct mapped_device *md, struct dm_table *map, * to finish. */ r = dm_wait_for_completion(md, interruptible); + if (!r) + set_bit(dmf_suspended_flag, &md->flags); if (noflush) clear_bit(DMF_NOFLUSH_SUSPENDING, &md->flags); @@ -2210,12 +2213,10 @@ retry: map = rcu_dereference_protected(md->map, lockdep_is_held(&md->suspend_lock)); - r = __dm_suspend(md, map, suspend_flags, TASK_INTERRUPTIBLE); + r = __dm_suspend(md, map, suspend_flags, TASK_INTERRUPTIBLE, DMF_SUSPENDED); if (r) goto out_unlock; - set_bit(DMF_SUSPENDED, &md->flags); - dm_table_postsuspend_targets(map); out_unlock: @@ -2309,9 +2310,8 @@ static void __dm_internal_suspend(struct mapped_device *md, unsigned suspend_fla * would require changing .presuspend to return an error -- avoid this * until there is a need for more elaborate variants of internal suspend. */ - (void) __dm_suspend(md, map, suspend_flags, TASK_UNINTERRUPTIBLE); - - set_bit(DMF_SUSPENDED_INTERNALLY, &md->flags); + (void) __dm_suspend(md, map, suspend_flags, TASK_UNINTERRUPTIBLE, + DMF_SUSPENDED_INTERNALLY); dm_table_postsuspend_targets(map); } -- cgit v1.2.3-58-ga151 From f15f64d65bc0a4cc0973b5a30854bb5091d34153 Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen Date: Wed, 27 Jul 2016 23:34:01 +0200 Subject: dm raid: fix processing of max_recovery_rate constructor flag __CTR_FLAG_MIN_RECOVERY_RATE was used instead of __CTR_FLAG_MAX_RECOVERY_RATE thus causing max_recovery_rate to be rejected in case min_recovery_rate was already set. Signed-off-by: Heinz Mauelshagen Signed-off-by: Mike Snitzer --- drivers/md/dm-raid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 84983549b5e1..5ef6b5af3fb4 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -1270,7 +1270,7 @@ static int parse_raid_params(struct raid_set *rs, struct dm_arg_set *as, } rs->md.sync_speed_min = (int)value; } else if (!strcasecmp(key, dm_raid_arg_name_by_flag(CTR_FLAG_MAX_RECOVERY_RATE))) { - if (test_and_set_bit(__CTR_FLAG_MIN_RECOVERY_RATE, &rs->ctr_flags)) { + if (test_and_set_bit(__CTR_FLAG_MAX_RECOVERY_RATE, &rs->ctr_flags)) { rs->ti->error = "Only one max_recovery_rate argument pair allowed"; return -EINVAL; } -- cgit v1.2.3-58-ga151 From b2a4872a45280217149324e3bbef228cd5a0a270 Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen Date: Wed, 3 Aug 2016 17:47:04 +0200 Subject: dm raid: constructor fails on non-zero incompat_features When lvm2 userspace requests a RaidLV repair, it sets the rebuild constructor flag on the new replacement DataLVs but does not clear the respective MetaLVs. Hence the superblock that is loaded from such new MetaLVs may have a non-zero incompat_features member and the constructor will fail with false-positive on incompat_features. Solve by initializing the incompat_features member properly. Signed-off-by: Heinz Mauelshagen Signed-off-by: Mike Snitzer --- drivers/md/dm-raid.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 5ef6b5af3fb4..7b403ab41bd3 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -1960,6 +1960,7 @@ static void super_sync(struct mddev *mddev, struct md_rdev *rdev) sb->data_offset = cpu_to_le64(rdev->data_offset); sb->new_data_offset = cpu_to_le64(rdev->new_data_offset); sb->sectors = cpu_to_le64(rdev->sectors); + sb->incompat_features = cpu_to_le32(0); /* Zero out the rest of the payload after the size of the superblock */ memset(sb + 1, 0, rdev->sb_size - sizeof(*sb)); -- cgit v1.2.3-58-ga151 From 2a034ec197aa574dd159961cc661c9b08cd76425 Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen Date: Wed, 3 Aug 2016 22:57:49 +0200 Subject: dm raid: fix use of wrong status char during resynchronization During a resynchronization, device status char 'a' is output on the raid status line for every device of a RAID set. It changes from 'a' to 'A' (unless device failure) when the resynchronization completes. Interrupting and restarting a resynchronization, by reloading the DM table, erroneously lead to status char 'A'. Fix this by avoiding setting the MD_RECOVERY_REQUESTED flag in raid_preresume(). Signed-off-by: Heinz Mauelshagen Signed-off-by: Mike Snitzer --- drivers/md/dm-raid.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 7b403ab41bd3..1b9795d75ef8 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -3578,7 +3578,6 @@ static int raid_preresume(struct dm_target *ti) /* Be prepared for mddev_resume() in raid_resume() */ set_bit(MD_RECOVERY_FROZEN, &mddev->recovery); if (mddev->recovery_cp && mddev->recovery_cp < MaxSector) { - set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery); set_bit(MD_RECOVERY_SYNC, &mddev->recovery); mddev->resync_min = mddev->recovery_cp; } -- cgit v1.2.3-58-ga151