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

[Zhang Yi <yizhang089@gmail.com>]

On 9/28/2026 6:36 PM, Ojaswin Mujoo wrote:
> On Thu, Sep 03, 2026 at 08:35:21PM +0800, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> When ext4_map_blocks() is called from the data submission path and I/O
>> end extent conversion path (EXT4_GET_BLOCKS_IO_SUBMIT), it should not
>> allocate blocks if the lookup returns a hole.
>>
>> The writeback path can legitimately encounter dirty ranges that map to
>> holes. For example, when a folio straddles i_size and the tail beyond
>> i_size is dirtied via a mmap write. Allocating blocks for such ranges is
>> wrong because there is no data to write back, the dirty bits should
>> simply be discarded without submitting I/O. This prepares for the
>> buffered iomap writeback conversion, mirrors the existing buffer_head
>> writeback path, where mpage_add_bh_to_extent() skip unmapped buffers and
>> ext4_bio_write_folio() clears their dirty bits.
> 
> Looks good, checking the iomap vs bh head logic. If we consider
> a 1k bs FS with 4k page size, with hte following ops:
> 
>    ftruncate(fd, 0);
>    pwrite(fd, buf, 1024, 0);
>    map = mmap(NULL, 1024, PROT_WRITE, MAP_SHARED, fd, 0);
>    map[0] = 'a';  ---->  mkwrite allocates 1 block and dirties the whole
>                          page
>    ftruncate(fd, 10000);
> 
> Here both non-iomap path (ext4) and iomap path (xfs) will allocate a
> single block but both will mark the whole page range dirty in bh/ifs.
> 
> In both iomap and ext4, while writeback, we skip bhs which are holes,
> and we clear dirty on them as well.. The only difference is that in
> iomap xfs_map_blocks() shall detect a hole and inform about the hole but
> in ext4, we filter holes out before the map block call.

Yeah. In the writeback path, almost all of the work is done by a single
call to ext4_map_blocks() now. This makes the writeback code look very
simple and clear. :)

> 
> So with this change we will be closer to iomap's behavior. Looks good
> althought it's unfortunate that IO_SUBMIT has yet another implicit
> behavior added to it's scope :)
> 
> Feel free to add:
> 
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> 
>>
>> 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.
> 
> Ahh that's right we do bail out but we don't seem to be returning an
> error in the atomic write path. I think ideally that would be the right
> thing to do. I'll fix it, thanks!

Indeed, thanks for fixing this.

Yi.

> 
> Regards,
> ojaswin
> 
>>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> ---
>>   fs/ext4/inode.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 7a5c74af8ff3..2881596bff8b 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -825,6 +825,13 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
>>   			map->m_flags |= EXT4_MAP_MAPPED;
>>   			goto out_handle;
>>   		}
>> +	} 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_handle;
>>   	}
>>   
>>   	if (!handle) {
>> -- 
>> 2.52.0
>>