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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4_convert_unwritten_extents_atomic() silently aborts conversion if it encounters a hole, leading to data loss for the rest of the atomic write range.
--

--- Patch [10]: [PATCH -next v5 10/32] ext4: skip block allocation for holes in the data submission path ---
commit 741674f6128ebda7382e0529fd0b709e4ae876be
Author: Zhang Yi <[email protected]>

ext4: skip block allocation for holes in the data submission path

This commit updates ext4_map_blocks() to avoid allocating blocks for holes
in the data submission path, returning 0 instead. To prevent data loss
during extent conversion, ext4_convert_unwritten_extents() is modified
to skip over unexpected holes and continue converting subsequent blocks.

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 76038b6c36552..0d62d9312284e 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;
>  		} else {
>  			conv_blocks += map.m_len;
>  		}

[Severity: High]
Since ext4_map_blocks() will now return 0 for holes when
EXT4_GET_BLOCKS_IO_SUBMIT is set, should
ext4_convert_unwritten_extents_atomic() also be updated to handle holes?

In ext4_convert_unwritten_extents_atomic():

	int flags = EXT4_GET_BLOCKS_IO_CONVERT_EXT | EXT4_EX_NOCACHE;
...
	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;
	}

It looks like if ext4_map_blocks() returns 0 for a hole, the loop will
break early and the function will return 0. Will this leave the remaining
blocks in the atomic write range unconverted and cause data loss?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10