From 565e0450129647df5112bff3df3ffd02b0c08e32 Mon Sep 17 00:00:00 2001 From: Aliaksei Karaliou Date: Sat, 23 Dec 2017 21:20:31 +0300 Subject: md/raid5: simplify uninitialization of shrinker Don't use shrinker.nr_deferred to check whether shrinker was initialized or not. Now this check was integrated into unregister_shrinker(), so it is safe to call it against unregistered shrinker. Signed-off-by: Aliaksei Karaliou Signed-off-by: Shaohua Li --- drivers/md/raid5.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 50d01144b805..36e050678f5a 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -6764,9 +6764,7 @@ static void free_conf(struct r5conf *conf) log_exit(conf); - if (conf->shrinker.nr_deferred) - unregister_shrinker(&conf->shrinker); - + unregister_shrinker(&conf->shrinker); free_thread_groups(conf); shrink_stripes(conf); raid5_free_percpu(conf); -- cgit v1.2.3-59-g8ed1b From 56a64c177abd85a01c5881e195c363b7bda448f2 Mon Sep 17 00:00:00 2001 From: Luis de Bethencourt Date: Wed, 17 Jan 2018 13:38:02 +0000 Subject: md/raid1: Fix trailing semicolon The trailing semicolon is an empty statement that does no operation. Removing it since it doesn't do anything. Signed-off-by: Luis de Bethencourt Signed-off-by: Shaohua Li --- drivers/md/raid1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index b2eae332e1a2..f978eddc7a21 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -1108,7 +1108,7 @@ static void alloc_behind_master_bio(struct r1bio *r1_bio, bio_copy_data(behind_bio, bio); skip_copy: - r1_bio->behind_master_bio = behind_bio;; + r1_bio->behind_master_bio = behind_bio; set_bit(R1BIO_BehindIO, &r1_bio->state); return; -- cgit v1.2.3-59-g8ed1b From 3acdb7b514198d81ef7efcb2e86f498776ac10a7 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 13 Jan 2018 09:49:03 +0100 Subject: md-multipath: Use seq_putc() in multipath_status() A single character (closing square bracket) should be put into a sequence. Thus use the corresponding function "seq_putc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Shaohua Li --- drivers/md/md-multipath.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/md-multipath.c b/drivers/md/md-multipath.c index e40065bdbfc8..0a7e99d62c69 100644 --- a/drivers/md/md-multipath.c +++ b/drivers/md/md-multipath.c @@ -157,7 +157,7 @@ static void multipath_status(struct seq_file *seq, struct mddev *mddev) seq_printf (seq, "%s", rdev && test_bit(In_sync, &rdev->flags) ? "U" : "_"); } rcu_read_unlock(); - seq_printf (seq, "]"); + seq_putc(seq, ']'); } static int multipath_congested(struct mddev *mddev, int bits) -- cgit v1.2.3-59-g8ed1b From 4b242e97d74192bbc5decd808c058cbc347af016 Mon Sep 17 00:00:00 2001 From: Guoqing Jiang Date: Fri, 19 Jan 2018 11:37:56 +0800 Subject: raid10: change the size of resync window for clustered raid To align with raid1's resync window, we need to set the resync window of raid10 to 32M as well. Fixes: 8db87912c9a8 ("md-cluster: Use a small window for raid10 resync") Reported-by: Zhilong Liu Signed-off-by: Guoqing Jiang Signed-off-by: Shaohua Li --- drivers/md/raid10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 99c9207899a7..8d7ddc947d9d 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -141,7 +141,7 @@ static void r10bio_pool_free(void *r10_bio, void *data) #define RESYNC_WINDOW (1024*1024) /* maximum number of concurrent requests, memory permitting */ #define RESYNC_DEPTH (32*1024*1024/RESYNC_BLOCK_SIZE) -#define CLUSTER_RESYNC_WINDOW (16 * RESYNC_WINDOW) +#define CLUSTER_RESYNC_WINDOW (32 * RESYNC_WINDOW) #define CLUSTER_RESYNC_WINDOW_SECTORS (CLUSTER_RESYNC_WINDOW >> 9) /* -- cgit v1.2.3-59-g8ed1b From b126194cbb799f9980b92a77e58db6ad794c8082 Mon Sep 17 00:00:00 2001 From: Xiao Ni Date: Wed, 24 Jan 2018 12:17:38 +0800 Subject: MD: Free bioset when md_run fails Signed-off-by: Xiao Ni Acked-by: Guoqing Jiang Signed-off-by: Shaohua Li --- drivers/md/md.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index bc67ab6844f0..bcf4ab9ab3df 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -5497,8 +5497,10 @@ int md_run(struct mddev *mddev) } if (mddev->sync_set == NULL) { mddev->sync_set = bioset_create(BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); - if (!mddev->sync_set) - return -ENOMEM; + if (!mddev->sync_set) { + err = -ENOMEM; + goto abort; + } } spin_lock(&pers_lock); @@ -5511,7 +5513,8 @@ int md_run(struct mddev *mddev) else pr_warn("md: personality for level %s is not loaded!\n", mddev->clevel); - return -EINVAL; + err = -EINVAL; + goto abort; } spin_unlock(&pers_lock); if (mddev->level != pers->level) { @@ -5524,7 +5527,8 @@ int md_run(struct mddev *mddev) pers->start_reshape == NULL) { /* This personality cannot handle reshaping... */ module_put(pers->owner); - return -EINVAL; + err = -EINVAL; + goto abort; } if (pers->sync_request) { @@ -5593,7 +5597,7 @@ int md_run(struct mddev *mddev) mddev->private = NULL; module_put(pers->owner); bitmap_destroy(mddev); - return err; + goto abort; } if (mddev->queue) { bool nonrot = true; @@ -5655,6 +5659,18 @@ int md_run(struct mddev *mddev) sysfs_notify_dirent_safe(mddev->sysfs_action); sysfs_notify(&mddev->kobj, NULL, "degraded"); return 0; + +abort: + if (mddev->bio_set) { + bioset_free(mddev->bio_set); + mddev->bio_set = NULL; + } + if (mddev->sync_set) { + bioset_free(mddev->sync_set); + mddev->sync_set = NULL; + } + + return err; } EXPORT_SYMBOL_GPL(md_run); -- cgit v1.2.3-59-g8ed1b From 4b6c1060eaa6495aa5b0032e8f2d51dd936b1257 Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen Date: Fri, 2 Feb 2018 23:13:19 +0100 Subject: md: fix md_write_start() deadlock w/o metadata devices If no metadata devices are configured on raid1/4/5/6/10 (e.g. via dm-raid), md_write_start() unconditionally waits for superblocks to be written thus deadlocking. Fix introduces mddev->has_superblocks bool, defines it in md_run() and checks for it in md_write_start() to conditionally avoid waiting. Once on it, check for non-existing superblocks in md_super_write(). Link: https://bugzilla.kernel.org/show_bug.cgi?id=198647 Fixes: cc27b0c78c796 ("md: fix deadlock between mddev_suspend() and md_write_start()") Signed-off-by: Heinz Mauelshagen Signed-off-by: Shaohua Li --- drivers/md/md.c | 10 ++++++++++ drivers/md/md.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/md/md.c b/drivers/md/md.c index bcf4ab9ab3df..9b73cf139b80 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -801,6 +801,9 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev, struct bio *bio; int ff = 0; + if (!page) + return; + if (test_bit(Faulty, &rdev->flags)) return; @@ -5452,6 +5455,7 @@ int md_run(struct mddev *mddev) * the only valid external interface is through the md * device. */ + mddev->has_superblocks = false; rdev_for_each(rdev, mddev) { if (test_bit(Faulty, &rdev->flags)) continue; @@ -5465,6 +5469,9 @@ int md_run(struct mddev *mddev) set_disk_ro(mddev->gendisk, 1); } + if (rdev->sb_page) + mddev->has_superblocks = true; + /* perform some consistency tests on the device. * We don't want the data to overlap the metadata, * Internal Bitmap issues have been handled elsewhere. @@ -8065,6 +8072,7 @@ EXPORT_SYMBOL(md_done_sync); bool md_write_start(struct mddev *mddev, struct bio *bi) { int did_change = 0; + if (bio_data_dir(bi) != WRITE) return true; @@ -8097,6 +8105,8 @@ bool md_write_start(struct mddev *mddev, struct bio *bi) rcu_read_unlock(); if (did_change) sysfs_notify_dirent_safe(mddev->sysfs_state); + if (!mddev->has_superblocks) + return true; wait_event(mddev->sb_wait, !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) || mddev->suspended); diff --git a/drivers/md/md.h b/drivers/md/md.h index 58cd20a5e85e..fbc925cce810 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -468,6 +468,8 @@ struct mddev { void (*sync_super)(struct mddev *mddev, struct md_rdev *rdev); struct md_cluster_info *cluster_info; unsigned int good_device_nr; /* good device num within cluster raid */ + + bool has_superblocks:1; }; enum recovery_flags { -- cgit v1.2.3-59-g8ed1b From f2785b527cda46314805123ddcbc871655b7c4c4 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Sat, 3 Feb 2018 09:19:30 +1100 Subject: md: document lifetime of internal rdev pointer. The rdev pointer kept in the local 'config' for each for raid1, raid10, raid4/5/6 has non-obvious lifetime rules. Sometimes RCU is needed, sometimes a lock, something nothing. Add documentation to explain this. Signed-off-by: NeilBrown Signed-off-by: Shaohua Li --- drivers/md/raid1.h | 12 ++++++++++++ drivers/md/raid10.h | 13 +++++++++++++ drivers/md/raid5.h | 12 ++++++++++++ 3 files changed, 37 insertions(+) diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h index c7294e7557e0..eb84bc68e2fd 100644 --- a/drivers/md/raid1.h +++ b/drivers/md/raid1.h @@ -26,6 +26,18 @@ #define BARRIER_BUCKETS_NR_BITS (PAGE_SHIFT - ilog2(sizeof(atomic_t))) #define BARRIER_BUCKETS_NR (1<reconfig_mutex + * 2/ when resync/recovery is known to be happening - i.e. in code that is + * called as part of performing resync/recovery. + * 3/ while holding rcu_read_lock(), use rcu_dereference to get the pointer + * and if it is non-NULL, increment rdev->nr_pending before dropping the + * RCU lock. + * When .rdev is set to NULL, the nr_pending count checked again and if it has + * been incremented, the pointer is put back in .rdev. + */ + struct raid1_info { struct md_rdev *rdev; sector_t head_position; diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h index db2ac22ac1b4..e2e8840de9bf 100644 --- a/drivers/md/raid10.h +++ b/drivers/md/raid10.h @@ -2,6 +2,19 @@ #ifndef _RAID10_H #define _RAID10_H +/* Note: raid10_info.rdev can be set to NULL asynchronously by + * raid10_remove_disk. + * There are three safe ways to access raid10_info.rdev. + * 1/ when holding mddev->reconfig_mutex + * 2/ when resync/recovery/reshape is known to be happening - i.e. in code + * that is called as part of performing resync/recovery/reshape. + * 3/ while holding rcu_read_lock(), use rcu_dereference to get the pointer + * and if it is non-NULL, increment rdev->nr_pending before dropping the + * RCU lock. + * When .rdev is set to NULL, the nr_pending count checked again and if it has + * been incremented, the pointer is put back in .rdev. + */ + struct raid10_info { struct md_rdev *rdev, *replacement; sector_t head_position; diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h index 2e6123825095..3f8da26032ac 100644 --- a/drivers/md/raid5.h +++ b/drivers/md/raid5.h @@ -450,6 +450,18 @@ enum { * HANDLE gets cleared if stripe_handle leaves nothing locked. */ +/* Note: disk_info.rdev can be set to NULL asynchronously by raid5_remove_disk. + * There are three safe ways to access disk_info.rdev. + * 1/ when holding mddev->reconfig_mutex + * 2/ when resync/recovery/reshape is known to be happening - i.e. in code that + * is called as part of performing resync/recovery/reshape. + * 3/ while holding rcu_read_lock(), use rcu_dereference to get the pointer + * and if it is non-NULL, increment rdev->nr_pending before dropping the RCU + * lock. + * When .rdev is set to NULL, the nr_pending count checked again and if + * it has been incremented, the pointer is put back in .rdev. + */ + struct disk_info { struct md_rdev *rdev, *replacement; struct page *extra_page; /* extra page to use in prexor */ -- cgit v1.2.3-59-g8ed1b From 39772f0a7be3b3dc26c74ea13fe7847fd1522c8b Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Sat, 3 Feb 2018 09:19:30 +1100 Subject: md: only allow remove_and_add_spares when no sync_thread running. The locking protocols in md assume that a device will never be removed from an array during resync/recovery/reshape. When that isn't happening, rcu or reconfig_mutex is needed to protect an rdev pointer while taking a refcount. When it is happening, that protection isn't needed. Unfortunately there are cases were remove_and_add_spares() is called when recovery might be happening: is state_store(), slot_store() and hot_remove_disk(). In each case, this is just an optimization, to try to expedite removal from the personality so the device can be removed from the array. If resync etc is happening, we just have to wait for md_check_recover to find a suitable time to call remove_and_add_spares(). This optimization and not essential so it doesn't matter if it fails. So change remove_and_add_spares() to abort early if resync/recovery/reshape is happening, unless it is called from md_check_recovery() as part of a newly started recovery. The parameter "this" is only NULL when called from md_check_recovery() so when it is NULL, there is no need to abort. As this can result in a NULL dereference, the fix is suitable for -stable. cc: yuyufen Cc: Tomasz Majchrzak Fixes: 8430e7e0af9a ("md: disconnect device from personality before trying to remove it.") Cc: stable@ver.kernel.org (v4.8+) Signed-off-by: NeilBrown Signed-off-by: Shaohua Li --- drivers/md/md.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/md/md.c b/drivers/md/md.c index 9b73cf139b80..ba152dddaaa3 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -8595,6 +8595,10 @@ static int remove_and_add_spares(struct mddev *mddev, int removed = 0; bool remove_some = false; + if (this && test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) + /* Mustn't remove devices when resync thread is running */ + return 0; + rdev_for_each(rdev, mddev) { if ((this == NULL || rdev == this) && rdev->raid_disk >= 0 && -- cgit v1.2.3-59-g8ed1b From 01a69cab01c184d3786af09e9339311123d63d22 Mon Sep 17 00:00:00 2001 From: Yufen Yu Date: Tue, 6 Feb 2018 17:39:15 +0800 Subject: md raid10: fix NULL deference in handle_write_completed() In the case of 'recover', an r10bio with R10BIO_WriteError & R10BIO_IsRecover will be progressed by handle_write_completed(). This function traverses all r10bio->devs[copies]. If devs[m].repl_bio != NULL, it thinks conf->mirrors[dev].replacement is also not NULL. However, this is not always true. When there is an rdev of raid10 has replacement, then each r10bio ->devs[m].repl_bio != NULL in conf->r10buf_pool. However, in 'recover', even if corresponded replacement is NULL, it doesn't clear r10bio ->devs[m].repl_bio, resulting in replacement NULL deference. This bug was introduced when replacement support for raid10 was added in Linux 3.3. As NeilBrown suggested: Elsewhere the determination of "is this device part of the resync/recovery" is made by resting bio->bi_end_io. If this is end_sync_write, then we tried to write here. If it is NULL, then we didn't try to write. Fixes: 9ad1aefc8ae8 ("md/raid10: Handle replacement devices during resync.") Cc: stable (V3.3+) Suggested-by: NeilBrown Signed-off-by: Yufen Yu Signed-off-by: Shaohua Li --- drivers/md/raid10.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 8d7ddc947d9d..9e9441fde8b3 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -2655,7 +2655,8 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio) for (m = 0; m < conf->copies; m++) { int dev = r10_bio->devs[m].devnum; rdev = conf->mirrors[dev].rdev; - if (r10_bio->devs[m].bio == NULL) + if (r10_bio->devs[m].bio == NULL || + r10_bio->devs[m].bio->bi_end_io == NULL) continue; if (!r10_bio->devs[m].bio->bi_status) { rdev_clear_badblocks( @@ -2670,7 +2671,8 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio) md_error(conf->mddev, rdev); } rdev = conf->mirrors[dev].replacement; - if (r10_bio->devs[m].repl_bio == NULL) + if (r10_bio->devs[m].repl_bio == NULL || + r10_bio->devs[m].repl_bio->bi_end_io == NULL) continue; if (!r10_bio->devs[m].repl_bio->bi_status) { -- cgit v1.2.3-59-g8ed1b From f4bc0c813e03bdb93f5300c3e06d7a0f07f65a74 Mon Sep 17 00:00:00 2001 From: Artur Paszkiewicz Date: Tue, 20 Feb 2018 10:10:36 +0100 Subject: raid5-ppl: fix handling flush requests Add missing bio completion. Without this any flush request would hang. Fixes: 1532d9e87e8b ("raid5-ppl: PPL support for disks with write-back cache enabled") Signed-off-by: Artur Paszkiewicz Signed-off-by: Shaohua Li --- drivers/md/raid5-log.h | 3 ++- drivers/md/raid5-ppl.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/md/raid5-log.h b/drivers/md/raid5-log.h index 0c76bcedfc1c..a001808a2b77 100644 --- a/drivers/md/raid5-log.h +++ b/drivers/md/raid5-log.h @@ -44,6 +44,7 @@ extern void ppl_write_stripe_run(struct r5conf *conf); extern void ppl_stripe_write_finished(struct stripe_head *sh); extern int ppl_modify_log(struct r5conf *conf, struct md_rdev *rdev, bool add); extern void ppl_quiesce(struct r5conf *conf, int quiesce); +extern int ppl_handle_flush_request(struct r5l_log *log, struct bio *bio); static inline bool raid5_has_ppl(struct r5conf *conf) { @@ -104,7 +105,7 @@ static inline int log_handle_flush_request(struct r5conf *conf, struct bio *bio) if (conf->log) ret = r5l_handle_flush_request(conf->log, bio); else if (raid5_has_ppl(conf)) - ret = 0; + ret = ppl_handle_flush_request(conf->log, bio); return ret; } diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c index 2764c2290062..42890a08375b 100644 --- a/drivers/md/raid5-ppl.c +++ b/drivers/md/raid5-ppl.c @@ -693,6 +693,16 @@ void ppl_quiesce(struct r5conf *conf, int quiesce) } } +int ppl_handle_flush_request(struct r5l_log *log, struct bio *bio) +{ + if (bio->bi_iter.bi_size == 0) { + bio_endio(bio); + return 0; + } + bio->bi_opf &= ~REQ_PREFLUSH; + return -EAGAIN; +} + void ppl_stripe_write_finished(struct stripe_head *sh) { struct ppl_io_unit *io; -- cgit v1.2.3-59-g8ed1b From 53b8d89ddbdbb0e4625a46d2cdbb6f106c52f801 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 20 Feb 2018 14:09:11 +0100 Subject: md: raid5: avoid string overflow warning gcc warns about a possible overflow of the kmem_cache string, when adding four characters to a string of the same length: drivers/md/raid5.c: In function 'setup_conf': drivers/md/raid5.c:2207:34: error: '-alt' directive writing 4 bytes into a region of size between 1 and 32 [-Werror=format-overflow=] sprintf(conf->cache_name[1], "%s-alt", conf->cache_name[0]); ^~~~ drivers/md/raid5.c:2207:2: note: 'sprintf' output between 5 and 36 bytes into a destination of size 32 sprintf(conf->cache_name[1], "%s-alt", conf->cache_name[0]); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ If I'm counting correctly, we need 11 characters for the fixed part of the string and 18 characters for a 64-bit pointer (when no gendisk is used), so that leaves three characters for conf->level, which should always be sufficient. This makes the code use snprintf() with the correct length, to make the code more robust against changes, and to get the compiler to shut up. In commit f4be6b43f1ac ("md/raid5: ensure we create a unique name for kmem_cache when mddev has no gendisk") from 2010, Neil said that the pointer could be removed "shortly" once devices without gendisk are disallowed. I have no idea if that happened, but if it did, that should probably be changed as well. Signed-off-by: Arnd Bergmann Signed-off-by: Shaohua Li --- drivers/md/raid5.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 36e050678f5a..e3b0f799fbfa 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -2196,15 +2196,16 @@ static int grow_one_stripe(struct r5conf *conf, gfp_t gfp) static int grow_stripes(struct r5conf *conf, int num) { struct kmem_cache *sc; + size_t namelen = sizeof(conf->cache_name[0]); int devs = max(conf->raid_disks, conf->previous_raid_disks); if (conf->mddev->gendisk) - sprintf(conf->cache_name[0], + snprintf(conf->cache_name[0], namelen, "raid%d-%s", conf->level, mdname(conf->mddev)); else - sprintf(conf->cache_name[0], + snprintf(conf->cache_name[0], namelen, "raid%d-%p", conf->level, conf->mddev); - sprintf(conf->cache_name[1], "%s-alt", conf->cache_name[0]); + snprintf(conf->cache_name[1], namelen, "%.27s-alt", conf->cache_name[0]); conf->active_name = 0; sc = kmem_cache_create(conf->cache_name[conf->active_name], -- cgit v1.2.3-59-g8ed1b From 43a521238aca0e24d50add1db125a61bda2a3527 Mon Sep 17 00:00:00 2001 From: Lidong Zhong Date: Tue, 23 Jan 2018 23:06:12 +0800 Subject: md-cluster: choose correct label when clustered layout is not supported r10conf is already successfully allocated before checking the layout Signed-off-by: Lidong Zhong Reviewed-by: Guoqing Jiang Signed-off-by: Shaohua Li --- drivers/md/raid10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 9e9441fde8b3..93fa947fef22 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -3784,7 +3784,7 @@ static int raid10_run(struct mddev *mddev) if (fc > 1 || fo > 0) { pr_err("only near layout is supported by clustered" " raid10\n"); - goto out; + goto out_free_conf; } } -- cgit v1.2.3-59-g8ed1b From 8876391e440ba615b10eef729576e111f0315f87 Mon Sep 17 00:00:00 2001 From: BingJing Chang Date: Thu, 22 Feb 2018 13:34:46 +0800 Subject: md: fix a potential deadlock of raid5/raid10 reshape There is a potential deadlock if mount/umount happens when raid5_finish_reshape() tries to grow the size of emulated disk. How the deadlock happens? 1) The raid5 resync thread finished reshape (expanding array). 2) The mount or umount thread holds VFS sb->s_umount lock and tries to write through critical data into raid5 emulated block device. So it waits for raid5 kernel thread handling stripes in order to finish it I/Os. 3) In the routine of raid5 kernel thread, md_check_recovery() will be called first in order to reap the raid5 resync thread. That is, raid5_finish_reshape() will be called. In this function, it will try to update conf and call VFS revalidate_disk() to grow the raid5 emulated block device. It will try to acquire VFS sb->s_umount lock. The raid5 kernel thread cannot continue, so no one can handle mount/ umount I/Os (stripes). Once the write-through I/Os cannot be finished, mount/umount will not release sb->s_umount lock. The deadlock happens. The raid5 kernel thread is an emulated block device. It is responible to handle I/Os (stripes) from upper layers. The emulated block device should not request any I/Os on itself. That is, it should not call VFS layer functions. (If it did, it will try to acquire VFS locks to guarantee the I/Os sequence.) So we have the resync thread to send resync I/O requests and to wait for the results. For solving this potential deadlock, we can put the size growth of the emulated block device as the final step of reshape thread. 2017/12/29: Thanks to Guoqing Jiang , we confirmed that there is the same deadlock issue in raid10. It's reproducible and can be fixed by this patch. For raid10.c, we can remove the similar code to prevent deadlock as well since they has been called before. Reported-by: Alex Wu Reviewed-by: Alex Wu Reviewed-by: Chung-Chiang Cheng Signed-off-by: BingJing Chang Signed-off-by: Shaohua Li --- drivers/md/md.c | 13 +++++++++++++ drivers/md/raid10.c | 8 +------- drivers/md/raid5.c | 8 +------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index ba152dddaaa3..254e44e44668 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -8569,6 +8569,19 @@ void md_do_sync(struct md_thread *thread) set_mask_bits(&mddev->sb_flags, 0, BIT(MD_SB_CHANGE_PENDING) | BIT(MD_SB_CHANGE_DEVS)); + if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) && + !test_bit(MD_RECOVERY_INTR, &mddev->recovery) && + mddev->delta_disks > 0 && + mddev->pers->finish_reshape && + mddev->pers->size && + mddev->queue) { + mddev_lock_nointr(mddev); + md_set_array_sectors(mddev, mddev->pers->size(mddev, 0, 0)); + mddev_unlock(mddev); + set_capacity(mddev->gendisk, mddev->array_sectors); + revalidate_disk(mddev->gendisk); + } + spin_lock(&mddev->lock); if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery)) { /* We completed so min/max setting can be forgotten if used. */ diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 93fa947fef22..c5e6c60fc0d4 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -4832,17 +4832,11 @@ static void raid10_finish_reshape(struct mddev *mddev) return; if (mddev->delta_disks > 0) { - sector_t size = raid10_size(mddev, 0, 0); - md_set_array_sectors(mddev, size); if (mddev->recovery_cp > mddev->resync_max_sectors) { mddev->recovery_cp = mddev->resync_max_sectors; set_bit(MD_RECOVERY_NEEDED, &mddev->recovery); } - mddev->resync_max_sectors = size; - if (mddev->queue) { - set_capacity(mddev->gendisk, mddev->array_sectors); - revalidate_disk(mddev->gendisk); - } + mddev->resync_max_sectors = mddev->array_sectors; } else { int d; rcu_read_lock(); diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index e3b0f799fbfa..b5d2601483e3 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -8000,13 +8000,7 @@ static void raid5_finish_reshape(struct mddev *mddev) if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery)) { - if (mddev->delta_disks > 0) { - md_set_array_sectors(mddev, raid5_size(mddev, 0, 0)); - if (mddev->queue) { - set_capacity(mddev->gendisk, mddev->array_sectors); - revalidate_disk(mddev->gendisk); - } - } else { + if (mddev->delta_disks <= 0) { int d; spin_lock_irq(&conf->device_lock); mddev->degraded = raid5_calc_degraded(conf); -- cgit v1.2.3-59-g8ed1b From 3de59bb9d551428cbdc76a9ea57883f82e350b4d Mon Sep 17 00:00:00 2001 From: Yufen Yu Date: Sat, 24 Feb 2018 12:05:56 +0800 Subject: md/raid1: fix NULL pointer dereference In handle_write_finished(), if r1_bio->bios[m] != NULL, it thinks the corresponding conf->mirrors[m].rdev is also not NULL. But, it is not always true. Even if some io hold replacement rdev(i.e. rdev->nr_pending.count > 0), raid1_remove_disk() can also set the rdev as NULL. That means, bios[m] != NULL, but mirrors[m].rdev is NULL, resulting in NULL pointer dereference in handle_write_finished and sync_request_write. This patch can fix BUGs as follows: BUG: unable to handle kernel NULL pointer dereference at 0000000000000140 IP: [] raid1d+0x2bd/0xfc0 PGD 12ab52067 PUD 12f587067 PMD 0 Oops: 0000 [#1] SMP CPU: 1 PID: 2008 Comm: md3_raid1 Not tainted 4.1.44+ #130 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26 04/01/2014 Call Trace: ? schedule+0x37/0x90 ? prepare_to_wait_event+0x83/0xf0 md_thread+0x144/0x150 ? wake_atomic_t_function+0x70/0x70 ? md_start_sync+0xf0/0xf0 kthread+0xd8/0xf0 ? kthread_worker_fn+0x160/0x160 ret_from_fork+0x42/0x70 ? kthread_worker_fn+0x160/0x160 BUG: unable to handle kernel NULL pointer dereference at 00000000000000b8 IP: sync_request_write+0x9e/0x980 PGD 800000007c518067 P4D 800000007c518067 PUD 8002b067 PMD 0 Oops: 0000 [#1] SMP PTI CPU: 24 PID: 2549 Comm: md3_raid1 Not tainted 4.15.0+ #118 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26 04/01/2014 Call Trace: ? sched_clock+0x5/0x10 ? sched_clock_cpu+0xc/0xb0 ? flush_pending_writes+0x3a/0xd0 ? pick_next_task_fair+0x4d5/0x5f0 ? __switch_to+0xa2/0x430 raid1d+0x65a/0x870 ? find_pers+0x70/0x70 ? find_pers+0x70/0x70 ? md_thread+0x11c/0x160 md_thread+0x11c/0x160 ? finish_wait+0x80/0x80 kthread+0x111/0x130 ? kthread_create_worker_on_cpu+0x70/0x70 ? do_syscall_64+0x6f/0x190 ? SyS_exit_group+0x10/0x10 ret_from_fork+0x35/0x40 Reviewed-by: NeilBrown Signed-off-by: Yufen Yu Signed-off-by: Shaohua Li --- drivers/md/raid1.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index f978eddc7a21..fe872dc6712e 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -1809,6 +1809,17 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev) struct md_rdev *repl = conf->mirrors[conf->raid_disks + number].rdev; freeze_array(conf, 0); + if (atomic_read(&repl->nr_pending)) { + /* It means that some queued IO of retry_list + * hold repl. Thus, we cannot set replacement + * as NULL, avoiding rdev NULL pointer + * dereference in sync_request_write and + * handle_write_finished. + */ + err = -EBUSY; + unfreeze_array(conf); + goto abort; + } clear_bit(Replacement, &repl->flags); p->rdev = repl; conf->mirrors[conf->raid_disks + number].rdev = NULL; -- cgit v1.2.3-59-g8ed1b