Messages in this thread | | | Subject | Re: [PATCH -next v2 3/7] md: delay choosing sync direction to md_start_sync() | From | Yu Kuai <> | Date | Wed, 16 Aug 2023 09:07:25 +0800 |
| |
Hi,
在 2023/08/15 23:54, Song Liu 写道: > On Tue, Aug 15, 2023 at 2:00 PM Yu Kuai <yukuai1@huaweicloud.com> wrote: > [...] >>> + >>> +not_running: >>> + clear_bit(MD_RECOVERY_SYNC, &mddev->recovery); >>> + clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery); >>> + clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery); >>> + clear_bit(MD_RECOVERY_CHECK, &mddev->recovery); >>> + clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery); >>> + mddev_unlock(mddev); >>> + >>> + wake_up(&resync_wait); >>> + if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) && >>> + mddev->sysfs_action) >>> + sysfs_notify_dirent_safe(mddev->sysfs_action); >>> } >>> >>> /* >>> @@ -9379,7 +9402,6 @@ void md_check_recovery(struct mddev *mddev) >>> return; >>> >>> if (mddev_trylock(mddev)) { >>> - int spares = 0; >>> bool try_set_sync = mddev->safemode != 0; >>> >>> if (!mddev->external && mddev->safemode == 1) >>> @@ -9467,29 +9489,11 @@ void md_check_recovery(struct mddev *mddev) >>> clear_bit(MD_RECOVERY_DONE, &mddev->recovery); >>> >>> if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) || >>> - test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) >>> - goto not_running; >>> - if (!md_choose_sync_direction(mddev, &spares)) >>> - goto not_running; >>> - if (mddev->pers->sync_request) { >>> - if (spares) { >>> - /* We are adding a device or devices to an array >>> - * which has the bitmap stored on all devices. >>> - * So make sure all bitmap pages get written >>> - */ >>> - md_bitmap_write_all(mddev->bitmap); >>> - } >>> + test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) { >> >> Sorry that I made a mistake here while rebasing v2, here should be >> >> !test_bit(MD_RECOVERY_FROZEN, &mddev->recovery) >> >> With this fixed, there are no new regression for mdadm tests using loop >> devicein my VM. > > if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) || > !test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) { > queue_work(md_misc_wq, &mddev->sync_work); > } else { > > This doesn't look right. Should we do > > if (test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) && > !test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) { > queue_work(md_misc_wq, &mddev->sync_work); > } else { > > instead? >
Yes you're right, this is exactly what I did in v1, sorry that I keep making mistake while rebasing.
Thanks, Kuai
> Thanks, > Song > . >
| |