On Sun, Aug 20, 2023 at 5:13 PM 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;
>
> This 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 | 43 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index cdc361c521d4..f08b683f724f 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4866,6 +4866,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> /* A write to sync_action is enough to justify
> * canceling read-auto mode
> */
> + flush_work(&mddev->sync_work);
> mddev->ro = MD_RDWR;
> md_wakeup_thread(mddev->sync_thread);
> }
> @@ -7638,6 +7639,10 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
> mutex_unlock(&mddev->open_mutex);
> sync_blockdev(bdev);
> }
> +
> + if (!md_is_rdwr(mddev))
> + flush_work(&mddev->sync_work);
> +
> err = mddev_lock(mddev);
> if (err) {
> pr_debug("md: ioctl lock interrupted, reason %d, cmd %d\n",
> @@ -8562,6 +8567,7 @@ bool md_write_start(struct mddev *mddev, struct bio *bi)
> BUG_ON(mddev->ro == MD_RDONLY);
> if (mddev->ro == MD_AUTO_READ) {
> /* need to switch to read/write */
> + flush_work(&mddev->sync_work);
> mddev->ro = MD_RDWR;
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
> md_wakeup_thread(mddev->thread);
> @@ -9191,6 +9197,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)
> {
> @@ -9311,6 +9327,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_action(mddev, &spares))
> goto not_running;
>
> @@ -9405,7 +9427,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)) ||
> @@ -9433,15 +9456,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.
> - */
Can we keep those comments?
> - 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);
> @@ -9449,6 +9465,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
>
Reviewed-by: Xiao Ni <xni@xxxxxxxxxx>
