| Message ID | 20240120103734.4155446-1-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | md: fix/prevent dm-raid regressions | expand |
On Sat, Jan 20, 2024 at 2:41 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: > > From: Yu Kuai <yukuai3@huawei.com> > > There are some problems that we fixed in md/raid, and some apis is changed. > However, dm-raid rely the old apis(noted that old apis is problematic in > corner cases), and now there are regressions in lvm2 testsuite. > > This patchset fix some regressions(patch 1-3), and revert changes to > prevent regressions(patch 4,5). Noted that the problems in patch 4,5 is > not clear yet, and I'm not able to locate the root cause ASAP, hence I > decide to revert changes to prevent regressions first. Thanks for looking into this! Patch 1-3 look good to me. But since we need to back port these fixes to 6.7 kernels, let's make it very clear what issues are being fixed. Please: 1. Test on both Linus' master branch and 6.7.y, and explain which tests are failing before the fixes. (From my tests, the two branches don't have the same test results). We can put these results in the cover letter and include them in a merge commit. 2. If possible, add Fixes tag to all patches. 3. Add more details in the commit log, so it is clear what is being fixed. 4. Add "reported-by" and maybe also "closes" tag. For patch 4-5, especially 5, I wonder whether the same issue also happens with md. We can probably ship 4-5 now, with the same improvements as patch 1-3. I will run more tests on my side. Mykulas, please also review and test these patches. Thanks, Song > > Yu Kuai (5): > md: don't ignore suspended array in md_check_recovery() > md: don't ignore read-only array in md_check_recovery() > md: make sure md_do_sync() will set MD_RECOVERY_DONE > md: revert commit fa2bbff7b0b4 ("md: synchronize flush io with array > reconfiguration") for dm-raid > md: use md_reap_sync_thread() directly for dm-raid > > drivers/md/md.c | 58 ++++++++++++++++++++++++++++++------------------- > 1 file changed, 36 insertions(+), 22 deletions(-) > > -- > 2.39.2 > >
Hi, 在 2024/01/21 12:41, Song Liu 写道: > On Sat, Jan 20, 2024 at 2:41 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: >> >> From: Yu Kuai <yukuai3@huawei.com> >> >> There are some problems that we fixed in md/raid, and some apis is changed. >> However, dm-raid rely the old apis(noted that old apis is problematic in >> corner cases), and now there are regressions in lvm2 testsuite. >> >> This patchset fix some regressions(patch 1-3), and revert changes to >> prevent regressions(patch 4,5). Noted that the problems in patch 4,5 is >> not clear yet, and I'm not able to locate the root cause ASAP, hence I >> decide to revert changes to prevent regressions first. > > Thanks for looking into this! > > Patch 1-3 look good to me. But since we need to back port these fixes > to 6.7 kernels, let's make it very clear what issues are being fixed. > Please: > 1. Test on both Linus' master branch and 6.7.y, and explain which tests > are failing before the fixes. (From my tests, the two branches don't have > the same test results). We can put these results in the cover letter and > include them in a merge commit. > 2. If possible, add Fixes tag to all patches. > 3. Add more details in the commit log, so it is clear what is being fixed. > 4. Add "reported-by" and maybe also "closes" tag. > Will do this is the next version. I verified that the following tests will pass now in my VM: shell/integrity-caching.sh shell/lvconvert-raid-reshape.sh > For patch 4-5, especially 5, I wonder whether the same issue also > happens with md. We can probably ship 4-5 now, with the same > improvements as patch 1-3. With patch 1-3, the test lvconvert-raid-reshape.sh won't hang anymore, however it still fails and complain that ext4 is corrupted, and I'm still trying to understand how reshape works in dm-raid. :( > > I will run more tests on my side. Notice that the problem Mykulas mentioned in the patch md: partially revert "md/raid6: use valid sector values to determine if an I/O should wait on the reshape" still exist. And again, I'm stll trying to understand how raid5 works in detail. > > Mykulas, please also review and test these patches. > > Thanks, > Song > > > >> >> Yu Kuai (5): >> md: don't ignore suspended array in md_check_recovery() >> md: don't ignore read-only array in md_check_recovery() >> md: make sure md_do_sync() will set MD_RECOVERY_DONE >> md: revert commit fa2bbff7b0b4 ("md: synchronize flush io with array >> reconfiguration") for dm-raid >> md: use md_reap_sync_thread() directly for dm-raid >> >> drivers/md/md.c | 58 ++++++++++++++++++++++++++++++------------------- >> 1 file changed, 36 insertions(+), 22 deletions(-) >> >> -- >> 2.39.2 >> >> > . >
Hi, 在 2024/01/21 12:41, Song Liu 写道: > On Sat, Jan 20, 2024 at 2:41 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: >> >> From: Yu Kuai <yukuai3@huawei.com> >> >> There are some problems that we fixed in md/raid, and some apis is changed. >> However, dm-raid rely the old apis(noted that old apis is problematic in >> corner cases), and now there are regressions in lvm2 testsuite. >> >> This patchset fix some regressions(patch 1-3), and revert changes to >> prevent regressions(patch 4,5). Noted that the problems in patch 4,5 is >> not clear yet, and I'm not able to locate the root cause ASAP, hence I >> decide to revert changes to prevent regressions first. > > Thanks for looking into this! > > Patch 1-3 look good to me. But since we need to back port these fixes > to 6.7 kernels, let's make it very clear what issues are being fixed. > Please: I'm attaching my test result here, before I send the next version. The tested patched add following changes for patch 5: @@ -9379,6 +9387,15 @@ static void md_start_sync(struct work_struct *ws) suspend ? mddev_suspend_and_lock_nointr(mddev) : mddev_lock_nointr(mddev); + if (!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { + /* + * dm-raid calls md_reap_sync_thread() directly to unregister + * sync_thread, and md/raid should never trigger this. + */ + WARN_ON_ONCE(mddev->gendisk); + goto not_running;; + } + if (!md_is_rdwr(mddev)) { Failed tests for v6.6: ### failed: [ndev-vanilla] shell/duplicate-vgid.sh ### failed: [ndev-vanilla] shell/fsadm-crypt.sh ### failed: [ndev-vanilla] shell/lvchange-raid1-writemostly.sh ### failed: [ndev-vanilla] shell/lvconvert-cache-abort.sh ### failed: [ndev-vanilla] shell/lvconvert-repair-raid.sh ### failed: [ndev-vanilla] shell/lvcreate-large-raid.sh ### failed: [ndev-vanilla] shell/lvextend-raid.sh ### failed: [ndev-vanilla] shell/select-report.sh Failed tests for next-20240117(latest linux-next, between v6.7 to v6.8-rc1) ### failed: [ndev-vanilla] shell/duplicate-vgid.sh ### failed: [ndev-vanilla] shell/fsadm-crypt.sh ### failed: [ndev-vanilla] shell/lvchange-raid1-writemostly.sh ### failed: [ndev-vanilla] shell/lvconvert-repair-raid.sh ### failed: [ndev-vanilla] shell/lvextend-raid.sh ### failed: [ndev-vanilla] shell/select-report.sh Please noted that the test lvconvert-raid-reshape.sh is still possible to fail due to commit c467e97f079f ("md/raid6: use valid sector values to determine if an I/O should wait on the reshape"). Thanks, Kuai > 1. Test on both Linus' master branch and 6.7.y, and explain which tests > are failing before the fixes. (From my tests, the two branches don't have > the same test results). We can put these results in the cover letter and > include them in a merge commit. > 2. If possible, add Fixes tag to all patches. > 3. Add more details in the commit log, so it is clear what is being fixed. > 4. Add "reported-by" and maybe also "closes" tag. > > For patch 4-5, especially 5, I wonder whether the same issue also > happens with md. We can probably ship 4-5 now, with the same > improvements as patch 1-3. > > I will run more tests on my side. > > Mykulas, please also review and test these patches. > > Thanks, > Song > > > >> >> Yu Kuai (5): >> md: don't ignore suspended array in md_check_recovery() >> md: don't ignore read-only array in md_check_recovery() >> md: make sure md_do_sync() will set MD_RECOVERY_DONE >> md: revert commit fa2bbff7b0b4 ("md: synchronize flush io with array >> reconfiguration") for dm-raid >> md: use md_reap_sync_thread() directly for dm-raid >> >> drivers/md/md.c | 58 ++++++++++++++++++++++++++++++------------------- >> 1 file changed, 36 insertions(+), 22 deletions(-) >> >> -- >> 2.39.2 >> >> > . >
On Mon, Jan 22, 2024 at 12:24 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: > > Hi, > > 在 2024/01/21 12:41, Song Liu 写道: > > On Sat, Jan 20, 2024 at 2:41 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: > >> > >> From: Yu Kuai <yukuai3@huawei.com> > >> > >> There are some problems that we fixed in md/raid, and some apis is changed. > >> However, dm-raid rely the old apis(noted that old apis is problematic in > >> corner cases), and now there are regressions in lvm2 testsuite. > >> > >> This patchset fix some regressions(patch 1-3), and revert changes to > >> prevent regressions(patch 4,5). Noted that the problems in patch 4,5 is > >> not clear yet, and I'm not able to locate the root cause ASAP, hence I > >> decide to revert changes to prevent regressions first. > > > > Thanks for looking into this! > > > > Patch 1-3 look good to me. But since we need to back port these fixes > > to 6.7 kernels, let's make it very clear what issues are being fixed. > > Please: > > I'm attaching my test result here, before I send the next version. > > The tested patched add following changes for patch 5: > > @@ -9379,6 +9387,15 @@ static void md_start_sync(struct work_struct *ws) > suspend ? mddev_suspend_and_lock_nointr(mddev) : > mddev_lock_nointr(mddev); > > + if (!test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) { > + /* > + * dm-raid calls md_reap_sync_thread() directly to > unregister > + * sync_thread, and md/raid should never trigger this. > + */ > + WARN_ON_ONCE(mddev->gendisk); > + goto not_running;; > + } > + > if (!md_is_rdwr(mddev)) { > > Failed tests for v6.6: > ### failed: [ndev-vanilla] shell/duplicate-vgid.sh > ### failed: [ndev-vanilla] shell/fsadm-crypt.sh > ### failed: [ndev-vanilla] shell/lvchange-raid1-writemostly.sh > ### failed: [ndev-vanilla] shell/lvconvert-cache-abort.sh > ### failed: [ndev-vanilla] shell/lvconvert-repair-raid.sh > ### failed: [ndev-vanilla] shell/lvcreate-large-raid.sh > ### failed: [ndev-vanilla] shell/lvextend-raid.sh > ### failed: [ndev-vanilla] shell/select-report.sh > > Failed tests for next-20240117(latest linux-next, between v6.7 to v6.8-rc1) > ### failed: [ndev-vanilla] shell/duplicate-vgid.sh > ### failed: [ndev-vanilla] shell/fsadm-crypt.sh > ### failed: [ndev-vanilla] shell/lvchange-raid1-writemostly.sh > ### failed: [ndev-vanilla] shell/lvconvert-repair-raid.sh > ### failed: [ndev-vanilla] shell/lvextend-raid.sh > ### failed: [ndev-vanilla] shell/select-report.sh > > Please noted that the test lvconvert-raid-reshape.sh is still possible > to fail due to commit c467e97f079f ("md/raid6: use valid sector values > to determine if an I/O should wait on the reshape"). Thanks for the information! I will look closer into the raid6 issue. Song
From: Yu Kuai <yukuai3@huawei.com> There are some problems that we fixed in md/raid, and some apis is changed. However, dm-raid rely the old apis(noted that old apis is problematic in corner cases), and now there are regressions in lvm2 testsuite. This patchset fix some regressions(patch 1-3), and revert changes to prevent regressions(patch 4,5). Noted that the problems in patch 4,5 is not clear yet, and I'm not able to locate the root cause ASAP, hence I decide to revert changes to prevent regressions first. Yu Kuai (5): md: don't ignore suspended array in md_check_recovery() md: don't ignore read-only array in md_check_recovery() md: make sure md_do_sync() will set MD_RECOVERY_DONE md: revert commit fa2bbff7b0b4 ("md: synchronize flush io with array reconfiguration") for dm-raid md: use md_reap_sync_thread() directly for dm-raid drivers/md/md.c | 58 ++++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 22 deletions(-)