* [PATCH V4] md: move bitmap_destroy to the beginning of __md_stop
@ 2017-03-14 1:40 Guoqing Jiang
2017-03-14 18:05 ` Shaohua Li
0 siblings, 1 reply; 2+ messages in thread
From: Guoqing Jiang @ 2017-03-14 1:40 UTC (permalink / raw)
To: linux-raid; +Cc: neilb, shli, Guoqing Jiang
Since we have switched to sync way to handle METADATA_UPDATED
msg for md-cluster, then process_metadata_update is depended
on mddev->thread->wqueue.
With the new change, clustered raid could possible hang if
array received a METADATA_UPDATED msg after array unregistered
mddev->thread, so we need to stop clustered raid (bitmap_destroy
-> bitmap_free -> md_cluster_stop) earlier than unregister
thread (mddev_detach -> md_unregister_thread).
And this change should be safe for non-clustered raid since
all writes are stopped before the destroy. Also in md_run,
we activate the personality (pers->run()) before activating
the bitmap (bitmap_create()). So it is pleasingly symmetric
to stop the bitmap (bitmap_destroy()) before stopping the
personality (__md_stop() calls pers->free()), we achieve this
by move bitmap_destroy to the beginning of __md_stop.
But we don't want to break the codes for waiting behind IO as
Shaohua mentioned, so introduce bitmap_wait_behind_writes to
call the codes, and call the new fun in both mddev_detach and
bitmap_destroy, then we will not break original behind IO code
and also fit the new condition well.
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
Changes from v3:
1. move bitmap_destroy to __md_stop
2. add bitmap_wait_behind_writes to handle behind IO
drivers/md/bitmap.c | 17 +++++++++++++++++
drivers/md/bitmap.h | 1 +
drivers/md/md.c | 13 ++-----------
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index b6fa55a3cff8..20cad80d6e34 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1764,6 +1764,21 @@ void bitmap_free(struct bitmap *bitmap)
}
EXPORT_SYMBOL(bitmap_free);
+void bitmap_wait_behind_writes(struct mddev *mddev)
+{
+ struct bitmap *bitmap = mddev->bitmap;
+
+ /* wait for behind writes to complete */
+ if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
+ pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
+ mdname(mddev));
+ /* need to kick something here to make sure I/O goes? */
+ wait_event(bitmap->behind_wait,
+ atomic_read(&bitmap->behind_writes) == 0);
+ }
+}
+EXPORT_SYMBOL(bitmap_wait_behind_writes);
+
void bitmap_destroy(struct mddev *mddev)
{
struct bitmap *bitmap = mddev->bitmap;
@@ -1771,6 +1786,8 @@ void bitmap_destroy(struct mddev *mddev)
if (!bitmap) /* there was no bitmap */
return;
+ bitmap_wait_behind_writes(mddev);
+
mutex_lock(&mddev->bitmap_info.mutex);
spin_lock(&mddev->lock);
mddev->bitmap = NULL; /* disconnect from the md device */
diff --git a/drivers/md/bitmap.h b/drivers/md/bitmap.h
index 9f761097aab2..d15721ac07a6 100644
--- a/drivers/md/bitmap.h
+++ b/drivers/md/bitmap.h
@@ -271,6 +271,7 @@ struct bitmap *get_bitmap_from_slot(struct mddev *mddev, int slot);
int bitmap_copy_from_slot(struct mddev *mddev, int slot,
sector_t *lo, sector_t *hi, bool clear_bits);
void bitmap_free(struct bitmap *bitmap);
+void bitmap_wait_behind_writes(struct mddev *mddev);
#endif
#endif
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 79a99a1c9ce7..dc131fabfc7c 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5534,15 +5534,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes);
static void mddev_detach(struct mddev *mddev)
{
- struct bitmap *bitmap = mddev->bitmap;
- /* wait for behind writes to complete */
- if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
- pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
- mdname(mddev));
- /* need to kick something here to make sure I/O goes? */
- wait_event(bitmap->behind_wait,
- atomic_read(&bitmap->behind_writes) == 0);
- }
+ bitmap_wait_behind_writes(mddev);
if (mddev->pers && mddev->pers->quiesce) {
mddev->pers->quiesce(mddev, 1);
mddev->pers->quiesce(mddev, 0);
@@ -5555,6 +5547,7 @@ static void mddev_detach(struct mddev *mddev)
static void __md_stop(struct mddev *mddev)
{
struct md_personality *pers = mddev->pers;
+ bitmap_destroy(mddev);
mddev_detach(mddev);
/* Ensure ->event_work is done */
flush_workqueue(md_misc_wq);
@@ -5575,7 +5568,6 @@ void md_stop(struct mddev *mddev)
* This is called from dm-raid
*/
__md_stop(mddev);
- bitmap_destroy(mddev);
if (mddev->bio_set)
bioset_free(mddev->bio_set);
}
@@ -5713,7 +5705,6 @@ static int do_md_stop(struct mddev *mddev, int mode,
if (mode == 0) {
pr_info("md: %s stopped.\n", mdname(mddev));
- bitmap_destroy(mddev);
if (mddev->bitmap_info.file) {
struct file *f = mddev->bitmap_info.file;
spin_lock(&mddev->lock);
--
2.6.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH V4] md: move bitmap_destroy to the beginning of __md_stop
2017-03-14 1:40 [PATCH V4] md: move bitmap_destroy to the beginning of __md_stop Guoqing Jiang
@ 2017-03-14 18:05 ` Shaohua Li
0 siblings, 0 replies; 2+ messages in thread
From: Shaohua Li @ 2017-03-14 18:05 UTC (permalink / raw)
To: Guoqing Jiang; +Cc: linux-raid, neilb, shli
On Tue, Mar 14, 2017 at 09:40:20AM +0800, Guoqing Jiang wrote:
> Since we have switched to sync way to handle METADATA_UPDATED
> msg for md-cluster, then process_metadata_update is depended
> on mddev->thread->wqueue.
>
> With the new change, clustered raid could possible hang if
> array received a METADATA_UPDATED msg after array unregistered
> mddev->thread, so we need to stop clustered raid (bitmap_destroy
> -> bitmap_free -> md_cluster_stop) earlier than unregister
> thread (mddev_detach -> md_unregister_thread).
>
> And this change should be safe for non-clustered raid since
> all writes are stopped before the destroy. Also in md_run,
> we activate the personality (pers->run()) before activating
> the bitmap (bitmap_create()). So it is pleasingly symmetric
> to stop the bitmap (bitmap_destroy()) before stopping the
> personality (__md_stop() calls pers->free()), we achieve this
> by move bitmap_destroy to the beginning of __md_stop.
>
> But we don't want to break the codes for waiting behind IO as
> Shaohua mentioned, so introduce bitmap_wait_behind_writes to
> call the codes, and call the new fun in both mddev_detach and
> bitmap_destroy, then we will not break original behind IO code
> and also fit the new condition well.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> Changes from v3:
> 1. move bitmap_destroy to __md_stop
> 2. add bitmap_wait_behind_writes to handle behind IO
>
> drivers/md/bitmap.c | 17 +++++++++++++++++
> drivers/md/bitmap.h | 1 +
> drivers/md/md.c | 13 ++-----------
> 3 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index b6fa55a3cff8..20cad80d6e34 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -1764,6 +1764,21 @@ void bitmap_free(struct bitmap *bitmap)
> }
> EXPORT_SYMBOL(bitmap_free);
>
> +void bitmap_wait_behind_writes(struct mddev *mddev)
> +{
> + struct bitmap *bitmap = mddev->bitmap;
> +
> + /* wait for behind writes to complete */
> + if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
> + pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
> + mdname(mddev));
> + /* need to kick something here to make sure I/O goes? */
> + wait_event(bitmap->behind_wait,
> + atomic_read(&bitmap->behind_writes) == 0);
> + }
> +}
> +EXPORT_SYMBOL(bitmap_wait_behind_writes);
Applied, thanks! I deleted the EXPORT_SYMBOL here, it's not used by a module.
> void bitmap_destroy(struct mddev *mddev)
> {
> struct bitmap *bitmap = mddev->bitmap;
> @@ -1771,6 +1786,8 @@ void bitmap_destroy(struct mddev *mddev)
> if (!bitmap) /* there was no bitmap */
> return;
>
> + bitmap_wait_behind_writes(mddev);
> +
> mutex_lock(&mddev->bitmap_info.mutex);
> spin_lock(&mddev->lock);
> mddev->bitmap = NULL; /* disconnect from the md device */
> diff --git a/drivers/md/bitmap.h b/drivers/md/bitmap.h
> index 9f761097aab2..d15721ac07a6 100644
> --- a/drivers/md/bitmap.h
> +++ b/drivers/md/bitmap.h
> @@ -271,6 +271,7 @@ struct bitmap *get_bitmap_from_slot(struct mddev *mddev, int slot);
> int bitmap_copy_from_slot(struct mddev *mddev, int slot,
> sector_t *lo, sector_t *hi, bool clear_bits);
> void bitmap_free(struct bitmap *bitmap);
> +void bitmap_wait_behind_writes(struct mddev *mddev);
> #endif
>
> #endif
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 79a99a1c9ce7..dc131fabfc7c 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5534,15 +5534,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes);
>
> static void mddev_detach(struct mddev *mddev)
> {
> - struct bitmap *bitmap = mddev->bitmap;
> - /* wait for behind writes to complete */
> - if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
> - pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
> - mdname(mddev));
> - /* need to kick something here to make sure I/O goes? */
> - wait_event(bitmap->behind_wait,
> - atomic_read(&bitmap->behind_writes) == 0);
> - }
> + bitmap_wait_behind_writes(mddev);
> if (mddev->pers && mddev->pers->quiesce) {
> mddev->pers->quiesce(mddev, 1);
> mddev->pers->quiesce(mddev, 0);
> @@ -5555,6 +5547,7 @@ static void mddev_detach(struct mddev *mddev)
> static void __md_stop(struct mddev *mddev)
> {
> struct md_personality *pers = mddev->pers;
> + bitmap_destroy(mddev);
> mddev_detach(mddev);
> /* Ensure ->event_work is done */
> flush_workqueue(md_misc_wq);
> @@ -5575,7 +5568,6 @@ void md_stop(struct mddev *mddev)
> * This is called from dm-raid
> */
> __md_stop(mddev);
> - bitmap_destroy(mddev);
> if (mddev->bio_set)
> bioset_free(mddev->bio_set);
> }
> @@ -5713,7 +5705,6 @@ static int do_md_stop(struct mddev *mddev, int mode,
> if (mode == 0) {
> pr_info("md: %s stopped.\n", mdname(mddev));
>
> - bitmap_destroy(mddev);
> if (mddev->bitmap_info.file) {
> struct file *f = mddev->bitmap_info.file;
> spin_lock(&mddev->lock);
> --
> 2.6.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-14 18:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-14 1:40 [PATCH V4] md: move bitmap_destroy to the beginning of __md_stop Guoqing Jiang
2017-03-14 18:05 ` Shaohua Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.