Date: Mon, 22 Jan 2024 16:24:03 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Song Liu <song@...nel.org>, Yu Kuai <yukuai1@...weicloud.com>,
 Mikulas Patocka <mpatocka@...hat.com>
Cc: dm-devel@...ts.linux.dev, msnitzer@...hat.com, heinzm@...hat.com,
 linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
 yi.zhang@...wei.com, yangerkun@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH 0/5] md: fix/prevent dm-raid regressions

Hi,

在 2024/01/21 12:41, Song Liu 写道:
> On Sat, Jan 20, 2024 at 2:41 AM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>
>> From: Yu Kuai <yukuai3@...wei.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
>>
>>
> .
> 

Powered by blists - more mailing lists