[PATCH] ext4: fix crash on test_mb_mark_used kunit tests

[PATCH] ext4: fix crash on test_mb_mark_used kunit tests

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

mb_set_largest_free_order() requires the parameter bb_largest_free_order
and the list bb_largest_free_order_node to be initialized, and
mb_update_avg_fragment_size() requires the parameter
bb_avg_fragment_size_order and bb_avg_fragment_size_node to be
initialized too. But the test_mb_mark_used kunit tests do not init these
parameters, and trigger the following crash issue.

 Pid: 35, comm: kunit_try_catch Tainted: G W N 6.16.0-rc4-00031-gbbe11dd13a3f-dirty
 RIP: 0033:mb_set_largest_free_order+0x5c/0xc0
 RSP: 00000000a0883d98  EFLAGS: 00010206
 RAX: 0000000060aeaa28 RBX: 0000000060a2d400 RCX: 0000000000000008
 RDX: 0000000060aea9c0 RSI: 0000000000000000 RDI: 0000000060864000
 RBP: 0000000060aea9c0 R08: 0000000000000000 R09: 0000000060a2d400
 R10: 0000000000000400 R11: 0000000060a9cc00 R12: 0000000000000006
 R13: 0000000000000400 R14: 0000000000000305 R15: 0000000000000000
 Kernel panic - not syncing: Segfault with no mm
 CPU: 0 UID: 0 PID: 35 Comm: kunit_try_catch Tainted: G W N 6.16.0-rc4-00031-gbbe11dd13a3f-dirty #36 NONE
 Tainted: [W]=WARN, [N]=TEST
 Stack:
  60210c60 00000200 60a9e400 00000400
  40060300280 60864000 60a9cc00 60a2d400
  00000400 60aea9c0 60a9cc00 60aea9c0
 Call Trace:
  [<60210c60>] ? ext4_mb_generate_buddy+0x1f0/0x230
  [<60215c3b>] ? test_mb_mark_used+0x28b/0x4e0
  [<601df5bc>] ? ext4_get_group_desc+0xbc/0x150
  [<600bf1c0>] ? ktime_get_ts64+0x0/0x190
  [<60086370>] ? to_kthread+0x0/0x40
  [<602b559b>] ? kunit_try_run_case+0x7b/0x100
  [<60086370>] ? to_kthread+0x0/0x40
  [<602b7850>] ? kunit_generic_run_threadfn_adapter+0x0/0x30
  [<602b7862>] ? kunit_generic_run_threadfn_adapter+0x12/0x30
  [<60086a51>] ? kthread+0xf1/0x250
  [<6004a541>] ? new_thread_handler+0x41/0x60
 [ERROR] Test: test_mb_mark_used: 0 tests run!

Fixes: bbe11dd13a3f ("ext4: fix largest free orders lists corruption on mb_optimize_scan switch")
Reported-by: Theodore Ts'o <tytso@mit.edu>
Closes: https://lore.kernel.org/linux-ext4/20250724145437.GD80823@mit.edu/
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
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").

 fs/ext4/mballoc-test.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
index d634c12f1984..ba939be0ec55 100644
--- a/fs/ext4/mballoc-test.c
+++ b/fs/ext4/mballoc-test.c
@@ -802,6 +802,10 @@ static void test_mb_mark_used(struct kunit *test)
 	KUNIT_ASSERT_EQ(test, ret, 0);
 
 	grp->bb_free = EXT4_CLUSTERS_PER_GROUP(sb);
+	grp->bb_largest_free_order = -1;
+	grp->bb_avg_fragment_size_order = -1;
+	INIT_LIST_HEAD(&grp->bb_largest_free_order_node);
+	INIT_LIST_HEAD(&grp->bb_avg_fragment_size_node);
 	mbt_generate_test_ranges(sb, ranges, TEST_RANGE_COUNT);
 	for (i = 0; i < TEST_RANGE_COUNT; i++)
 		test_mb_mark_used_range(test, &e4b, ranges[i].start,
@@ -875,6 +879,10 @@ static void test_mb_free_blocks(struct kunit *test)
 	ext4_unlock_group(sb, TEST_GOAL_GROUP);
 
 	grp->bb_free = 0;
+	grp->bb_largest_free_order = -1;
+	grp->bb_avg_fragment_size_order = -1;
+	INIT_LIST_HEAD(&grp->bb_largest_free_order_node);
+	INIT_LIST_HEAD(&grp->bb_avg_fragment_size_node);
 	memset(bitmap, 0xff, sb->s_blocksize);
 
 	mbt_generate_test_ranges(sb, ranges, TEST_RANGE_COUNT);
-- 
2.46.1

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

[Jan Kara]

On Fri 25-07-25 10:16:54, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> mb_set_largest_free_order() requires the parameter bb_largest_free_order
> and the list bb_largest_free_order_node to be initialized, and
> mb_update_avg_fragment_size() requires the parameter
> bb_avg_fragment_size_order and bb_avg_fragment_size_node to be
> initialized too. But the test_mb_mark_used kunit tests do not init these
> parameters, and trigger the following crash issue.
> 
>  Pid: 35, comm: kunit_try_catch Tainted: G W N 6.16.0-rc4-00031-gbbe11dd13a3f-dirty
>  RIP: 0033:mb_set_largest_free_order+0x5c/0xc0
>  RSP: 00000000a0883d98  EFLAGS: 00010206
>  RAX: 0000000060aeaa28 RBX: 0000000060a2d400 RCX: 0000000000000008
>  RDX: 0000000060aea9c0 RSI: 0000000000000000 RDI: 0000000060864000
>  RBP: 0000000060aea9c0 R08: 0000000000000000 R09: 0000000060a2d400
>  R10: 0000000000000400 R11: 0000000060a9cc00 R12: 0000000000000006
>  R13: 0000000000000400 R14: 0000000000000305 R15: 0000000000000000
>  Kernel panic - not syncing: Segfault with no mm
>  CPU: 0 UID: 0 PID: 35 Comm: kunit_try_catch Tainted: G W N 6.16.0-rc4-00031-gbbe11dd13a3f-dirty #36 NONE
>  Tainted: [W]=WARN, [N]=TEST
>  Stack:
>   60210c60 00000200 60a9e400 00000400
>   40060300280 60864000 60a9cc00 60a2d400
>   00000400 60aea9c0 60a9cc00 60aea9c0
>  Call Trace:
>   [<60210c60>] ? ext4_mb_generate_buddy+0x1f0/0x230
>   [<60215c3b>] ? test_mb_mark_used+0x28b/0x4e0
>   [<601df5bc>] ? ext4_get_group_desc+0xbc/0x150
>   [<600bf1c0>] ? ktime_get_ts64+0x0/0x190
>   [<60086370>] ? to_kthread+0x0/0x40
>   [<602b559b>] ? kunit_try_run_case+0x7b/0x100
>   [<60086370>] ? to_kthread+0x0/0x40
>   [<602b7850>] ? kunit_generic_run_threadfn_adapter+0x0/0x30
>   [<602b7862>] ? kunit_generic_run_threadfn_adapter+0x12/0x30
>   [<60086a51>] ? kthread+0xf1/0x250
>   [<6004a541>] ? new_thread_handler+0x41/0x60
>  [ERROR] Test: test_mb_mark_used: 0 tests run!
> 
> Fixes: bbe11dd13a3f ("ext4: fix largest free orders lists corruption on mb_optimize_scan switch")
> Reported-by: Theodore Ts'o <tytso@mit.edu>
> Closes: https://lore.kernel.org/linux-ext4/20250724145437.GD80823@mit.edu/
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
> 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.

								Honza

 
>  fs/ext4/mballoc-test.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ext4/mballoc-test.c b/fs/ext4/mballoc-test.c
> index d634c12f1984..ba939be0ec55 100644
> --- a/fs/ext4/mballoc-test.c
> +++ b/fs/ext4/mballoc-test.c
> @@ -802,6 +802,10 @@ static void test_mb_mark_used(struct kunit *test)
>  	KUNIT_ASSERT_EQ(test, ret, 0);
>  
>  	grp->bb_free = EXT4_CLUSTERS_PER_GROUP(sb);
> +	grp->bb_largest_free_order = -1;
> +	grp->bb_avg_fragment_size_order = -1;
> +	INIT_LIST_HEAD(&grp->bb_largest_free_order_node);
> +	INIT_LIST_HEAD(&grp->bb_avg_fragment_size_node);
>  	mbt_generate_test_ranges(sb, ranges, TEST_RANGE_COUNT);
>  	for (i = 0; i < TEST_RANGE_COUNT; i++)
>  		test_mb_mark_used_range(test, &e4b, ranges[i].start,
> @@ -875,6 +879,10 @@ static void test_mb_free_blocks(struct kunit *test)
>  	ext4_unlock_group(sb, TEST_GOAL_GROUP);
>  
>  	grp->bb_free = 0;
> +	grp->bb_largest_free_order = -1;
> +	grp->bb_avg_fragment_size_order = -1;
> +	INIT_LIST_HEAD(&grp->bb_largest_free_order_node);
> +	INIT_LIST_HEAD(&grp->bb_avg_fragment_size_node);
>  	memset(bitmap, 0xff, sb->s_blocksize);
>  
>  	mbt_generate_test_ranges(sb, ranges, TEST_RANGE_COUNT);
> -- 
> 2.46.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Theodore Ts'o]

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.

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

Cheers,

					- Ted

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

[Zhang Yi]

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

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.

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

[Baokun Li]

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

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

[Theodore Ts'o]

On Sat, Jul 26, 2025 at 09:42:37AM +0800, Zhang Yi wrote:
> > 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.

Yes. I was thinking similar thoughts; perhap some of the structure
initialization should be refactored and put in mballoc.c instead of
mballoc-test.c.  Even if we have to have some #ifdef
CONFIG_EXT4_KUNIT_TESTS so that some of the test mocks are in same
place that the structure manipulation functions in a single file.

      	       		 	      		- Ted