Hi, 在 2023/08/16 15:18, Xiao Ni 写道:
On Tue, Aug 15, 2023 at 11:13 AM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:From: Yu Kuai <yukuai3@xxxxxxxxxx> Before this patch, for read-only array: md_check_recovery() check that 'MD_RECOVERY_NEEDED' is set, then it will call remove_and_add_spares() directly to try to remove and add rdevs from array. After this patch: 1) md_check_recovery() check that 'MD_RECOVERY_NEEDED' is set, and the worker 'sync_work' is not pending, and there are rdevs can be added or removed, then it will queue new work md_start_sync(); 2) md_start_sync() will call remove_and_add_spares() and exist;Hi Kuai If md_check_recovery returns and md_starts_sync doesn't start, during the window the raid changes to read-write, the sync thread can be run but MD_RECOVERY_RUNNING isn't set. Is there such a problem?
That's a good question, looks like this is possible. Read-only array doesn't test or set MD_RECOVERY_RUNNING at all in md_check_recovery(). I'll use MD_RECOVERY_RUNNING to prevent such concurrent scenario in the new version. Thanks, Kuai
Regards XiaoThis change make sure that array reconfiguration is independent from daemon, and it'll be much easier to synchronize it with io, consier that io may rely on daemon thread to be done. Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx> --- drivers/md/md.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index d26d2c35f9af..74d529479fcf 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -9191,6 +9191,16 @@ static bool rdev_addable(struct md_rdev *rdev) return true; } +static bool md_spares_need_change(struct mddev *mddev) +{ + struct md_rdev *rdev; + + rdev_for_each(rdev, mddev) + if (rdev_removeable(rdev) || rdev_addable(rdev)) + return true; + return false; +} + static int remove_and_add_spares(struct mddev *mddev, struct md_rdev *this) { @@ -9309,6 +9319,12 @@ static void md_start_sync(struct work_struct *ws) mddev_lock_nointr(mddev); + if (!md_is_rdwr(mddev)) { + remove_and_add_spares(mddev, NULL); + mddev_unlock(mddev); + return; + } + if (!md_choose_sync_direction(mddev, &spares)) goto not_running; @@ -9403,7 +9419,8 @@ void md_check_recovery(struct mddev *mddev) } if (!md_is_rdwr(mddev) && - !test_bit(MD_RECOVERY_NEEDED, &mddev->recovery)) + (!test_bit(MD_RECOVERY_NEEDED, &mddev->recovery) || + work_pending(&mddev->sync_work))) return; if ( ! ( (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING)) || @@ -9431,15 +9448,8 @@ void md_check_recovery(struct mddev *mddev) */ rdev_for_each(rdev, mddev) clear_bit(Blocked, &rdev->flags); - /* On a read-only array we can: - * - remove failed devices - * - add already-in_sync devices if the array itself - * is in-sync. - * As we only add devices that are already in-sync, - * we can activate the spares immediately. - */ - remove_and_add_spares(mddev, NULL); - /* There is no thread, but we need to call + /* + * There is no thread, but we need to call * ->spare_active and clear saved_raid_disk */ set_bit(MD_RECOVERY_INTR, &mddev->recovery); @@ -9447,6 +9457,13 @@ void md_check_recovery(struct mddev *mddev) clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery); clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery); clear_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags); + + /* + * Let md_start_sync() to remove and add rdevs to the + * array. + */ + if (md_spares_need_change(mddev)) + queue_work(md_misc_wq, &mddev->sync_work); goto unlock; } -- 2.39.2.
