Re: [PATCH v6 05/31] ext4: recheck extent status tree before block allocation

[Ojaswin Mujoo]

On Thu, Sep 03, 2026 at 08:35:17PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> After acquiring i_data_sem in write mode, recheck that the mapping
> found via the extent status tree or disk query has not changed. A
> racing truncate may have trimmed the extent between the earlier lookup
> and the write lock acquisition, since writeback does not hold i_rwsem
> or the folio locks covering the full extent. This could cause
> ext4_map_create_blocks() to allocate blocks beyond the truncated range,
> potentially leading to quota leaks in the upcomming iomap buffered
> writeback path since the iomap writeback infrastructure caches extents
> beyond the folio range.
> 
> Therefore, if we find a valid extent and the sequence number has
> changed, retry the entire lookup to obtain the correct trimmed mapping.
> 
> Suggested-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
>  fs/ext4/inode.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 84991fe99071..b71b1d2588ae 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -734,6 +734,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>  	else
>  		ext4_check_map_extents_env(inode);
>  
> +create_retry:
>  	/* Lookup extent status tree firstly */
>  	if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
>  		if (ext4_es_is_written(&es) || ext4_es_is_unwritten(&es)) {
> @@ -784,6 +785,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>  	down_read(&EXT4_I(inode)->i_data_sem);
>  	retval = ext4_map_query_blocks(handle, inode, map, flags);
>  	up_read((&EXT4_I(inode)->i_data_sem));
> +	if (retval < 0)
> +		return retval;

Hey Zhang, I think there's a slight change in behavior that we return if
the query fails now. Earlier we would have still tried the create_blocks
call and, in case of transient errors like ENOMEM, might have suceeded.
I don't think it's that big of a deal though. 

Rest looks fine and I agree with your reply to Sashiko, I think we
can tweak those things if we ever encounter a livelock.

Feel free to add:

Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>

Also, if you don't mind can you add

Link: https://lore.kernel.org/linux-ext4/b0781809-4759-4e12-be17-71555b764f48@xxxxxxxxx/

(or the call trace) to the next version as it explains the issue pretty well.
Looking at these things later takes some time to recall everything and
such traces help a lot.

Regards,
ojaswin

>  
>  found:
>  	if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
> @@ -820,6 +823,19 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>  	 * with create == 1 flag.
>  	 */
>  	down_write(&EXT4_I(inode)->i_data_sem);
> +
> +	/*
> +	 * Check the validity of the mapping found via the extent status
> +	 * tree or the disk query. A racing truncate may have changed the
> +	 * extent, since writeback does not hold i_rwsem or the folio locks
> +	 * covering the full extent.
> +	 */
> +	if (map->m_seq != READ_ONCE(EXT4_I(inode)->i_es_seq)) {
> +		up_write(&EXT4_I(inode)->i_data_sem);
> +		map->m_flags = 0;
> +		map->m_len = orig_mlen;
> +		goto create_retry;
> +	}
>  	retval = ext4_map_create_blocks(handle, inode, map, flags);
>  	up_write((&EXT4_I(inode)->i_data_sem));
>  
> -- 
> 2.52.0
>