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

[Ojaswin Mujoo]

On Thu, Sep 03, 2026 at 08:35:21PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> 
> 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.

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

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

Regards,
ojaswin

> 
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
>  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
>