Re: [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: missing ES tree update in ext4_split_extent_zeroout() causes stale fast commit logs
--

commit 9e702f427b4057cfc7aebd5f1480625ae8d66b78
Author: Daejun Park <daejun7.park@samsung.com>

ext4: track zeroed out blocks of unwritten extents for fast commit

When ext4_ext_convert_to_initialized() converts part of an unwritten
extent, it may zero out the blocks before and after that range and convert
them to written together with it. This patch tracks these zeroed out blocks
for fast commit in ext4_zeroout_es() and ext4_split_extent_zeroout().

Fixes: aa75f4d3daae ("ext4: main fast-commit commit path")

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index d65e30f3c1a74..3b45b55931c2b 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3149,7 +3149,8 @@ void ext4_ext_release(struct super_block *sb)
>  #endif
>  }
>  
> -static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
> +static void ext4_zeroout_es(handle_t *handle, struct inode *inode,
> +			    struct ext4_extent *ex)
>  {
>  	ext4_lblk_t  ee_block;
>  	ext4_fsblk_t ee_pblock;
> @@ -3164,6 +3165,12 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
>  
>  	ext4_es_insert_extent(inode, ee_block, ee_len, ee_pblock,
>  			      EXTENT_STATUS_WRITTEN, false);
> +	/*
> +	 * The zeroed out blocks became written together with the mapped
> +	 * range, but ext4_map_blocks() reports only the mapped range to fast
> +	 * commit.
> +	 */
> +	ext4_fc_track_range(handle, inode, ee_block, ee_block + ee_len - 1);
>  }
>  
>  /* FIXME!! we need to try to merge to left or right after zero-out  */

[ ... ]

> @@ -3400,6 +3407,14 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
>  	if (err)
>  		return err;
>  
> +	/*
> +	 * The whole extent is written now, not only the range in @map that
> +	 * ext4_map_blocks() reports to fast commit.
> +	 */
> +	if (flags & EXT4_GET_BLOCKS_CONVERT)
> +		ext4_fc_track_range(handle, inode, ee_block,
> +				    ee_block + ee_len - 1);
> +
>  	return 0;
>  }

[Severity: High]
Does this code need to update the in-memory ES tree by calling
ext4_es_insert_extent() before tracking the range for fast commit?

Unlike ext4_zeroout_es(), ext4_split_extent_zeroout() does not update the ES
tree, so the outer zeroed blocks remain EXTENT_STATUS_UNWRITTEN in the cache.

When the fast commit thread runs ext4_fc_snapshot_inode_data(), it traverses
the tracked bounding box and queries the ES tree. Because the ES tree is stale,
it sees the outer blocks as unwritten and incorrectly logs them as
EXT4_FC_TAG_ADD_RANGE with unwritten = 1.

After a crash and replay, could this cause the extent to be incorrectly restored
as unwritten instead of initialized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-ext4-fc-zeroout-v1-0-b86de6439431@samsung.com?part=2