On Tue, Aug 15, 2023 at 6:07 PM Yu Kuai [off-list ref] wrote:

Hi,

在 2023/08/15 23:54, Song Liu 写道:

quoted

On Tue, Aug 15, 2023 at 2:00 PM Yu Kuai [off-list ref] wrote:
[...]

quoted

quoted

+
+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.

Unfortunately, we won't ship this (and the two other big sets) in 6.6.

Thanks,
Song