[PATCH -next v5 19/32] ext4: tolerate unexpected holes in ext4_convert_unwritten_extents()

[Zhang Yi]

From: Zhang Yi <yi.zhang@xxxxxxxxxx>

Because the iomap infrastructure does not always create an ifs to
manage sub-folio state when folio size is larger than blocksize,
invalidating a partial dirty folio during punch hole may fail to
clear the dirty state of the affected range. As a result, writeback
of that folio may observe a hole. At writeback submit time,
ext4_map_blocks() already handles this case and will not allocate
blocks. However, when punch hole races with writeback, the
following scenario can cause I/O completion to encounter a hole.

  punch hole                        writeback
  ----------                        ---------
  ext4_punch_hole()
    ext4_truncate_page_cache_block_range()
      iomap_invalidate_folio() [partial folio]
        iomap_clear_range_dirty()
        -- no ifs, sub-block dirty bits NOT cleared
                                    ext4_iomap_writepages()
                                      iomap_writepages()
                                        ext4_iomap_writeback_submit()
                                          ext4_iomap_map_writeback_range()
                                            ext4_map_blocks(IO_SUBMIT)
                                              -> extent exists, not a hole
                                          submit_io()  -> bio in flight
    down_write(&i_data_sem)
    ext4_es_remove_extent()
    ext4_ext_remove_space()
      -> extent removed, hole inserted
    up_write(&i_data_sem)
                                    [bio completes]
                                    ext4_iomap_finish_ioend()
                                      ext4_convert_unwritten_extents()
                                        ext4_map_blocks(IO_CONVERT_EXT)
                                          -> returns 0 (hole found)

Therefore, in ext4_convert_unwritten_extents() we need to be
careful about the case where ext4_map_blocks() returns 0. Instead
of triggering a warning, we should ignore the hole and continue
with the subsequent conversion.

Link: https://lore.kernel.org/linux-ext4/a638a8fb-c184-4069-ae33-379ec12cd514@xxxxxxxxxxxxxxx/
Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
---
 fs/ext4/extents.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 0d62d9312284..5a06259a9b5d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5166,19 +5166,21 @@ int ext4_convert_unwritten_extents(handle_t *handle, struct inode *inode,
 		ret = ext4_map_blocks(handle, inode, &map,
 				      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.
-			 */
+		/*
+		 * A return value of zero means an unexpected hole was found.
+		 * This can happen when writeback races with a concurrent
+		 * punch hole in the iomap path. Because iomap may not create
+		 * ifs for folios larger than block size, the dirty bit can
+		 * be set again after punching. If writeback happens between
+		 * partial folio invalidation and extent removal, a hole is
+		 * observed at I/O completion.
+		 */
+		if (ret < 0)
 			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);
-		} else {
+		else if (ret > 0)
 			conv_blocks += map.m_len;
-		}
 
 		ret2 = ext4_mark_inode_dirty(handle, inode);
 		if (credits) {
-- 
2.52.0