Re: [PATCH 03/27] ext4: add KUnit tests for ext4_ext_split() leaf and index splitting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] ext4: unexplained test_swap_extents() changes and stale Fixes
  SHA
--

commit 55141b7904f9009059aca2d2a02dc5c501ed9bde
Author: Theodore Ts'o <tytso@mit.edu>

> ext4: add KUnit tests for ext4_ext_split() leaf and index splitting
>
> Commit 7831b3664109 ("ext4: add KUnit tests for fs/ext4/extents.c")
> updated both block-allocation paths in ext4_ext_split() to call
> ext4_ext_getblk() and noted node splitting in the commit message, but
> none of the test cases in fs/ext4/extents-test.c actually reached
> ext4_ext_split(). In test_insert_extent_grow_and_merge_up(), inserting a
> 5th extent into a full depth-0 root triggers ext4_ext_grow_indepth(),
> moving the 4 extents into an external leaf block with eh_max == 340 and
> returning from ext4_ext_create_new_leaf() without splitting.
>
> Add two KUnit test cases to exercise ext4_ext_split() across leaf and
> intermediate index levels:
>
> - test_insert_extent_leaf_split(): fills a depth-1 external leaf block
>   to capacity (340 extents), inserts in the middle to trigger a leaf
>   split that moves tail extents into a new leaf block, and then fills
>   the second leaf and appends past its end to exercise the m == 0
>   leaf-addition path.
> - test_insert_extent_index_split(): exercises depth 1 -> 2 tree growth
>   followed by leaf splitting via the repeat loop in
>   ext4_ext_create_new_leaf(), and then clamps the depth-1 index block's
>   eh_max to exercise simultaneous leaf and intermediate index block
>   splitting (k == 1) in ext4_ext_split().
>
> Fixes: 7831b3664109 ("ext4: add KUnit tests for fs/ext4/extents.c")
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>

> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 7056481862188..47ddeb94e2bf4 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c

[ ... ]

> @@ -2073,17 +2265,13 @@ static void test_swap_extents(struct kunit *test)
>  	KUNIT_ASSERT_EQ(test, ext_insert_one(inode1, 10, 10, 100, false, 0), 0);
>  	KUNIT_ASSERT_EQ(test, ext_insert_one(inode2, 10, 10, 200, false, 0), 0);
>  
> -	inode_lock(inode1);
> -	inode_lock(inode2);
> -	down_write(&EXT4_I(inode1)->i_data_sem);
> -	down_write(&EXT4_I(inode2)->i_data_sem);
> +	lock_two_nondirectories(inode1, inode2);
> +	ext4_double_down_write_data_sem(inode1, inode2);
>  
>  	replaced = ext4_swap_extents(NULL, inode1, inode2, 12, 12, 5, 1, &err);
>  
> -	up_write(&EXT4_I(inode2)->i_data_sem);
> -	up_write(&EXT4_I(inode1)->i_data_sem);
> -	inode_unlock(inode2);
> -	inode_unlock(inode1);
> +	ext4_double_up_write_data_sem(inode1, inode2);
> +	unlock_two_nondirectories(inode1, inode2);
>  
>  	KUNIT_EXPECT_EQ(test, err, 0);
>  	KUNIT_EXPECT_EQ(test, replaced, 5);

[ ... ]

> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 3329b7ad5dbd8..d1405e7639128 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -656,3 +656,8 @@ int ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
>  	unlock_two_nondirectories(orig_inode, donor_inode);
>  	return ret;
>  }
> +
> +#if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS)
> +EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_double_down_write_data_sem);
> +EXPORT_SYMBOL_FOR_EXT4_TEST(ext4_double_up_write_data_sem);
> +#endif

[Severity: Low]
This isn't a bug, but the commit message does not mention updating
test_swap_extents() to use lock_two_nondirectories() and
ext4_double_down_write_data_sem() (and exporting those helpers from
fs/ext4/move_extent.c) to avoid lockdep warnings on i_rwsem and
i_data_sem.

Also, the commit message and Fixes: trailer cite a stale pre-rebase sha
for commit "ext4: add KUnit tests for fs/ext4/extents.c" rather than the
parent commit in this branch.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112040638.1576836-1-tytso@mit.edu?part=3