Re: [PATCH v2] ext4: skip eof zeroing when the zero range covers the eof block

[Zhang Yi]

On 10/22/2026 8:26 PM, Baokun Li wrote:
> ext4_zero_range() drops the page cache of the range before allocating,
> to avoid exposing stale cached data.  However, the dropped page can be
> read back by the eof zeroing:
> 
>   fallocate(fd, FALLOC_FL_ZERO_RANGE, 0, 4096)
>     ext4_zero_range()
>       ext4_truncate_page_cache_block_range(0, 4096)
>         truncate_pagecache_range(0, 4095)
>           truncate_inode_pages_range()
>             delete_from_page_cache_batch()  # drops the page
>       ext4_alloc_file_blocks(..., 4096)
>         ext4_block_zero_eof(512, LLONG_MAX) # not block aligned
>           ext4_load_tail_bh()
>              __filemap_get_folio(FGP_WRITEBEGIN | FGP_ACCESSED)
>              # FGP_CREAT reads the dropped page back
>              # zeroes [512,1024), marks the buffer dirty
>         ext4_map_blocks(...)  # converts the extent to unwritten
>         ext4_update_inode_size(4096)
> 
> The bytes below the old EOF keep the stale disk content.  The page is
> left dirty, so read() returns old data while O_DIRECT reads zeroes, and
> a write into the same block is not converted and is lost until the page
> is dropped:
> 
>   page [0, 4096), 1k block size, file ends at 512
>    0            512          1024                      4096
>    |--- data ----|-- tail ---|---- zero range ----------|
>    | old data    |           |                          |
> 
> For example:
> 
>   $ xfs_io -f -c "pwrite -S 1 0 512" f
>   $ sync
>   $ fallocate -z -o 0 -l 4096 f
>   $ xfs_io -c "pread -v 0 16" f  # old data, not zeroes
> 
> FALLOC_FL_WRITE_ZEROES hits the same path.  ext4_issue_zeroout() zeroes
> the block on disk without updating the page cache, and the block stays
> written.  Therefore, writeback of the dirty page overwrites the zeroes
> with old data.
> 
> Since the whole eof block is converted to unwritten or zeroed out,
> zeroing it here is redundant.  Therefore, skip the zeroing when this
> allocation covers the eof block entirely.
> 
> Fixes: c4602a1d09ec ("ext4: move zero partial block range functions out of active handle")
> Fixes: f4265b8d32c4 ("ext4: add FALLOC_FL_WRITE_ZEROES support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Baokun Li <libaokun@xxxxxxxxxxxxxxxxx>

Hi Baokun,

Thank you for the fix, this is a nice catch. This makes sense to me.
Reviewed-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

One small point, the condition you added looks a bit confusing. I'm
wondering whether, in the future, it would be more straightforward to
extract ext4_block_zero_eof() from ext4_alloc_file_blocks() into
ext4_zero_range() and ext4_do_fallocate()?

Thanks,
Yi.

> ---
> Changes since v1:
>  * Also skip the eof zeroing for FALLOC_FL_WRITE_ZEROES, which
>    reads the dropped page back in the same way (Reported by Sashiko).
>  * Corrected the Fixes tags: the ZERO_RANGE regression comes from
>    c4602a1d09ec, and f4265b8d32c4 introduced the WRITE_ZEROES one.
>  * Fixed the call chains in the commit message.
> 
> v1: https://patch.msgid.link/20261022065342.3569693-1-libaokun@xxxxxxxxxxxxxxxxx
> 
> This hunk does not apply to v6.17 - v7.0, where the eof zeroing is
> done inside the allocation loop; those trees need a manual backport.
> 
>  fs/ext4/extents.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ad0d80e33f60..b05015b8b62c 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4639,8 +4639,15 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>  	credits = ext4_chunk_trans_blocks(inode, len_lblk);
>  	depth = ext_depth(inode);
>  
> -	/* Zero to the end of the block containing i_size */
> -	if (new_size > old_size) {
> +	/*
> +	 * Zero to the end of the block containing i_size.
> +	 * Loading the eof block back before it is converted to unwritten
> +	 * or zeroed out may leave the page cache inconsistent with the
> +	 * disk, and zeroing it is redundant, so skip it.
> +	 */
> +	if (new_size > old_size &&
> +	    !(in_range(old_size >> blkbits, map.m_lblk, map.m_len) &&
> +	      (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN || alloc_zero))) {
>  		ret = ext4_block_zero_eof(inode, old_size, LLONG_MAX);
>  		if (ret)
>  			return ret;