Message-ID: <2f53f9a8-380a-4fe4-8407-03d5b4e78140@huaweicloud.com>
Date: Sat, 26 Jul 2025 09:42:37 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, adilger.kernel@...ger.ca,
 ojaswin@...ux.ibm.com, linux@...ck-us.net, yi.zhang@...wei.com,
 libaokun1@...wei.com, yukuai3@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH] ext4: fix crash on test_mb_mark_used kunit tests

On 2025/7/25 21:15, Theodore Ts'o wrote:
> On Fri, Jul 25, 2025 at 01:06:18PM +0200, Jan Kara wrote:
>>> This patch applies to the kernel that has only merged bbe11dd13a3f
>>> ("ext4: fix largest free orders lists corruption on mb_optimize_scan
>>> switch"), but not merged 458bfb991155 ("ext4: convert free groups order
>>> lists to xarrays").
>>
>> Hum, I think it would be best to just squash this into bbe11dd13a3f and
>> then just rebase & squash the other unittest fixup to the final commit when
>> we have to rebase anyway. Because otherwise backports to stable kernel will
>> quickly become rather messy.
> 
> What I ended up doing was to add a squashed combination of these two
> commits and dropped it in before the block allocation scalabiltity
> with the following commit description:
> 
>     ext4: initialize superblock fields in the kballoc-test.c kunit tests
>     
>     Various changes in the "ext4: better scalability for ext4 block
>     allocation" patch series have resulted in kunit test failures, most
>     notably in the test_new_blocks_simple and the test_mb_mark_used tests.
>     The root cause of these failures is that various in-memory ext4 data
>     structures were not getting initialized, and while previous versions
>     of the functions exercised by the unit tests didn't use these
>     structure members, this was arguably a test bug.
>     
>     Since one of the patches in the block allocation scalability patches
>     is a fix which is has a cc:stable tag, this commit also has a
>     cc:stable tag.
>     
>     CC: stable@...r.kernel.org
>     Link: https://lore.kernel.org/r/20250714130327.1830534-1-libaokun1@huawei.com
>     Link: https://patch.msgid.link/20250725021550.3177573-1-yi.zhang@huaweicloud.com
>     Link: https://patch.msgid.link/20250725021654.3188798-1-yi.zhang@huaweicloud.com
>     Reported-by: Guenter Roeck <linux@...ck-us.net>
>     Closes: https://lore.kernel.org/linux-ext4/b0635ad0-7ebf-4152-a69b-58e7e87d5085@roeck-us.net/
>     Tested-by: Guenter Roeck <linux@...ck-us.net>
>     Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
>     Signed-off-by: Theodore Ts'o <tytso@....edu>
> 
> Then in the commit "ext4: convert free groups order lists to xarrays"
> which removed list_head, I modified it to remove the linked list
> initialization from mballoc-test.c, since that's the commit which
> removed those structures.
> 

Thank you for revising this series. This way, it will be less likely
to miss this fix when merging into the LTS branch.

> In the future, we should try to make sure that when we modify data
> structures to add or remove struct elements, that we also make sure
> that kunit test should also be updated.

Yes, currently in the Kunit tests, the initialization and maintenance
of data structures are too fragmented and fragile, making it easy to
overlook during modifications. In the future, I think we should provide
some general interfaces to handle the initialization and
deinitialization of those data structures.

> To that end, I've updated the
> kbuild script[1] in xfstests-bld repo so that "kbuild --test" will run
> the Kunit tests.  Hopefully reducing the friction for running tests
> will encourage more kunit tests to be created and so they will kept
> under regular maintenance.
> 
> [1] https://github.com/tytso/xfstests-bld/blob/master/kernel-build/kbuild
> 

That would be great! Then we won't miss Kunit tests again, and it
will also make those tests more useful. :-)

Best Regards,
Yi.

Powered by blists - more mailing lists