On Fri, Feb 23, 2024 at 11:09 AM Yu Kuai [off-list ref] wrote:

Hi,

在 2024/02/20 23:30, Xiao Ni 写道:

quoted

stripe_ahead_of_reshape is used to check if a stripe region cross the
reshape position. So first, change the function name to
stripe_across_reshape to describe the usage of this function.

For reshape backwards, it starts reshape from the end of array and conf->
reshape_progress is init to raid5_size. During reshape, if previous is true
(set in make_stripe_request) and max_sector >= conf->reshape_progress, ios
should wait until reshape window moves forward. But ios don't need to wait
if max_sector is raid5_size.

And put the conditions into the function directly to make understand the
codes easily.

This can be reproduced easily by lvm2 test shell/lvconvert-raid-reshape.sh
For dm raid reshape, before starting sync thread, it needs to reload table
some times. In one time dm raid uses MD_RECOVERY_WAIT to delay reshape and
it doesn't start sync thread this time. Then one io comes in and it waits
because stripe_ahead_of_reshape returns true because it's a backward
reshape and max_sectors > conf->reshape_progress. But the reshape hasn't
started. So skip this check when reshape_progress is raid5_size

Like I said before, after following merged patch:

ad39c08186f8 md: Don't register sync_thread for reshape directly

Hi Kuai

The reason I send this patch set is I don't agree the patch 01 of your
patch set. Does the patch (md: Don't register sync_thread for reshape
directly) rely on patch 01 from your patch set? Because your patch 01
only changes one logic and other patches rely on it. So it's the
reason I can't accept the following patches.

Regards
Xiao

You should not see that sync_thread fo reshape is registered while
MD_RECOVERY_WAIT is set, so this patch should not be needed.

Thanks,
Kuai

quoted

Fixes: 486f60558607 ("md/raid5: Check all disks in a stripe_head for reshape progress")
Signed-off-by: Xiao Ni <redacted>
---
  drivers/md/raid5.c | 22 ++++++++++------------
  1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8497880135ee..4c71df4e2370 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c

@@ -5832,17 +5832,12 @@ static bool ahead_of_reshape(struct mddev *mddev, sector_t sector,
                                        sector >= reshape_sector;
  }

-static bool range_ahead_of_reshape(struct mddev *mddev, sector_t min,
-                                sector_t max, sector_t reshape_sector)
-{
-     return mddev->reshape_backwards ? max < reshape_sector :
-                                       min >= reshape_sector;
-}
-
-static bool stripe_ahead_of_reshape(struct mddev *mddev, struct r5conf *conf,
+static sector_t raid5_size(struct mddev *mddev, sector_t sectors, int raid_disks);
+static bool stripe_across_reshape(struct mddev *mddev, struct r5conf *conf,
                                  struct stripe_head *sh)
  {
      sector_t max_sector = 0, min_sector = MaxSector;
+     sector_t reshape_pos = 0;
      bool ret = false;
      int dd_idx;

@@ -5856,9 +5851,12 @@ static bool stripe_ahead_of_reshape(struct mddev *mddev, struct r5conf *conf,

      spin_lock_irq(&conf->device_lock);

-     if (!range_ahead_of_reshape(mddev, min_sector, max_sector,
-                                  conf->reshape_progress))
-             /* mismatch, need to try again */
+     reshape_pos = conf->reshape_progress;
+     if (mddev->reshape_backwards) {
+             if (max_sector >= reshape_pos &&
+                 reshape_pos != raid5_size(mddev, 0, 0))
+                     ret = true;
+     } else if (min_sector < reshape_pos)
              ret = true;

      spin_unlock_irq(&conf->device_lock);

@@ -5969,7 +5967,7 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
      }

      if (unlikely(previous) &&
-         stripe_ahead_of_reshape(mddev, conf, sh)) {
+         stripe_across_reshape(mddev, conf, sh)) {
              /*
               * Expansion moved on while waiting for a stripe.
               * Expansion could still move past after this