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

[Zhang Yi <yizhang089@gmail.com>]

On 9/24/2026 9:22 PM, Ojaswin Mujoo wrote:
> On Thu, Sep 03, 2026 at 08:35:17PM +0800, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> 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@suse.cz>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> ---
>>   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@linux.ibm.com>
> 
> Also, if you don't mind can you add
> 
> Link: https://lore.kernel.org/linux-ext4/b0781809-4759-4e12-be17-71555b764f48@gmail.com/
> 
> (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.
> 

Hi, Ojaswin!

Thank you for the review and suggestion, this makes sense to me. I will
add this link in the next iteration.

Thanks,
Yi.

> 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
>>