Re: [PATCH -next v5 10/32] ext4: skip block allocation for holes in the data submission path
Zhang Yi <[email protected]>
| Newsgroups | org.kernel.vger.linux-ext4,dev.linux.lists.sashiko-reviews |
|---|---|
| Message-ID | <[email protected]> |
On 8/14/2026 6:15 PM, [email protected] wrote: > 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? > For atomic writes, if we hit an unexpected hole during conversion, there's no point in continuing the loop since the data is already torn. We should just return early. Thanks, Yi.