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

[Zhang Yi <yizhang089@gmail.com>]

On 9/3/2026 9:02 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unbounded retry loop in `ext4_map_blocks` can lead to livelock.
> --
> 
> commit 084efaf4a2100728f08798dc518e2664e9b89954
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: recheck extent status tree before block allocation
> 
> This commit adds a sequence number recheck of the extent status tree
> before block allocation in ext4_map_blocks(). The goal is to ensure a
> racing truncate does not trim the extent between lookup and write lock
> acquisition, which could lead to quota leaks. If the sequence number
> has changed, the lookup is retried.
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index d703e95a6f62d..fb4834608fd89 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)) {
> 
> [ ... ]
> 
>> @@ -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;
> 
> [Severity: High]
> Could this goto statement in ext4_map_blocks() lead to an unbounded retry
> loop causing a livelock?
> 
> If an unprivileged user triggers a file writeback concurrently with a
> tight loop of operations modifying the extent tree (such as truncate
> or fallocate), the i_es_seq sequence number will continuously update.
> 
> Since there is no retry counter or a pessimistic locking fallback
> (like performing the lookup entirely under the write lock after some
> attempts), it appears the writeback thread could be starved indefinitely
> here in fs/ext4/inode.c, spinning at 100% CPU.
> 
> Does this need an upper bound on retries or a fallback mechanism to
> prevent system stalls?

The retry here mirrors the existing write_ops->iomap_valid() +
IOMAP_F_STALE mechanism in iomap, which also rechecks the sequence
number and retries without any upper bound, and has not shown livelock
problems in practice.

A retry only happens when i_es_seq has actually changed, which requires
another thread to hold i_data_sem in write mode, so each retry implies
real lock contention, and the writer yields on the rwsem slowpath rather
than spinning.

So I don't think an explicit retry bound is needed here.

Thanks,
Yi.

> 
>> +	}
>>   	retval = ext4_map_create_blocks(handle, inode, map, flags);
>>   	up_write((&EXT4_I(inode)->i_data_sem));
>