Re: [PATCH V3 2/2] Revert "md/raid5: Wait for MD_SB_CHANGE_PENDING in raid5d"

Hi Song,

These two patches get reviewed-by from Kuai and Logan, I didn't see them 
in your tree yet, would you like review it and pick it up?

Thanks,

Junxiao.

On 11/8/23 10:28 PM, Yu Kuai wrote:

在 2023/11/09 2:22, Junxiao Bi 写道:

quoted

This reverts commit 5e2cf333b7bd5d3e62595a44d598a254c697cd74.

That commit introduced the following race and can cause system hung.

  md_write_start:             raid5d:
  // mddev->in_sync == 1
  set "MD_SB_CHANGE_PENDING"
                             // running before md_write_start wakeup it
                              waiting "MD_SB_CHANGE_PENDING" cleared

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

hung

  wakeup mddev->thread
  ...
  waiting "MD_SB_CHANGE_PENDING" cleared
  >>>> hung, raid5d should clear this flag
  but get hung by same flag.

The issue reverted commit fixing is fixed by last patch in a new way.

Fixes: 5e2cf333b7bd ("md/raid5: Wait for MD_SB_CHANGE_PENDING in 
raid5d")
Signed-off-by: Junxiao Bi <redacted>

LGTM
Reviewed-by: Yu Kuai <redacted>

quoted

---
  drivers/md/raid5.c | 12 ------------
  1 file changed, 12 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index dc031d42f53b..fcc8a44dd4fd 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c

@@ -36,7 +36,6 @@

   */
    #include <linux/blkdev.h>
-#include <linux/delay.h>
  #include <linux/kthread.h>
  #include <linux/raid/pq.h>
  #include <linux/async_tx.h>

@@ -6820,18 +6819,7 @@ static void raid5d(struct md_thread *thread)

              spin_unlock_irq(&conf->device_lock);
              md_check_recovery(mddev);
              spin_lock_irq(&conf->device_lock);
-
-            /*
-             * Waiting on MD_SB_CHANGE_PENDING below may deadlock
-             * seeing md_check_recovery() is needed to clear
-             * the flag when using mdmon.
-             */
-            continue;
          }
-
-        wait_event_lock_irq(mddev->sb_wait,
-            !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags),
-            conf->device_lock);
      }
      pr_debug("%d stripes handled\n", handled);