Re: [PATCH] ext4: fix crash on test_mb_mark_used kunit tests

[Baokun Li <libaokun1@huawei.com>]

On 7/25/2025 9:15 PM, 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@vger.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@roeck-us.net>
>      Closes: https://lore.kernel.org/linux-ext4/b0635ad0-7ebf-4152-a69b-58e7e87d5085@roeck-us.net/
>      Tested-by: Guenter Roeck <linux@roeck-us.net>
>      Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>      Signed-off-by: Theodore Ts'o <tytso@mit.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.

This looks good to me. Thank you for helping to adapt this patch!

>
> 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.  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

Yeah, unit tests are a much more efficient way to catch problems compared
to full system tests. Running them regularly would be a great way to
quickly surface issues.

On top of that, I think it's worth revisiting our current code and cleaning
up some of the logic. Specifically, refactoring initialization functions to
align with the single-responsibility principle would enable reuse between
production and testing flows, and minimize strange edge cases we’ve been
seeing.


Cheers,
Baokun