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

[sashiko-bot@kernel.org]

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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903123543.2302999-1-yi.zhang@huaweicloud.com?part=5