Re: [PATCH -next v5 10/32] ext4: skip block allocation for holes in the data submission path

[Zhang Yi]

On 8/14/2026 5:33 PM, 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 mirrors the existing
> buffer_head writeback path, where mpage_add_bh_to_extent() skips
> unmapped buffers and ext4_bio_write_folio() clears their dirty bits.
>
> 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. In this case, to avoid losing data beyond the hole, do not
> stop conversion, continue on the remaining ranges. This prepares for the
> buffered iomap writeback conversion.
>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> ---
>   fs/ext4/extents.c | 8 ++++++--
>   fs/ext4/inode.c   | 7 +++++++
>   2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 76038b6c3655..0d62d9312284 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5167,11 +5167,15 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
>   				      EXT4_GET_BLOCKS_IO_CONVERT_EXT |
>   				      EXT4_EX_NOCACHE);
>   		if (ret <= 0) {
> +			/*
> +			 * If the ret is zero, an unexpected hole may cause
> +			 * conversion to fail.  To avoid data loss during I/O
> +			 * end conversion, skip the hole and continue
> +			 * converting subsequent blocks.
> +			 */
>   			ext4_warning(inode->i_sb,
>   				     "inode #%llu: block %u: len %u: ext4_map_blocks returned %d",
>   				     inode->i_ino, map.m_lblk, map.m_len, ret);
> -			if (unlikely(ret == 0))
> -				ret = -EINVAL;

Hmm, we'd lose the error code here. Sashiko also mentioned in the
review of patch 19 that hitting a hole during conversion could
corrupt other files. That's a serious bug, so continuing the
conversion doesn't make much sense. I think we should drop this
change and just return the error early.

Yi.

>   		} else {
>   			conv_blocks += map.m_len;
>   		}
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5dcc3f7b2ffd..d8c3e5e13b8a 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -823,6 +823,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) {