Re: [PATCH v2 3/8] ext4: Add extent status cache support to kunit tests

[Zhang Yi <yizhang089@gmail.com>]

On 1/14/2026 10:57 PM, Ojaswin Mujoo wrote:
> Add support in Kunit tests to ensure that the extent status cache is
> also in sync after the extent split and conversion operations.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

It looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>   fs/ext4/extents-test.c   | 106 ++++++++++++++++++++++++---------------
>   fs/ext4/extents.c        |   2 -
>   fs/ext4/extents_status.c |   5 --
>   3 files changed, 65 insertions(+), 48 deletions(-)
> 
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index ebd7af64315a..86fcac66be6f 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -149,12 +149,6 @@ static void extents_kunit_exit(struct kunit *test)
>   	kfree(k_ctx.k_data);
>   }
>   
> -static void ext4_cache_extents_stub(struct inode *inode,
> -				    struct ext4_extent_header *eh)
> -{
> -	return;
> -}
> -
>   static int __ext4_ext_dirty_stub(const char *where, unsigned int line,
>   				 handle_t *handle, struct inode *inode,
>   				 struct ext4_ext_path *path)
> @@ -170,24 +164,6 @@ ext4_ext_insert_extent_stub(handle_t *handle, struct inode *inode,
>   	return ERR_PTR(-ENOSPC);
>   }
>   
> -static void ext4_es_remove_extent_stub(struct inode *inode, ext4_lblk_t lblk,
> -				       ext4_lblk_t len)
> -{
> -	return;
> -}
> -
> -void ext4_es_insert_extent_stub(struct inode *inode, ext4_lblk_t lblk,
> -				ext4_lblk_t len, ext4_fsblk_t pblk,
> -				unsigned int status, bool delalloc_reserve_used)
> -{
> -	return;
> -}
> -
> -static void ext4_zeroout_es_stub(struct inode *inode, struct ext4_extent *ex)
> -{
> -	return;
> -}
> -
>   /*
>    * We will zeroout the equivalent range in the data area
>    */
> @@ -244,13 +220,7 @@ static int extents_kunit_init(struct kunit *test)
>   	struct ext4_sb_info *sbi = NULL;
>   	struct kunit_ext_test_param *param =
>   		(struct kunit_ext_test_param *)(test->param_value);
> -
> -	/* setup the mock inode */
> -	k_ctx.k_ei = kzalloc(sizeof(struct ext4_inode_info), GFP_KERNEL);
> -	if (k_ctx.k_ei == NULL)
> -		return -ENOMEM;
> -	ei = k_ctx.k_ei;
> -	inode = &ei->vfs_inode;
> +	int err;
>   
>   	sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
>   	if (IS_ERR(sb))
> @@ -269,6 +239,24 @@ static int extents_kunit_init(struct kunit *test)
>   	if (!param || !param->disable_zeroout)
>   		sbi->s_extent_max_zeroout_kb = 32;
>   
> +	/* setup the mock inode */
> +	k_ctx.k_ei = kzalloc(sizeof(struct ext4_inode_info), GFP_KERNEL);
> +	if (k_ctx.k_ei == NULL)
> +		return -ENOMEM;
> +	ei = k_ctx.k_ei;
> +	inode = &ei->vfs_inode;
> +
> +	err = ext4_es_register_shrinker(sbi);
> +	if (err)
> +		return err;
> +
> +	ext4_es_init_tree(&ei->i_es_tree);
> +	rwlock_init(&ei->i_es_lock);
> +	INIT_LIST_HEAD(&ei->i_es_list);
> +	ei->i_es_all_nr = 0;
> +	ei->i_es_shk_nr = 0;
> +	ei->i_es_shrink_lblk = 0;
> +
>   	ei->i_disksize = (EX_DATA_LBLK + EX_DATA_LEN + 10)
>   			 << sb->s_blocksize_bits;
>   	ei->i_flags = 0;
> @@ -305,16 +293,15 @@ static int extents_kunit_init(struct kunit *test)
>   	if (!param || param->is_unwrit_at_start)
>   		ext4_ext_mark_unwritten(EXT_FIRST_EXTENT(eh));
>   
> +	ext4_es_insert_extent(inode, EX_DATA_LBLK, EX_DATA_LEN, EX_DATA_PBLK,
> +			      ext4_ext_is_unwritten(EXT_FIRST_EXTENT(eh)) ?
> +				      EXTENT_STATUS_UNWRITTEN :
> +				      EXTENT_STATUS_WRITTEN,
> +			      0);
> +
>   	/* Add stubs */
> -	kunit_activate_static_stub(test, ext4_cache_extents,
> -				   ext4_cache_extents_stub);
>   	kunit_activate_static_stub(test, __ext4_ext_dirty,
>   				   __ext4_ext_dirty_stub);
> -	kunit_activate_static_stub(test, ext4_es_remove_extent,
> -				   ext4_es_remove_extent_stub);
> -	kunit_activate_static_stub(test, ext4_es_insert_extent,
> -				   ext4_es_insert_extent_stub);
> -	kunit_activate_static_stub(test, ext4_zeroout_es, ext4_zeroout_es_stub);
>   	kunit_activate_static_stub(test, ext4_ext_zeroout, ext4_ext_zeroout_stub);
>   	kunit_activate_static_stub(test, ext4_issue_zeroout,
>   				   ext4_issue_zeroout_stub);
> @@ -379,11 +366,12 @@ static void test_split_convert(struct kunit *test)
>   		kunit_activate_static_stub(test, ext4_ext_insert_extent,
>   					   ext4_ext_insert_extent_stub);
>   
> -	path = ext4_find_extent(inode, EX_DATA_LBLK, NULL, 0);
> +	path = ext4_find_extent(inode, EX_DATA_LBLK, NULL, EXT4_EX_NOCACHE);
>   	ex = path->p_ext;
>   	KUNIT_EXPECT_EQ(test, 10, ex->ee_block);
>   	KUNIT_EXPECT_EQ(test, 3, ext4_ext_get_actual_len(ex));
> -	KUNIT_EXPECT_EQ(test, param->is_unwrit_at_start, ext4_ext_is_unwritten(ex));
> +	KUNIT_EXPECT_EQ(test, param->is_unwrit_at_start,
> +			ext4_ext_is_unwritten(ex));
>   	if (param->is_zeroout_test)
>   		KUNIT_EXPECT_EQ(test, 0,
>   				check_buffer(k_ctx.k_data, 'X',
> @@ -404,17 +392,47 @@ static void test_split_convert(struct kunit *test)
>   		KUNIT_FAIL(test, "param->type %d not support.", param->type);
>   	}
>   
> -	path = ext4_find_extent(inode, EX_DATA_LBLK, NULL, 0);
> +	path = ext4_find_extent(inode, EX_DATA_LBLK, NULL, EXT4_EX_NOCACHE);
>   	ex = path->p_ext;
>   
>   	for (int i = 0; i < param->nr_exp_ext; i++) {
>   		struct kunit_ext_state exp_ext = param->exp_ext_state[i];
> +		bool es_check_needed = param->type != TEST_SPLIT_CONVERT;
> +		struct extent_status es;
> +		int contains_ex, ex_end, es_end, es_pblk;
>   
>   		KUNIT_EXPECT_EQ(test, exp_ext.ex_lblk, ex->ee_block);
>   		KUNIT_EXPECT_EQ(test, exp_ext.ex_len,
>   				ext4_ext_get_actual_len(ex));
>   		KUNIT_EXPECT_EQ(test, exp_ext.is_unwrit,
>   				ext4_ext_is_unwritten(ex));
> +		/*
> +		 * Confirm extent cache is in sync. Note that es cache can be
> +		 * merged even when on-disk extents are not so take that into
> +		 * account.
> +		 *
> +		 * Also, ext4_split_convert_extents() forces EXT4_EX_NOCACHE hence
> +		 * es status are ignored for that case.
> +		 */
> +		if (es_check_needed) {
> +			ext4_es_lookup_extent(inode, ex->ee_block, NULL, &es,
> +					      NULL);
> +
> +			ex_end = exp_ext.ex_lblk + exp_ext.ex_len;
> +			es_end = es.es_lblk + es.es_len;
> +			contains_ex = es.es_lblk <= exp_ext.ex_lblk &&
> +				      es_end >= ex_end;
> +			es_pblk = ext4_es_pblock(&es) +
> +				  (exp_ext.ex_lblk - es.es_lblk);
> +
> +			KUNIT_EXPECT_EQ(test, contains_ex, 1);
> +			KUNIT_EXPECT_EQ(test, ext4_ext_pblock(ex), es_pblk);
> +			KUNIT_EXPECT_EQ(test, 1,
> +					(exp_ext.is_unwrit &&
> +					 ext4_es_is_unwritten(&es)) ||
> +						(!exp_ext.is_unwrit &&
> +						 ext4_es_is_written(&es)));
> +		}
>   
>   		/* Only printed on failure */
>   		kunit_log(KERN_INFO, test,
> @@ -424,6 +442,12 @@ static void test_split_convert(struct kunit *test)
>   			  "# [extent %d] got: lblk:%d len:%d unwrit:%d\n", i,
>   			  ex->ee_block, ext4_ext_get_actual_len(ex),
>   			  ext4_ext_is_unwritten(ex));
> +		if (es_check_needed)
> +			kunit_log(
> +				KERN_INFO, test,
> +				"# [extent %d] es: lblk:%d len:%d pblk:%lld type:0x%x\n",
> +				i, es.es_lblk, es.es_len, ext4_es_pblock(&es),
> +				ext4_es_type(&es));
>   		kunit_log(KERN_INFO, test, "------------------\n");
>   
>   		ex = ex + 1;
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 4cebd82ef3e4..a581e9278d48 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3149,8 +3149,6 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
>   	ext4_fsblk_t ee_pblock;
>   	unsigned int ee_len;
>   
> -	KUNIT_STATIC_STUB_REDIRECT(ext4_zeroout_es, inode, ex);
> -
>   	ee_block = le32_to_cpu(ex->ee_block);
>   	ee_len = ext4_ext_get_actual_len(ex);
>   	ee_pblock = ext4_ext_pblock(ex);
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 095ccb7ba4ba..a1538bac51c6 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -916,9 +916,6 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>   	struct pending_reservation *pr = NULL;
>   	bool revise_pending = false;
>   
> -	KUNIT_STATIC_STUB_REDIRECT(ext4_es_insert_extent, inode, lblk, len,
> -				   pblk, status, delalloc_reserve_used);
> -
>   	if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
>   		return;
>   
> @@ -1631,8 +1628,6 @@ void ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
>   	int reserved = 0;
>   	struct extent_status *es = NULL;
>   
> -	KUNIT_STATIC_STUB_REDIRECT(ext4_es_remove_extent, inode, lblk, len);
> -
>   	if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
>   		return;
>