lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <i6ymajhzfwy7wh2fe4imbgnnljmdt4mjriv4oi7gtp2kz2p6bd@d6xjxxroty6k>
Date: Thu, 15 Jan 2026 11:38:48 +0100
From: Jan Kara <jack@...e.cz>
To: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Cc: linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>, 
	Ritesh Harjani <ritesh.list@...il.com>, Zhang Yi <yi.zhang@...wei.com>, Jan Kara <jack@...e.cz>, 
	libaokun1@...wei.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/8] ext4: Add extent status cache support to kunit
 tests

On Wed 14-01-26 20:27:47, 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@...ux.ibm.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
>  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;
>  
> -- 
> 2.52.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ