Re: [PATCH v2 03/13] ext4: don't zero the entire extent if EXT4_EXT_DATA_PARTIAL_VALID1

[Ojaswin Mujoo]

On Fri, Nov 21, 2025 at 02:08:01PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> When allocating initialized blocks from a large unwritten extent, or
> when splitting an unwritten extent during end I/O and converting it to
> initialized, there is currently a potential issue of stale data if the
> extent needs to be split in the middle.
> 
>        0  A      B  N
>        [UUUUUUUUUUUU]    U: unwritten extent
>        [--DDDDDDDD--]    D: valid data
>           |<-  ->| ----> this range needs to be initialized
> 
> ext4_split_extent() first try to split this extent at B with
> EXT4_EXT_DATA_ENTIRE_VALID1 and EXT4_EXT_MAY_ZEROOUT flag set, but
> ext4_split_extent_at() failed to split this extent due to temporary lack
> of space. It zeroout B to N and mark the entire extent from 0 to N
> as written.
> 
>        0  A      B  N
>        [WWWWWWWWWWWW]    W: written extent
>        [SSDDDDDDDDZZ]    Z: zeroed, S: stale data
> 
> ext4_split_extent() then try to split this extent at A with
> EXT4_EXT_DATA_VALID2 flag set. This time, it split successfully and left
> a stale written extent from 0 to A.
> 
>        0  A      B   N
>        [WW|WWWWWWWWWW]
>        [SS|DDDDDDDDZZ]
> 
> Fix this by pass EXT4_EXT_DATA_PARTIAL_VALID1 to ext4_split_extent_at()
> when splitting at B, don't convert the entire extent to written and left
> it as unwritten after zeroing out B to N. The remaining work is just
> like the standard two-part split. ext4_split_extent() will pass the
> EXT4_EXT_DATA_VALID2 flag when it calls ext4_split_extent_at() for the
> second time, allowing it to properly handle the split. If the split is
> successful, it will keep extent from 0 to A as unwritten.
> 
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

Hi Yi,

This patch looks good to me. I'm just wondering since this is a stale
data exposure that might need a backport, should we add a Fixes: tag 
and also keep these fixes before the refactor in 1/13 so backport is
easier.

Other than that, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

Regards,
ojaswin

> ---
>  fs/ext4/extents.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index f7aa497e5d6c..cafe66cb562f 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3294,6 +3294,13 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>  		err = ext4_ext_zeroout(inode, &zero_ex);
>  		if (err)
>  			goto fix_extent_len;
> +		/*
> +		 * The first half contains partially valid data, the splitting
> +		 * of this extent has not been completed, fix extent length
> +		 * and ext4_split_extent() split will the first half again.
> +		 */
> +		if (split_flag & EXT4_EXT_DATA_PARTIAL_VALID1)
> +			goto fix_extent_len;
>  
>  		/* update the extent length and mark as initialized */
>  		ex->ee_len = cpu_to_le16(ee_len);
> @@ -3364,7 +3371,9 @@ 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_ENTIRE_VALID1;
> +			split_flag1 |= map->m_lblk > ee_block ?
> +				       EXT4_EXT_DATA_PARTIAL_VALID1 :
> +				       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))
> -- 
> 2.46.1
>