[PATCH v6 18/31] ext4: drain writeback before removing extents on the iomap path

[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)
    [ transaction commits, freed blocks released to buddy ]
    [ allocator reuses freed blocks for another inode ]
                                    [bio completes]
                                    ext4_iomap_finish_ioend()
                                      ext4_convert_unwritten_extents()
                                        ext4_map_blocks(IO_CONVERT_EXT)
                                          -> returns 0 (hole found)
                                    [bio writes to reallocated blocks ]
                                    -> silent data corruption

If the bio completes after the freed blocks have been reallocated to
another inode, it silently overwrites them and causes data corruption.
Therefore, on the iomap path, drain writeback in ext4_punch_hole() after
partial zeroing and before starting the freeing transaction, so any bio
in flight on a partial folio completes against blocks still owned by
this inode before extent removal.  Once extent removal begins, the ES
hole insert and the existing IO_SUBMIT short-circuit prevent further bio
submission.

This is a workaround. The proper fix is for the iomap infrastructure to
always maintain an ifs when folio size is larger than blocksize, so that
partial invalidate reliably clears sub-block dirty bits and no draining
is needed.

Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
---
 fs/ext4/inode.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b876a8127d09..e91b4efce098 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5028,7 +5028,14 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
 	ret = ext4_zero_partial_blocks(inode, offset, length, &partial_zeroed);
 	if (ret)
 		return ret;
-	if (((file->f_flags & O_SYNC) || IS_SYNC(inode)) && partial_zeroed) {
+	/*
+	 * On the iomap path, partial invalidate of a folio without an ifs
+	 * leaves sub-block dirty bits uncleared.  Drain writeback before
+	 * removing extents so that any bio in flight on a partial folio
+	 * completes against blocks still owned by this inode.
+	 */
+	if (ext4_inode_buffered_iomap(inode) ||
+	    (((file->f_flags & O_SYNC) || IS_SYNC(inode)) && partial_zeroed)) {
 		ret = filemap_write_and_wait_range(inode->i_mapping, offset,
 						   end - 1);
 		if (ret)
-- 
2.52.0