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-fsdevel,org.kernel.vger.linux-ext4,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/14/2026 5:33 PM, Zhang Yi wrote: > From: Zhang Yi <[email protected]> > > 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 <[email protected]> > --- > 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) {