| 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
| ||
|
Message-ID: <735xahhes7x62f2yuhpfgoojerfclo2kdwf5d7h2lgxtuq57ew@jt4ijbxfpocq>
Date: Thu, 15 Jan 2026 11:20:25 +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 1/8] ext4: kunit tests for extent splitting and
conversion
On Wed 14-01-26 20:27:45, Ojaswin Mujoo wrote:
> Add multiple KUnit tests to test various permutations of extent
> splitting and conversion.
>
> We test the following cases:
>
> 1. Split of unwritten extent into 2 parts and convert 1 part to written
> 2. Split of unwritten extent into 3 parts and convert 1 part to written
> 3. Split of written extent into 2 parts and convert 1 part to unwritten
> 4. Split of written extent into 3 parts and convert 1 part to unwritten
> 5. Zeroout fallback for all the above cases except 3-4 because zeroout
> is not supported for written to unwritten splits
>
> The main function we test here is ext4_split_convert_extents().
> Currently some of the tests are failing due to issues in implementation.
> All failures are mitigated at other layers in ext4 [1] but still point
> out the mismatch in expectation of what the caller wants vs what the
> function does.
>
> The aim is to eventually fix all the failures we see here. More detailed
> implementation notes can be found in the topmost commit in the test file.
>
> [1] for example, EXT4_GET_BLOCKS_CONVERT doesn't
> really convert the split extent to written, but rather the callers end up
> doing the conversion.
>
> Signed-off-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Overall this looks good to me so feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Couple nits below:
> +static void test_split_convert(struct kunit *test)
> +{
> + struct ext4_ext_path *path;
> + struct inode *inode = &k_ctx.k_ei->vfs_inode;
> + struct ext4_extent *ex;
> + struct ext4_map_blocks map;
> + const struct kunit_ext_test_param *param =
> + (const struct kunit_ext_test_param *)(test->param_value);
> + int blkbits = inode->i_sb->s_blocksize_bits;
> +
> + if (param->is_zeroout_test)
> + /*
> + * Force zeroout by making ext4_ext_insert_extent return ENOSPC
> + */
> + kunit_activate_static_stub(test, ext4_ext_insert_extent,
> + ext4_ext_insert_extent_stub);
> +
> + path = ext4_find_extent(inode, EX_DATA_LBLK, NULL, 0);
> + ex = path->p_ext;
> + KUNIT_EXPECT_EQ(test, 10, ex->ee_block);
> + KUNIT_EXPECT_EQ(test, 3, ext4_ext_get_actual_len(ex));
Shouldn't we use EX_DATA_LBLK and other constants instead for numbers?
...
> +static const struct kunit_ext_test_param test_split_convert_params[] = {
> + /* unwrit to writ splits */
> + { .desc = "split unwrit extent to 2 extents and convert 1st half writ",
> + .is_unwrit_at_start = 1,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT,
> + .split_map = { .m_lblk = 10, .m_len = 1 },
> + .nr_exp_ext = 2,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 1, .is_unwrit = 0 },
> + { .ex_lblk = 11, .ex_len = 2, .is_unwrit = 1 } },
> + .is_zeroout_test = 0 },
> + { .desc = "split unwrit extent to 2 extents and convert 2nd half writ",
> + .is_unwrit_at_start = 1,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT,
> + .split_map = { .m_lblk = 11, .m_len = 2 },
> + .nr_exp_ext = 2,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 1, .is_unwrit = 1 },
> + { .ex_lblk = 11, .ex_len = 2, .is_unwrit = 0 } },
> + .is_zeroout_test = 0 },
> + { .desc = "split unwrit extent to 3 extents and convert 2nd half to writ",
> + .is_unwrit_at_start = 1,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT,
> + .split_map = { .m_lblk = 11, .m_len = 1 },
> + .nr_exp_ext = 3,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 1, .is_unwrit = 1 },
> + { .ex_lblk = 11, .ex_len = 1, .is_unwrit = 0 },
> + { .ex_lblk = 12, .ex_len = 1, .is_unwrit = 1 } },
> + .is_zeroout_test = 0 },
> +
> + /* writ to unwrit splits */
> + { .desc = "split writ extent to 2 extents and convert 1st half unwrit",
> + .is_unwrit_at_start = 0,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT_UNWRITTEN,
> + .split_map = { .m_lblk = 10, .m_len = 1 },
> + .nr_exp_ext = 2,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 1, .is_unwrit = 1 },
> + { .ex_lblk = 11, .ex_len = 2, .is_unwrit = 0 } },
> + .is_zeroout_test = 0 },
> + { .desc = "split writ extent to 2 extents and convert 2nd half unwrit",
> + .is_unwrit_at_start = 0,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT_UNWRITTEN,
> + .split_map = { .m_lblk = 11, .m_len = 2 },
> + .nr_exp_ext = 2,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 1, .is_unwrit = 0 },
> + { .ex_lblk = 11, .ex_len = 2, .is_unwrit = 1 } },
> + .is_zeroout_test = 0 },
> + { .desc = "split writ extent to 3 extents and convert 2nd half to unwrit",
> + .is_unwrit_at_start = 0,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT_UNWRITTEN,
> + .split_map = { .m_lblk = 11, .m_len = 1 },
> + .nr_exp_ext = 3,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 1, .is_unwrit = 0 },
> + { .ex_lblk = 11, .ex_len = 1, .is_unwrit = 1 },
> + { .ex_lblk = 12, .ex_len = 1, .is_unwrit = 0 } },
> + .is_zeroout_test = 0 },
> +
> + /*
> + * ***** zeroout tests *****
> + */
> + /* unwrit to writ splits */
> + { .desc = "split unwrit extent to 2 extents and convert 1st half writ (zeroout)",
> + .is_unwrit_at_start = 1,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT,
> + .split_map = { .m_lblk = 10, .m_len = 1 },
> + .nr_exp_ext = 1,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 3, .is_unwrit = 0 } },
> + .is_zeroout_test = 1,
> + .nr_exp_data_segs = 2,
> + /* 1 block of data followed by 2 blocks of zeroes */
> + .exp_data_state = { { .exp_char = 'X', .off_blk = 0, .len_blk = 1 },
> + { .exp_char = 0, .off_blk = 1, .len_blk = 2 } } },
> + { .desc = "split unwrit extent to 2 extents and convert 2nd half writ (zeroout)",
> + .is_unwrit_at_start = 1,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT,
> + .split_map = { .m_lblk = 11, .m_len = 2 },
> + .nr_exp_ext = 1,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 3, .is_unwrit = 0 } },
> + .is_zeroout_test = 1,
> + .nr_exp_data_segs = 2,
> + /* 1 block of zeroes followed by 2 blocks of data */
> + .exp_data_state = { { .exp_char = 0, .off_blk = 0, .len_blk = 1 },
> + { .exp_char = 'X', .off_blk = 1, .len_blk = 2 } } },
> + { .desc = "split unwrit extent to 3 extents and convert 2nd half writ (zeroout)",
> + .is_unwrit_at_start = 1,
> + .split_flags = EXT4_GET_BLOCKS_CONVERT,
> + .split_map = { .m_lblk = 11, .m_len = 1 },
> + .nr_exp_ext = 1,
> + .exp_ext_state = { { .ex_lblk = 10, .ex_len = 3, .is_unwrit = 0 } },
> + .is_zeroout_test = 1,
> + .nr_exp_data_segs = 3,
> + /* [zeroes] [data] [zeroes] */
> + .exp_data_state = { { .exp_char = 0, .off_blk = 0, .len_blk = 1 },
> + { .exp_char = 'X', .off_blk = 1, .len_blk = 1 },
> + { .exp_char = 0, .off_blk = 2, .len_blk = 1 } } },
> +
> +};
Also in these definitions of split_map and exp_ext_state I think it would be
cleaner to use EX_DATA_LBLK and EX_DATA_LEN...
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists