From: Yu Kuai <yukuai1@huaweicloud.com>
To: guoqing.jiang@linux.dev, logang@deltatee.com,
pmenzel@molgen.mpg.de, agk@redhat.com, snitzer@kernel.org,
song@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
yukuai3@huawei.com, yukuai1@huaweicloud.com, yi.zhang@huawei.com,
yangerkun@huawei.com
Subject: [PATCH -next 4/6] md: refactor idle/frozen_sync_thread()
Date: Wed, 22 Mar 2023 14:41:20 +0800 [thread overview]
Message-ID: <20230322064122.2384589-5-yukuai1@huaweicloud.com> (raw)
In-Reply-To: <20230322064122.2384589-1-yukuai1@huaweicloud.com>
From: Yu Kuai <yukuai3@huawei.com>
Our test found a following deadlock in raid10:
1) Issue a normal write, and such write failed:
raid10_end_write_request
set_bit(R10BIO_WriteError, &r10_bio->state)
one_write_done
reschedule_retry
// later from md thread
raid10d
handle_write_completed
list_add(&r10_bio->retry_list, &conf->bio_end_io_list)
// later from md thread
raid10d
if (!test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags))
list_move(conf->bio_end_io_list.prev, &tmp)
r10_bio = list_first_entry(&tmp, struct r10bio, retry_list)
raid_end_bio_io(r10_bio)
Dependency chain 1: normal io is waiting for updating superblock
2) Trigger a recovery:
raid10_sync_request
raise_barrier
Dependency chain 2: sync thread is waiting for normal io
3) echo idle/frozen to sync_action:
action_store
mddev_lock
md_unregister_thread
kthread_stop
Dependency chain 3: drop 'reconfig_mutex' is waiting for sync thread
4) md thread can't update superblock:
raid10d
md_check_recovery
if (mddev_trylock(mddev))
md_update_sb
Dependency chain 4: update superblock is waiting for 'reconfig_mutex'
Hence cyclic dependency exist, in order to fix the problem, we must
break one of them. Dependency 1 and 2 can't be broken because they are
foundation design. Dependency 4 may be possible if it can be guaranteed
that no io can be inflight, however, this requires a new mechanism which
seems complex. Dependency 3 is a good choice, because idle/frozen only
requires sync thread to finish, which can be done asynchronously that is
already implemented, and 'reconfig_mutex' is not needed anymore.
This patch switch 'idle' and 'frozen' to wait sync thread to be done
asynchronously, and this patch also add a sequence counter to record how
many times sync thread is done, so that 'idle' won't keep waiting on new
started sync thread.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
drivers/md/md.c | 24 ++++++++++++++++++++----
drivers/md/md.h | 2 ++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 223c03149852..ddf33a11d8de 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -662,6 +662,7 @@ void mddev_init(struct mddev *mddev)
timer_setup(&mddev->safemode_timer, md_safemode_timeout, 0);
atomic_set(&mddev->active, 1);
atomic_set(&mddev->openers, 0);
+ atomic_set(&mddev->sync_seq, 0);
spin_lock_init(&mddev->lock);
atomic_set(&mddev->flush_pending, 0);
init_waitqueue_head(&mddev->sb_wait);
@@ -4774,19 +4775,28 @@ static void stop_sync_thread(struct mddev *mddev)
if (work_pending(&mddev->del_work))
flush_workqueue(md_misc_wq);
- if (mddev->sync_thread) {
- set_bit(MD_RECOVERY_INTR, &mddev->recovery);
- md_reap_sync_thread(mddev);
- }
+ set_bit(MD_RECOVERY_INTR, &mddev->recovery);
+ /*
+ * Thread might be blocked waiting for metadata update which will now
+ * never happen.
+ */
+ if (mddev->sync_thread)
+ wake_up_process(mddev->sync_thread->tsk);
mddev_unlock(mddev);
}
static void idle_sync_thread(struct mddev *mddev)
{
+ int sync_seq = atomic_read(&mddev->sync_seq);
+
mutex_lock(&mddev->sync_mutex);
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
stop_sync_thread(mddev);
+
+ wait_event(resync_wait, sync_seq != atomic_read(&mddev->sync_seq) ||
+ !test_bit(MD_RECOVERY_RUNNING, &mddev->recovery));
+
mutex_unlock(&mddev->sync_mutex);
}
@@ -4795,6 +4805,10 @@ static void frozen_sync_thread(struct mddev *mddev)
mutex_lock(&mddev->sync_mutex);
set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
stop_sync_thread(mddev);
+
+ wait_event(resync_wait, mddev->sync_thread == NULL &&
+ !test_bit(MD_RECOVERY_RUNNING, &mddev->recovery));
+
mutex_unlock(&mddev->sync_mutex);
}
@@ -9451,6 +9465,8 @@ void md_reap_sync_thread(struct mddev *mddev)
/* resync has finished, collect result */
md_unregister_thread(&mddev->sync_thread);
+ atomic_inc(&mddev->sync_seq);
+
if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery) &&
!test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery) &&
mddev->degraded != mddev->raid_disks) {
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 64474e458545..10d425a3daa3 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -537,6 +537,8 @@ struct mddev {
/* Used to synchronize idle and frozen for action_store() */
struct mutex sync_mutex;
+ /* The sequence number for sync thread */
+ atomic_t sync_seq;
};
enum recovery_flags {
--
2.31.1
next prev parent reply other threads:[~2023-03-22 7:02 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-22 6:41 [PATCH -next 0/6] md: fix that MD_RECOVERY_RUNNING can be cleared while sync_thread is still running Yu Kuai
2023-03-22 6:41 ` [PATCH -next 1/6] Revert "md: unlock mddev before reap sync_thread in action_store" Yu Kuai
2023-03-22 7:19 ` Guoqing Jiang
2023-03-22 9:00 ` Yu Kuai
2023-03-22 14:32 ` Guoqing Jiang
2023-03-23 1:36 ` Yu Kuai
2023-03-23 3:50 ` Guoqing Jiang
2023-03-23 6:32 ` Yu Kuai
2023-03-28 23:58 ` Song Liu
2023-04-06 8:53 ` Yu Kuai
2023-05-05 9:05 ` Yu Kuai
2023-03-22 6:41 ` [PATCH -next 2/6] md: refactor action_store() for 'idle' and 'frozen' Yu Kuai
2023-03-22 6:41 ` [PATCH -next 3/6] md: add a mutex to synchronize idle and frozen in action_store() Yu Kuai
2023-03-22 6:41 ` Yu Kuai [this message]
2023-03-22 6:41 ` [PATCH -next 5/6] md: wake up 'resync_wait' at last in md_reap_sync_thread() Yu Kuai
2023-03-22 6:41 ` [PATCH -next 6/6] md: enhance checking in md_check_recovery() Yu Kuai
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=20230322064122.2384589-5-yukuai1@huaweicloud.com \
--to=yukuai1@huaweicloud.com \
--cc=agk@redhat.com \
--cc=guoqing.jiang@linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=logang@deltatee.com \
--cc=pmenzel@molgen.mpg.de \
--cc=snitzer@kernel.org \
--cc=song@kernel.org \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
/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).