| Message ID | 20240220153059.11233-5-xni@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | Fix regression bugs | expand |
Hi, 在 2024/02/20 23:30, Xiao Ni 写道: > 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 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 > > Fixes: 486f60558607 ("md/raid5: Check all disks in a stripe_head for reshape progress") > Signed-off-by: Xiao Ni <xni@redhat.com> > --- > 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 >
On Fri, Feb 23, 2024 at 11:09 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: > > Hi, > > 在 2024/02/20 23:30, Xiao Ni 写道: > > 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 > > > > > Fixes: 486f60558607 ("md/raid5: Check all disks in a stripe_head for reshape progress") > > Signed-off-by: Xiao Ni <xni@redhat.com> > > --- > > 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 > > >
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
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 Fixes: 486f60558607 ("md/raid5: Check all disks in a stripe_head for reshape progress") Signed-off-by: Xiao Ni <xni@redhat.com> --- drivers/md/raid5.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)