Re: [PATCH v7 08/31] ext4: skip block allocation for holes in the data submission path

[Zhang Yi <yizhang089@gmail.com>]

On 10/9/2026 6:58 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] ext4: silent data loss in atomic extent conversion due to unhandled 0 return

Yeah, this is a pre-existing issue in
ext4_convert_unwritten_extents_atomic(), and Ojaswin had already pointed
this out, he will address this separately.

Thanks,
Yi.

> --
> 
> commit d986a085e4ae32e7076239e912abb3b6e2e97038
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: skip block allocation for holes in the data submission path
> 
> When ext4_map_blocks() is called from the data submission path and I/O
> end extent conversion path, it should not allocate blocks if the lookup
> returns a hole. This patch prepares for the buffered iomap writeback
> conversion by filtering out holes and bailing out with an error.
> 
>>      In the ioend extent conversion path, holes are also not expected because
>>      we should wait for folio writeback before punching hole. If one is
>>      encountered, it likely indicates a failure in the concurrency
>>      protection, so ext4_map_blocks() returns zero, and we keep warning and
>>      bail out with -EINVAL to surface the failure rather than continuing
>>      conversion on torn data. Atomic writes in
>>      ext4_convert_unwritten_extents_atomic() already bail out similarly.
> 
> [Severity: High]
> The commit message states that ext4_convert_unwritten_extents_atomic()
> "already bails out similarly" (presumably with an error code like -EINVAL)
> when ext4_map_blocks() returns 0 for a hole.
> 
> However, looking at ext4_convert_unwritten_extents_atomic() in
> fs/ext4/extents.c, it breaks the loop when ret is 0, and returns 0 (success):
> 
> fs/ext4/extents.c:ext4_convert_unwritten_extents_atomic() {
>      ...
> 	while (ret >= 0 && ret < max_blocks) {
> 		map.m_lblk += ret;
> 		map.m_len = (max_blocks -= ret);
> 		ret = ext4_map_blocks(handle, inode, &map, flags);
> 		...
> 		if (ret <= 0)
> 			break;
> 	}
>      ...
> 	if (ret <= 0 || ret2)
> 		ext4_warning(inode->i_sb, ...);
> 
> 	return ret > 0 ? ret2 : ret;
> }
> 
> If ret is 0, the expression ret > 0 ? ret2 : ret evaluates to 0.
> 
> Could this cause a regression where the unwritten extent conversion fails
> due to encountering a hole, but the direct I/O layer is told it succeeded,
> leaving unwritten extents on disk that will return zeroes on subsequent
> reads?
> 
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 6b0489cbb1319..778cf539f6df2 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
> 
> [ ... ]
> 
>> @@ -830,6 +830,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>>   				map->m_flags |= EXT4_MAP_MAPPED;
>>   			goto out;
>>   		}
>> +	} else if (retval == 0) {
>> +		/*
>> +		 * Do not allocate blocks for holes in the context of
>> +		 * data submission path.
>> +		 */
>> +		if (!map->m_flags && (flags & EXT4_GET_BLOCKS_IO_SUBMIT))
>> +			goto out;
>>   	}
>>   
>>   	if (!handle) {
>