From: NeilBrown <neilb@suse.com>
To: Shaohua Li <shli@kernel.org>
Cc: linux-raid@vger.kernel.org
Subject: [md PATCH 6/5] md: remove special meaning of ->quiesce(.., 2)
Date: Thu, 19 Oct 2017 12:49:15 +1100 [thread overview]
Message-ID: <87infbhppg.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <87bml4j43c.fsf@notabene.neil.brown.name>
[-- Attachment #1: Type: text/plain, Size: 9986 bytes --]
The '2' argument means "wake up anything that is waiting".
This is an inelegant part of the design and was added
to help support management of suspend_lo/suspend_hi setting.
Now that suspend_lo/hi is managed in mddev_suspend/resume,
that need is gone.
These is still a couple of places where we call 'quiesce'
with an argument of '2', but they can safely be changed to
call ->quiesce(.., 1); ->quiesce(.., 0) which
achieve the same result at the small cost of pausing IO
briefly.
This removes a small "optimization" from suspend_{hi,lo}_store,
but it isn't clear that optimization served a useful purpose.
The code now is a lot clearer.
Suggested-by: Shaohua Li <shli@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.com>
---
As suggested, this remove the special meaning of '2'.
Thanks,
NeilBrown
drivers/md/md-cluster.c | 6 +++---
drivers/md/md.c | 34 ++++++++++------------------------
drivers/md/md.h | 9 ++++-----
drivers/md/raid0.c | 2 +-
drivers/md/raid1.c | 13 +++----------
drivers/md/raid10.c | 10 +++-------
drivers/md/raid5-cache.c | 12 ++++++------
drivers/md/raid5-log.h | 2 +-
drivers/md/raid5.c | 18 ++++++------------
9 files changed, 37 insertions(+), 69 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 03082e17c65c..72ce0bccc865 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -442,10 +442,11 @@ static void __remove_suspend_info(struct md_cluster_info *cinfo, int slot)
static void remove_suspend_info(struct mddev *mddev, int slot)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ mddev->pers->quiesce(mddev, 1);
spin_lock_irq(&cinfo->suspend_lock);
__remove_suspend_info(cinfo, slot);
spin_unlock_irq(&cinfo->suspend_lock);
- mddev->pers->quiesce(mddev, 2);
+ mddev->pers->quiesce(mddev, 0);
}
@@ -492,13 +493,12 @@ static void process_suspend_info(struct mddev *mddev,
s->lo = lo;
s->hi = hi;
mddev->pers->quiesce(mddev, 1);
- mddev->pers->quiesce(mddev, 0);
spin_lock_irq(&cinfo->suspend_lock);
/* Remove existing entry (if exists) before adding */
__remove_suspend_info(cinfo, slot);
list_add(&s->list, &cinfo->suspend_list);
spin_unlock_irq(&cinfo->suspend_lock);
- mddev->pers->quiesce(mddev, 2);
+ mddev->pers->quiesce(mddev, 0);
}
static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 4be3140adfe8..e1e7e8dc6878 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4857,7 +4857,7 @@ suspend_lo_show(struct mddev *mddev, char *page)
static ssize_t
suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
{
- unsigned long long old, new;
+ unsigned long long new;
int err;
err = kstrtoull(buf, 10, &new);
@@ -4873,17 +4873,10 @@ suspend_lo_store(struct mddev *mddev, const char *buf, size_t len)
if (mddev->pers == NULL ||
mddev->pers->quiesce == NULL)
goto unlock;
- old = mddev->suspend_lo;
+ mddev_suspend(mddev);
mddev->suspend_lo = new;
- if (new >= old) {
- /* Shrinking suspended region */
- wake_up(&mddev->sb_wait);
- mddev->pers->quiesce(mddev, 2);
- } else {
- /* Expanding suspended region - need to wait */
- mddev_suspend(mddev);
- mddev_resume(mddev);
- }
+ mddev_resume(mddev);
+
err = 0;
unlock:
mddev_unlock(mddev);
@@ -4901,7 +4894,7 @@ suspend_hi_show(struct mddev *mddev, char *page)
static ssize_t
suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
{
- unsigned long long old, new;
+ unsigned long long new;
int err;
err = kstrtoull(buf, 10, &new);
@@ -4914,20 +4907,13 @@ suspend_hi_store(struct mddev *mddev, const char *buf, size_t len)
if (err)
return err;
err = -EINVAL;
- if (mddev->pers == NULL ||
- mddev->pers->quiesce == NULL)
+ if (mddev->pers == NULL)
goto unlock;
- old = mddev->suspend_hi;
+
+ mddev_suspend(mddev);
mddev->suspend_hi = new;
- if (new <= old) {
- /* Shrinking suspended region */
- wake_up(&mddev->sb_wait);
- mddev->pers->quiesce(mddev, 2);
- } else {
- /* Expanding suspended region - need to wait */
- mddev_suspend(mddev);
- mddev_resume(mddev);
- }
+ mddev_resume(mddev);
+
err = 0;
unlock:
mddev_unlock(mddev);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 8c2158f3bd59..5dcdba84932f 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -545,12 +545,11 @@ struct md_personality
int (*check_reshape) (struct mddev *mddev);
int (*start_reshape) (struct mddev *mddev);
void (*finish_reshape) (struct mddev *mddev);
- /* quiesce moves between quiescence states
- * 0 - fully active
- * 1 - no new requests allowed
- * others - reserved
+ /* quiesce suspends or resumes internal processing.
+ * 1 - stop new actions and wait for action io to complete
+ * 0 - return to normal behaviour
*/
- void (*quiesce) (struct mddev *mddev, int state);
+ void (*quiesce) (struct mddev *mddev, int quiesce);
/* takeover is used to transition an array from one
* personality to another. The new personality must be able
* to handle the data in the current layout.
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 5a00fc118470..5ecba9eef441 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -768,7 +768,7 @@ static void *raid0_takeover(struct mddev *mddev)
return ERR_PTR(-EINVAL);
}
-static void raid0_quiesce(struct mddev *mddev, int state)
+static void raid0_quiesce(struct mddev *mddev, int quiesce)
{
}
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 277a145b3ff5..5f21cb9ac778 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -3273,21 +3273,14 @@ static int raid1_reshape(struct mddev *mddev)
return 0;
}
-static void raid1_quiesce(struct mddev *mddev, int state)
+static void raid1_quiesce(struct mddev *mddev, int quiesce)
{
struct r1conf *conf = mddev->private;
- switch(state) {
- case 2: /* wake for suspend */
- wake_up(&conf->wait_barrier);
- break;
- case 1:
+ if (quiesce)
freeze_array(conf, 0);
- break;
- case 0:
+ else
unfreeze_array(conf);
- break;
- }
}
static void *raid1_takeover(struct mddev *mddev)
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 374df5796649..76f042bf9a57 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3832,18 +3832,14 @@ static void raid10_free(struct mddev *mddev, void *priv)
kfree(conf);
}
-static void raid10_quiesce(struct mddev *mddev, int state)
+static void raid10_quiesce(struct mddev *mddev, int quiesce)
{
struct r10conf *conf = mddev->private;
- switch(state) {
- case 1:
+ if (quiesce)
raise_barrier(conf, 0);
- break;
- case 0:
+ else
lower_barrier(conf);
- break;
- }
}
static int raid10_resize(struct mddev *mddev, sector_t sectors)
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 6a631dd21f0b..fce290878eb5 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -1589,21 +1589,21 @@ void r5l_wake_reclaim(struct r5l_log *log, sector_t space)
md_wakeup_thread(log->reclaim_thread);
}
-void r5l_quiesce(struct r5l_log *log, int state)
+void r5l_quiesce(struct r5l_log *log, int quiesce)
{
struct mddev *mddev;
- if (!log || state == 2)
+ if (!log)
return;
- if (state == 0)
- kthread_unpark(log->reclaim_thread->tsk);
- else if (state == 1) {
+
+ if (quiesce) {
/* make sure r5l_write_super_and_discard_space exits */
mddev = log->rdev->mddev;
wake_up(&mddev->sb_wait);
kthread_park(log->reclaim_thread->tsk);
r5l_wake_reclaim(log, MaxSector);
r5l_do_reclaim(log);
- }
+ } else
+ kthread_unpark(log->reclaim_thread->tsk);
}
bool r5l_log_disk_error(struct r5conf *conf)
diff --git a/drivers/md/raid5-log.h b/drivers/md/raid5-log.h
index 328d67aedda4..c3596a27a5a8 100644
--- a/drivers/md/raid5-log.h
+++ b/drivers/md/raid5-log.h
@@ -8,7 +8,7 @@ extern void r5l_write_stripe_run(struct r5l_log *log);
extern void r5l_flush_stripe_to_raid(struct r5l_log *log);
extern void r5l_stripe_write_finished(struct stripe_head *sh);
extern int r5l_handle_flush_request(struct r5l_log *log, struct bio *bio);
-extern void r5l_quiesce(struct r5l_log *log, int state);
+extern void r5l_quiesce(struct r5l_log *log, int quiesce);
extern bool r5l_log_disk_error(struct r5conf *conf);
extern bool r5c_is_writeback(struct r5l_log *log);
extern int
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 89ad79614309..13db3d31e983 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8026,16 +8026,12 @@ static void raid5_finish_reshape(struct mddev *mddev)
}
}
-static void raid5_quiesce(struct mddev *mddev, int state)
+static void raid5_quiesce(struct mddev *mddev, int quiesce)
{
struct r5conf *conf = mddev->private;
- switch(state) {
- case 2: /* resume for a suspend */
- wake_up(&conf->wait_for_overlap);
- break;
-
- case 1: /* stop all writes */
+ if (quiesce) {
+ /* stop all writes */
lock_all_device_hash_locks_irq(conf);
/* '2' tells resync/reshape to pause so that all
* active stripes can drain
@@ -8051,17 +8047,15 @@ static void raid5_quiesce(struct mddev *mddev, int state)
unlock_all_device_hash_locks_irq(conf);
/* allow reshape to continue */
wake_up(&conf->wait_for_overlap);
- break;
-
- case 0: /* re-enable writes */
+ } else {
+ /* re-enable writes */
lock_all_device_hash_locks_irq(conf);
conf->quiesce = 0;
wake_up(&conf->wait_for_quiescent);
wake_up(&conf->wait_for_overlap);
unlock_all_device_hash_locks_irq(conf);
- break;
}
- r5l_quiesce(conf->log, state);
+ r5l_quiesce(conf->log, quiesce);
}
static void *raid45_takeover_raid0(struct mddev *mddev, int level)
--
2.14.0.rc0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2017-10-19 1:49 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-17 2:46 [md PATCH 0/5] Address deadlock associated with setting suspend_lo NeilBrown
2017-10-17 2:46 ` [md PATCH 2/5] md: don't call bitmap_create() while array is quiesced NeilBrown
2017-10-17 2:46 ` [md PATCH 3/5] md: move suspend_hi/lo handling into core md code NeilBrown
2017-10-18 6:16 ` Shaohua Li
2017-10-18 7:40 ` NeilBrown
2017-10-19 1:49 ` NeilBrown [this message]
2017-10-17 2:46 ` [md PATCH 5/5] md: allow metadata update while suspending NeilBrown
2017-10-17 2:46 ` [md PATCH 4/5] md: use mddev_suspend/resume instead of ->quiesce() NeilBrown
2017-10-17 2:46 ` [md PATCH 1/5] md: always hold reconfig_mutex when calling mddev_suspend() NeilBrown
2017-10-18 6:11 ` Shaohua Li
2017-10-18 7:35 ` NeilBrown
2017-10-19 1:17 ` [md PATCH 1/5 v2] " NeilBrown
2017-10-19 3:45 ` Shaohua Li
2017-10-19 6:29 ` NeilBrown
2017-10-20 4:37 ` Shaohua Li
2017-10-23 0:02 ` NeilBrown
2017-10-23 1:48 ` Shaohua Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87infbhppg.fsf@notabene.neil.brown.name \
--to=neilb@suse.com \
--cc=linux-raid@vger.kernel.org \
--cc=shli@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).