Re: [PATCH v2 02/13] ext4: subdivide EXT4_EXT_DATA_VALID1

[Ojaswin Mujoo]

On Fri, Nov 21, 2025 at 02:08:00PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> When splitting an extent, if the EXT4_GET_BLOCKS_CONVERT flag is set and
> it is necessary to split the target extent in the middle,
> ext4_split_extent() first handles splitting the latter half of the
> extent and passes the EXT4_EXT_DATA_VALID1 flag. This flag implies that
> all blocks before the split point contain valid data; however, this
> assumption is incorrect.
> 
> Therefore, subdivid EXT4_EXT_DATA_VALID1 into
> EXT4_EXT_DATA_ENTIRE_VALID1 and EXT4_EXT_DATA_PARTIAL_VALID1, which
> indicate that the first half of the extent is either entirely valid or
> only partially valid, respectively. These two flags cannot be set
> simultaneously.
> 
> This patch does not use EXT4_EXT_DATA_PARTIAL_VALID1, it only replaces
> EXT4_EXT_DATA_VALID1 with EXT4_EXT_DATA_ENTIRE_VALID1 at the location
> where it is set, no logical changes.
> 
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

Looks good, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

> ---
>  fs/ext4/extents.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 91682966597d..f7aa497e5d6c 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -43,8 +43,13 @@
>  #define EXT4_EXT_MARK_UNWRIT1	0x2  /* mark first half unwritten */
>  #define EXT4_EXT_MARK_UNWRIT2	0x4  /* mark second half unwritten */
>  
> -#define EXT4_EXT_DATA_VALID1	0x8  /* first half contains valid data */
> -#define EXT4_EXT_DATA_VALID2	0x10 /* second half contains valid data */
> +/* first half contains valid data */
> +#define EXT4_EXT_DATA_ENTIRE_VALID1	0x8   /* has partially valid data */
> +#define EXT4_EXT_DATA_PARTIAL_VALID1	0x10  /* has entirely valid data */
> +#define EXT4_EXT_DATA_VALID1		(EXT4_EXT_DATA_ENTIRE_VALID1 | \
> +					 EXT4_EXT_DATA_PARTIAL_VALID1)
> +
> +#define EXT4_EXT_DATA_VALID2	0x20 /* second half contains valid data */
>  
>  static __le32 ext4_extent_block_csum(struct inode *inode,
>  				     struct ext4_extent_header *eh)
> @@ -3190,8 +3195,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>  	unsigned int ee_len, depth;
>  	int err = 0;
>  
> -	BUG_ON((split_flag & (EXT4_EXT_DATA_VALID1 | EXT4_EXT_DATA_VALID2)) ==
> -	       (EXT4_EXT_DATA_VALID1 | EXT4_EXT_DATA_VALID2));
> +	BUG_ON((split_flag & EXT4_EXT_DATA_VALID1) == EXT4_EXT_DATA_VALID1);
> +	BUG_ON((split_flag & EXT4_EXT_DATA_VALID1) &&
> +	       (split_flag & EXT4_EXT_DATA_VALID2));
>  
>  	ext_debug(inode, "logical block %llu\n", (unsigned long long)split);
>  
> @@ -3358,7 +3364,7 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle,
>  			split_flag1 |= EXT4_EXT_MARK_UNWRIT1 |
>  				       EXT4_EXT_MARK_UNWRIT2;
>  		if (split_flag & EXT4_EXT_DATA_VALID2)
> -			split_flag1 |= EXT4_EXT_DATA_VALID1;
> +			split_flag1 |= EXT4_EXT_DATA_ENTIRE_VALID1;
>  		path = ext4_split_extent_at(handle, inode, path,
>  				map->m_lblk + map->m_len, split_flag1, flags1);
>  		if (IS_ERR(path))
> @@ -3717,7 +3723,7 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle,
>  
>  	/* Convert to unwritten */
>  	if (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN) {
> -		split_flag |= EXT4_EXT_DATA_VALID1;
> +		split_flag |= EXT4_EXT_DATA_ENTIRE_VALID1;
>  	/* Convert to initialized */
>  	} else if (flags & EXT4_GET_BLOCKS_CONVERT) {
>  		split_flag |= ee_block + ee_len <= eof_block ?
> -- 
> 2.46.1
>