Messages in this thread | | | Subject | Re: [PATCH -next v2 3/7] md: delay choosing sync direction to md_start_sync() | From | Yu Kuai <> | Date | Sun, 20 Aug 2023 09:45:48 +0800 |
| |
Hi,
在 2023/08/18 5:53, Song Liu 写道: > On Tue, Aug 15, 2023 at 6:07 PM Yu Kuai <yukuai1@huaweicloud.com> wrote: >> >> 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. > > Please fix this, address comments from other reviews, and resend the > patches. Also, there are some typos in the commit logs, please also fix them. >
Of course, and sorry for the dealy, I was ill and rested at home for a few days.
Thanks, Kuai
> Unfortunately, we won't ship this (and the two other big sets) in 6.6. > > Thanks, > Song > . >
| |