[PATCH v2 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out

[PATCH v2 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out

[Daejun Park via B4 Relay]

When ext4_ext_convert_to_initialized() converts part of an unwritten
extent, it may zero out the blocks around the range and convert them
too, and ext4_split_extent() does the same with a whole extent when a
split fails. Two things go wrong around that.

1/2: if the zeroout fails, the blocks on that side stay unwritten in the
extent tree, but the extent status tree caches them as written. Reads
return old disk contents, and writes go in place without converting the
extent, so they read back as zeroes once the entry is dropped.

2/2: when the zeroout succeeds, fast commit is told only about the range
that was written. A later in-place overwrite of a zeroed block is not
tracked either, and after a crash the fsynced data reads back as zeroes.

Tested in QEMU on ext4 dev 9091c97be340 plus this series:
 - the reproducer described in 2/2, with -o nodelalloc and with
   -o dioread_lock, also on a PROVE_LOCKING and DEBUG_ATOMIC_SLEEP kernel
   (no report), and the dm-error reproducer described in 1/2;
 - FALLOC_FL_WRITE_ZEROES on scsi_debug (lbpws=1 lbprz=1) over a whole
   unwritten extent, over one block in the middle of one, and over a
   hole, each followed by an in-place write, fsync and
   EXT4_IOC_SHUTDOWN: the written block survives replay, with and
   without the series;
 - xfstests ext4/044 ext4/045 generic/455 generic/456 generic/482 with
   -O fast_commit, with and without -o nodelalloc: the same results as
   without the series. generic/455 fails on both, at different marks.
   generic/482 fails now and then on both (base 3 of 53 runs, with the
   series 2 of 46), with e2fsck reporting i_blocks one block short after
   replay;
 - the ext4 KUnit tests, 72 of 72. They are the only ones to reach the
   zeroout after a failed split, with the zeroout stubbed and without
   fast commit, so the tracking there is covered by review only.

For stable: the fast commit part goes back to 5.10. 2/2 applies as is
from 7.3-rc1 on, and I will send backports for older trees.

---
Changes in v2:
- 2/2: track the range in ext4_issue_zeroout(), once the zeroout has
  succeeded, instead of in ext4_zeroout_es() and
  ext4_split_extent_zeroout() (Jan). ext4_issue_zeroout() and
  ext4_ext_zeroout() take the handle for that.
  ext4_alloc_file_blocks() zeroes out without a handle and passes NULL.
  The ext4_convert_unwritten_extents() call after it tracks the blocks
  it converts through ext4_map_blocks(), blocks that were already
  written keep their mapping, and the FALLOC_FL_WRITE_ZEROES test above
  keeps the block on the base kernel too, so I found no gap there.
- 1/2: add Jan's Reviewed-by.
- 2/2: Cc stable without "# 7.0.x"; the fix is in ext4_issue_zeroout()
  now, which older trees have as well.
- Link to v1: https://lore.kernel.org/r/20261007-ext4-fc-zeroout-v1-0-b86de6439431@samsung.com

---
Daejun Park (2):
      ext4: don't cache unzeroed blocks as written after a failed zeroout
      ext4: track zeroed out blocks of unwritten extents for fast commit

 fs/ext4/ext4.h         |  5 +++--
 fs/ext4/ext4_extents.h |  3 ++-
 fs/ext4/extents-test.c |  8 +++++---
 fs/ext4/extents.c      | 39 ++++++++++++++++++++++++++-------------
 fs/ext4/inode.c        | 25 +++++++++++++++++--------
 5 files changed, 53 insertions(+), 27 deletions(-)
---
base-commit: 9091c97be34083587a75db174aab51551d8e8543
change-id: 20261006-ext4-fc-zeroout-e429007c72a0

Best regards,
-- 
Daejun Park <daejun7.park@samsung.com>

[PATCH v2 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out

[Daejun Park]

When ext4_ext_convert_to_initialized() converts part of an unwritten
extent, it may zero out the blocks around the range and convert them
too, and ext4_split_extent() does the same with a whole extent when a
split fails. Two things go wrong around that.

1/2: if the zeroout fails, the blocks on that side stay unwritten in the
extent tree, but the extent status tree caches them as written. Reads
return old disk contents, and writes go in place without converting the
extent, so they read back as zeroes once the entry is dropped.

2/2: when the zeroout succeeds, fast commit is told only about the range
that was written. A later in-place overwrite of a zeroed block is not
tracked either, and after a crash the fsynced data reads back as zeroes.

Tested in QEMU on ext4 dev 9091c97be340 plus this series:
 - the reproducer described in 2/2, with -o nodelalloc and with
   -o dioread_lock, also on a PROVE_LOCKING and DEBUG_ATOMIC_SLEEP kernel
   (no report), and the dm-error reproducer described in 1/2;
 - FALLOC_FL_WRITE_ZEROES on scsi_debug (lbpws=1 lbprz=1) over a whole
   unwritten extent, over one block in the middle of one, and over a
   hole, each followed by an in-place write, fsync and
   EXT4_IOC_SHUTDOWN: the written block survives replay, with and
   without the series;
 - xfstests ext4/044 ext4/045 generic/455 generic/456 generic/482 with
   -O fast_commit, with and without -o nodelalloc: the same results as
   without the series. generic/455 fails on both, at different marks.
   generic/482 fails now and then on both (base 3 of 53 runs, with the
   series 2 of 46), with e2fsck reporting i_blocks one block short after
   replay;
 - the ext4 KUnit tests, 72 of 72. They are the only ones to reach the
   zeroout after a failed split, with the zeroout stubbed and without
   fast commit, so the tracking there is covered by review only.

For stable: the fast commit part goes back to 5.10. 2/2 applies as is
from 7.3-rc1 on, and I will send backports for older trees.

---
Changes in v2:
- 2/2: track the range in ext4_issue_zeroout(), once the zeroout has
  succeeded, instead of in ext4_zeroout_es() and
  ext4_split_extent_zeroout() (Jan). ext4_issue_zeroout() and
  ext4_ext_zeroout() take the handle for that.
  ext4_alloc_file_blocks() zeroes out without a handle and passes NULL.
  The ext4_convert_unwritten_extents() call after it tracks the blocks
  it converts through ext4_map_blocks(), blocks that were already
  written keep their mapping, and the FALLOC_FL_WRITE_ZEROES test above
  keeps the block on the base kernel too, so I found no gap there.
- 1/2: add Jan's Reviewed-by.
- 2/2: Cc stable without "# 7.0.x"; the fix is in ext4_issue_zeroout()
  now, which older trees have as well.
- Link to v1: https://lore.kernel.org/r/20261007-ext4-fc-zeroout-v1-0-b86de6439431@samsung.com

---
Daejun Park (2):
      ext4: don't cache unzeroed blocks as written after a failed zeroout
      ext4: track zeroed out blocks of unwritten extents for fast commit

 fs/ext4/ext4.h         |  5 +++--
 fs/ext4/ext4_extents.h |  3 ++-
 fs/ext4/extents-test.c |  8 +++++---
 fs/ext4/extents.c      | 39 ++++++++++++++++++++++++++-------------
 fs/ext4/inode.c        | 25 +++++++++++++++++--------
 5 files changed, 53 insertions(+), 27 deletions(-)
---
base-commit: 9091c97be34083587a75db174aab51551d8e8543
change-id: 20261006-ext4-fc-zeroout-e429007c72a0

Best regards,
-- 
Daejun Park <daejun7.park@samsung.com>

[PATCH v2 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout

[Daejun Park via B4 Relay]

From: Daejun Park <daejun7.park@samsung.com>

ext4_ext_convert_to_initialized() may zero out the blocks before and
after the range being written and convert them to written together with
it, instead of splitting them off the unwritten extent. If
ext4_ext_zeroout() fails for one side, it falls back to splitting the
extent, so the blocks on that side stay unwritten in the extent tree.
But zero_ex1 or zero_ex2 keeps its length, and ext4_zeroout_es() still
inserts those blocks into the extent status tree as written.

Until that entry goes away, a read of those blocks returns whatever was
on the disk before the extent was allocated. A write to them is mapped
from the cache and goes in place without converting the extent, so its
data would read back as zeroes once the entry is dropped. To reproduce
on 4 KiB blocks with -o nodelalloc: fallocate 32 KiB, map the last seven
blocks of that extent to dm-error, write the first block, restore the
mapping and read the second block. It returns the old disk contents
instead of zeroes.

Clear the length of an extent that could not be zeroed out, so that
ext4_zeroout_es() skips it, as the comment at the out label intends.

Fixes: 308c57ccf431 ("ext4: if zeroout fails fall back to splitting the extent node")
Cc: stable@vger.kernel.org
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/ext4/extents.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 836396ea79..d65e30f3c1 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3755,8 +3755,10 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 				ext4_ext_pblock(ex) + split_map.m_lblk +
 				split_map.m_len - ee_block);
 			err = ext4_ext_zeroout(inode, &zero_ex1);
-			if (err)
+			if (err) {
+				zero_ex1.ee_len = 0;
 				goto fallback;
+			}
 			split_map.m_len = *allocated;
 		}
 		if (split_map.m_lblk - ee_block + split_map.m_len <
@@ -3769,8 +3771,10 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 				ext4_ext_store_pblock(&zero_ex2,
 						      ext4_ext_pblock(ex));
 				err = ext4_ext_zeroout(inode, &zero_ex2);
-				if (err)
+				if (err) {
+					zero_ex2.ee_len = 0;
 					goto fallback;
+				}
 			}
 
 			split_map.m_len += split_map.m_lblk - ee_block;

-- 
2.43.0

[PATCH v2 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout

[Daejun Park]

ext4_ext_convert_to_initialized() may zero out the blocks before and
after the range being written and convert them to written together with
it, instead of splitting them off the unwritten extent. If
ext4_ext_zeroout() fails for one side, it falls back to splitting the
extent, so the blocks on that side stay unwritten in the extent tree.
But zero_ex1 or zero_ex2 keeps its length, and ext4_zeroout_es() still
inserts those blocks into the extent status tree as written.

Until that entry goes away, a read of those blocks returns whatever was
on the disk before the extent was allocated. A write to them is mapped
from the cache and goes in place without converting the extent, so its
data would read back as zeroes once the entry is dropped. To reproduce
on 4 KiB blocks with -o nodelalloc: fallocate 32 KiB, map the last seven
blocks of that extent to dm-error, write the first block, restore the
mapping and read the second block. It returns the old disk contents
instead of zeroes.

Clear the length of an extent that could not be zeroed out, so that
ext4_zeroout_es() skips it, as the comment at the out label intends.

Fixes: 308c57ccf431 ("ext4: if zeroout fails fall back to splitting the extent node")
Cc: stable@vger.kernel.org
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/ext4/extents.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 836396ea79..d65e30f3c1 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3755,8 +3755,10 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 				ext4_ext_pblock(ex) + split_map.m_lblk +
 				split_map.m_len - ee_block);
 			err = ext4_ext_zeroout(inode, &zero_ex1);
-			if (err)
+			if (err) {
+				zero_ex1.ee_len = 0;
 				goto fallback;
+			}
 			split_map.m_len = *allocated;
 		}
 		if (split_map.m_lblk - ee_block + split_map.m_len <
@@ -3769,8 +3771,10 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 				ext4_ext_store_pblock(&zero_ex2,
 						      ext4_ext_pblock(ex));
 				err = ext4_ext_zeroout(inode, &zero_ex2);
-				if (err)
+				if (err) {
+					zero_ex2.ee_len = 0;
 					goto fallback;
+				}
 			}
 
 			split_map.m_len += split_map.m_lblk - ee_block;

-- 
2.43.0

[PATCH v2 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[Daejun Park via B4 Relay]

From: Daejun Park <daejun7.park@samsung.com>

When ext4_ext_convert_to_initialized() converts part of an unwritten
extent, it may zero out the blocks before and after that range (a side
only if it fits together with the range in s_extent_max_zeroout_kb,
32 KiB by default, and the extent ends within i_disksize or the write)
and convert them to written together with it, instead of splitting them
off. ext4_split_extent() zeroes out the rest of the extent and converts
all of it when a split fails with -ENOSPC, -EDQUOT or -ENOMEM. In both
cases ext4_map_blocks() reports only the mapped range to fast commit, so
the other converted blocks are not tracked.

A later write to one of those blocks overwrites a written block in
place. That changes no mapping and is not tracked either, so the next
fsync is a fast commit that logs the inode but no range for the block.
After a crash, replay starts from the last full commit, where the block
is still part of an unwritten extent, and the fsynced data reads back
as zeroes. Fast commit tracks one [min, max] range per inode, so this
shows only when no other block beyond the zeroed ones was tracked in
the same commit, which makes it easy to miss.

The first conversion runs when writes allocate through ext4_map_blocks()
with EXT4_GET_BLOCKS_CREATE, for example with -o nodelalloc,
-o dioread_lock or DAX. On a 4 KiB block file system made with
-O fast_commit and mounted with -o nodelalloc,commit=60:

  fallocate -l 32k f; sync      # blocks 0-7 unwritten
  write block 0; fsync f        # zeroes out 1-7, tracks only 0
  write block 2; fsync f        # in place, nothing tracked
  crash (kill the VM), mount    # fast commit replay
  block 2 reads back as zeroes

Track the range in ext4_issue_zeroout(), where the zeroes are written,
once the zeroout has succeeded. It takes the handle for that, and so
does ext4_ext_zeroout(). The EXT4_GET_BLOCKS_CONVERT_UNWRITTEN zeroout
in ext4_split_extent_zeroout() and the EXT4_GET_BLOCKS_ZERO one in
ext4_map_create_blocks() zero out the mapped range, which
ext4_map_blocks() tracks anyway.
ext4_alloc_file_blocks() zeroes out without a handle and passes NULL:
ext4_convert_unwritten_extents() right after it tracks the blocks it
converts through ext4_map_blocks(), and blocks that were already
written keep their mapping. The callers with a handle hold i_data_sem,
as fast commit tracking already does when ext4_ext_dirty() changes an
extent kept in the inode (through ext4_mark_inode_dirty()).

Fixes: aa75f4d3daae ("ext4: main fast-commit commit path")
Cc: stable@vger.kernel.org
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/ext4/ext4.h         |  5 +++--
 fs/ext4/ext4_extents.h |  3 ++-
 fs/ext4/extents-test.c |  8 +++++---
 fs/ext4/extents.c      | 31 ++++++++++++++++++++-----------
 fs/ext4/inode.c        | 25 +++++++++++++++++--------
 5 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 7fd078de26..f469b3548b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3209,8 +3209,9 @@ extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
 extern void ext4_da_release_space(struct inode *inode, int to_free);
 extern void ext4_da_update_reserve_space(struct inode *inode,
 					int used, int quota_claim);
-extern int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk,
-			      ext4_fsblk_t pblk, ext4_lblk_t len);
+extern int ext4_issue_zeroout(handle_t *handle, struct inode *inode,
+			      ext4_lblk_t lblk, ext4_fsblk_t pblk,
+			      ext4_lblk_t len);
 
 static inline bool is_special_ino(struct super_block *sb, unsigned long ino)
 {
diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index ebaf7cc424..f4218ff084 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -267,7 +267,8 @@ static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix,
 extern int __ext4_ext_dirty(const char *where, unsigned int line,
 			    handle_t *handle, struct inode *inode,
 			    struct ext4_ext_path *path);
-extern int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex);
+extern int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
+			    struct ext4_extent *ex);
 #if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS)
 extern int ext4_ext_space_root_idx_test(struct inode *inode, int check);
 extern struct ext4_ext_path *ext4_split_convert_extents_test(
diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index bd7795a826..4e205de575 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -180,7 +180,8 @@ ext4_ext_insert_extent_stub(handle_t *handle, struct inode *inode,
 /*
  * We will zeroout the equivalent range in the data area
  */
-static int ext4_ext_zeroout_stub(struct inode *inode, struct ext4_extent *ex)
+static int ext4_ext_zeroout_stub(handle_t *handle, struct inode *inode,
+				 struct ext4_extent *ex)
 {
 	ext4_lblk_t ee_block, off_blk;
 	loff_t ee_len;
@@ -203,8 +204,9 @@ static int ext4_ext_zeroout_stub(struct inode *inode, struct ext4_extent *ex)
 	return 0;
 }
 
-static int ext4_issue_zeroout_stub(struct inode *inode, ext4_lblk_t lblk,
-				   ext4_fsblk_t pblk, ext4_lblk_t len)
+static int ext4_issue_zeroout_stub(handle_t *handle, struct inode *inode,
+				   ext4_lblk_t lblk, ext4_fsblk_t pblk,
+				   ext4_lblk_t len)
 {
 	ext4_lblk_t off_blk;
 	loff_t off_bytes;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d65e30f3c1..aa55e7cf26 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3167,17 +3167,18 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
 }
 
 /* FIXME!! we need to try to merge to left or right after zero-out  */
-int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
+int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
+		     struct ext4_extent *ex)
 {
 	ext4_fsblk_t ee_pblock;
 	unsigned int ee_len;
 
-	KUNIT_STATIC_STUB_REDIRECT(ext4_ext_zeroout, inode, ex);
+	KUNIT_STATIC_STUB_REDIRECT(ext4_ext_zeroout, handle, inode, ex);
 
 	ee_len    = ext4_ext_get_actual_len(ex);
 	ee_pblock = ext4_ext_pblock(ex);
-	return ext4_issue_zeroout(inode, le32_to_cpu(ex->ee_block), ee_pblock,
-				  ee_len);
+	return ext4_issue_zeroout(handle, inode, le32_to_cpu(ex->ee_block),
+				  ee_pblock, ee_len);
 }
 
 /*
@@ -3347,7 +3348,8 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 			lblk = ee_block;
 			len = map->m_lblk - ee_block;
 			pblk = ext4_ext_pblock(ex);
-			err = ext4_issue_zeroout(inode, lblk, pblk, len);
+			err = ext4_issue_zeroout(handle, inode, lblk, pblk,
+						 len);
 			if (err)
 				/* ZEROOUT failed, just return original error */
 				return err;
@@ -3358,7 +3360,8 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 			lblk = map_end;
 			len = ex_end - map_end;
 			pblk = ext4_ext_pblock(ex) + (map_end - ee_block);
-			err = ext4_issue_zeroout(inode, lblk, pblk, len);
+			err = ext4_issue_zeroout(handle, inode, lblk, pblk,
+						 len);
 			if (err)
 				/* ZEROOUT failed, just return original error */
 				return err;
@@ -3377,7 +3380,7 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 		lblk = map->m_lblk;
 		len = map->m_len;
 		pblk = ext4_ext_pblock(ex) + (map->m_lblk - ee_block);
-		err = ext4_issue_zeroout(inode, lblk, pblk, len);
+		err = ext4_issue_zeroout(handle, inode, lblk, pblk, len);
 		if (err)
 			/* ZEROOUT failed, just return original error */
 			return err;
@@ -3754,7 +3757,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 			ext4_ext_store_pblock(&zero_ex1,
 				ext4_ext_pblock(ex) + split_map.m_lblk +
 				split_map.m_len - ee_block);
-			err = ext4_ext_zeroout(inode, &zero_ex1);
+			err = ext4_ext_zeroout(handle, inode, &zero_ex1);
 			if (err) {
 				zero_ex1.ee_len = 0;
 				goto fallback;
@@ -3770,7 +3773,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 							ee_block);
 				ext4_ext_store_pblock(&zero_ex2,
 						      ext4_ext_pblock(ex));
-				err = ext4_ext_zeroout(inode, &zero_ex2);
+				err = ext4_ext_zeroout(handle, inode, &zero_ex2);
 				if (err) {
 					zero_ex2.ee_len = 0;
 					goto fallback;
@@ -4693,8 +4696,14 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 			WARN_ON_ONCE(map.m_lblk + map.m_len >
 				EXT4_B_TO_LBLK(inode, new_size ?: old_size));
 
-			ret = ext4_issue_zeroout(inode, map.m_lblk, map.m_pblk,
-						 map.m_len);
+			/*
+			 * Zeroed out without a handle, see the comment
+			 * on EXT4_GET_BLOCKS_ZERO above. The conversion
+			 * below tracks the blocks it converts for fast
+			 * commit, and written blocks keep their mapping.
+			 */
+			ret = ext4_issue_zeroout(NULL, inode, map.m_lblk,
+						 map.m_pblk, map.m_len);
 			if (unlikely(ret))
 				break;
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 48ef9bb387..618ce271ba 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -406,23 +406,32 @@ static int __check_block_validity(struct inode *inode, const char *func,
 	return 0;
 }
 
-int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk, ext4_fsblk_t pblk,
-		       ext4_lblk_t len)
+/*
+ * The zeroes written here are file data. With a @handle the range is tracked
+ * for fast commit, because some callers zero out blocks outside the range that
+ * ext4_map_blocks() tracks and convert them to written. A caller without a
+ * handle must make sure that the conversion tracks the range.
+ */
+int ext4_issue_zeroout(handle_t *handle, struct inode *inode, ext4_lblk_t lblk,
+		       ext4_fsblk_t pblk, ext4_lblk_t len)
 {
 	int ret;
 
-	KUNIT_STATIC_STUB_REDIRECT(ext4_issue_zeroout, inode, lblk, pblk, len);
+	KUNIT_STATIC_STUB_REDIRECT(ext4_issue_zeroout, handle, inode, lblk, pblk,
+				   len);
 
 	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
-		return fscrypt_zeroout_range(inode,
+		ret = fscrypt_zeroout_range(inode,
 				(loff_t)lblk << inode->i_blkbits,
 				pblk << (inode->i_blkbits - SECTOR_SHIFT),
 				(u64)len << inode->i_blkbits);
-
-	ret = sb_issue_zeroout(inode->i_sb, pblk, len, GFP_NOFS);
+	else
+		ret = sb_issue_zeroout(inode->i_sb, pblk, len, GFP_NOFS);
 	if (ret > 0)
 		ret = 0;
 
+	if (!ret && handle)
+		ext4_fc_track_range(handle, inode, lblk, lblk + len - 1);
 	return ret;
 }
 
@@ -657,8 +666,8 @@ int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
 	 */
 	if (flags & EXT4_GET_BLOCKS_ZERO &&
 	    map->m_flags & EXT4_MAP_MAPPED && map->m_flags & EXT4_MAP_NEW) {
-		err = ext4_issue_zeroout(inode, map->m_lblk, map->m_pblk,
-					 map->m_len);
+		err = ext4_issue_zeroout(handle, inode, map->m_lblk,
+					 map->m_pblk, map->m_len);
 		if (err)
 			return err;
 	}

-- 
2.43.0

[PATCH v2 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[Daejun Park]

When ext4_ext_convert_to_initialized() converts part of an unwritten
extent, it may zero out the blocks before and after that range (a side
only if it fits together with the range in s_extent_max_zeroout_kb,
32 KiB by default, and the extent ends within i_disksize or the write)
and convert them to written together with it, instead of splitting them
off. ext4_split_extent() zeroes out the rest of the extent and converts
all of it when a split fails with -ENOSPC, -EDQUOT or -ENOMEM. In both
cases ext4_map_blocks() reports only the mapped range to fast commit, so
the other converted blocks are not tracked.

A later write to one of those blocks overwrites a written block in
place. That changes no mapping and is not tracked either, so the next
fsync is a fast commit that logs the inode but no range for the block.
After a crash, replay starts from the last full commit, where the block
is still part of an unwritten extent, and the fsynced data reads back
as zeroes. Fast commit tracks one [min, max] range per inode, so this
shows only when no other block beyond the zeroed ones was tracked in
the same commit, which makes it easy to miss.

The first conversion runs when writes allocate through ext4_map_blocks()
with EXT4_GET_BLOCKS_CREATE, for example with -o nodelalloc,
-o dioread_lock or DAX. On a 4 KiB block file system made with
-O fast_commit and mounted with -o nodelalloc,commit=60:

  fallocate -l 32k f; sync      # blocks 0-7 unwritten
  write block 0; fsync f        # zeroes out 1-7, tracks only 0
  write block 2; fsync f        # in place, nothing tracked
  crash (kill the VM), mount    # fast commit replay
  block 2 reads back as zeroes

Track the range in ext4_issue_zeroout(), where the zeroes are written,
once the zeroout has succeeded. It takes the handle for that, and so
does ext4_ext_zeroout(). The EXT4_GET_BLOCKS_CONVERT_UNWRITTEN zeroout
in ext4_split_extent_zeroout() and the EXT4_GET_BLOCKS_ZERO one in
ext4_map_create_blocks() zero out the mapped range, which
ext4_map_blocks() tracks anyway.
ext4_alloc_file_blocks() zeroes out without a handle and passes NULL:
ext4_convert_unwritten_extents() right after it tracks the blocks it
converts through ext4_map_blocks(), and blocks that were already
written keep their mapping. The callers with a handle hold i_data_sem,
as fast commit tracking already does when ext4_ext_dirty() changes an
extent kept in the inode (through ext4_mark_inode_dirty()).

Fixes: aa75f4d3daae ("ext4: main fast-commit commit path")
Cc: stable@vger.kernel.org
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/ext4/ext4.h         |  5 +++--
 fs/ext4/ext4_extents.h |  3 ++-
 fs/ext4/extents-test.c |  8 +++++---
 fs/ext4/extents.c      | 31 ++++++++++++++++++++-----------
 fs/ext4/inode.c        | 25 +++++++++++++++++--------
 5 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 7fd078de26..f469b3548b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3209,8 +3209,9 @@ extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
 extern void ext4_da_release_space(struct inode *inode, int to_free);
 extern void ext4_da_update_reserve_space(struct inode *inode,
 					int used, int quota_claim);
-extern int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk,
-			      ext4_fsblk_t pblk, ext4_lblk_t len);
+extern int ext4_issue_zeroout(handle_t *handle, struct inode *inode,
+			      ext4_lblk_t lblk, ext4_fsblk_t pblk,
+			      ext4_lblk_t len);
 
 static inline bool is_special_ino(struct super_block *sb, unsigned long ino)
 {
diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index ebaf7cc424..f4218ff084 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -267,7 +267,8 @@ static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix,
 extern int __ext4_ext_dirty(const char *where, unsigned int line,
 			    handle_t *handle, struct inode *inode,
 			    struct ext4_ext_path *path);
-extern int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex);
+extern int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
+			    struct ext4_extent *ex);
 #if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS)
 extern int ext4_ext_space_root_idx_test(struct inode *inode, int check);
 extern struct ext4_ext_path *ext4_split_convert_extents_test(
diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
index bd7795a826..4e205de575 100644
--- a/fs/ext4/extents-test.c
+++ b/fs/ext4/extents-test.c
@@ -180,7 +180,8 @@ ext4_ext_insert_extent_stub(handle_t *handle, struct inode *inode,
 /*
  * We will zeroout the equivalent range in the data area
  */
-static int ext4_ext_zeroout_stub(struct inode *inode, struct ext4_extent *ex)
+static int ext4_ext_zeroout_stub(handle_t *handle, struct inode *inode,
+				 struct ext4_extent *ex)
 {
 	ext4_lblk_t ee_block, off_blk;
 	loff_t ee_len;
@@ -203,8 +204,9 @@ static int ext4_ext_zeroout_stub(struct inode *inode, struct ext4_extent *ex)
 	return 0;
 }
 
-static int ext4_issue_zeroout_stub(struct inode *inode, ext4_lblk_t lblk,
-				   ext4_fsblk_t pblk, ext4_lblk_t len)
+static int ext4_issue_zeroout_stub(handle_t *handle, struct inode *inode,
+				   ext4_lblk_t lblk, ext4_fsblk_t pblk,
+				   ext4_lblk_t len)
 {
 	ext4_lblk_t off_blk;
 	loff_t off_bytes;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d65e30f3c1..aa55e7cf26 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3167,17 +3167,18 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
 }
 
 /* FIXME!! we need to try to merge to left or right after zero-out  */
-int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
+int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
+		     struct ext4_extent *ex)
 {
 	ext4_fsblk_t ee_pblock;
 	unsigned int ee_len;
 
-	KUNIT_STATIC_STUB_REDIRECT(ext4_ext_zeroout, inode, ex);
+	KUNIT_STATIC_STUB_REDIRECT(ext4_ext_zeroout, handle, inode, ex);
 
 	ee_len    = ext4_ext_get_actual_len(ex);
 	ee_pblock = ext4_ext_pblock(ex);
-	return ext4_issue_zeroout(inode, le32_to_cpu(ex->ee_block), ee_pblock,
-				  ee_len);
+	return ext4_issue_zeroout(handle, inode, le32_to_cpu(ex->ee_block),
+				  ee_pblock, ee_len);
 }
 
 /*
@@ -3347,7 +3348,8 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 			lblk = ee_block;
 			len = map->m_lblk - ee_block;
 			pblk = ext4_ext_pblock(ex);
-			err = ext4_issue_zeroout(inode, lblk, pblk, len);
+			err = ext4_issue_zeroout(handle, inode, lblk, pblk,
+						 len);
 			if (err)
 				/* ZEROOUT failed, just return original error */
 				return err;
@@ -3358,7 +3360,8 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 			lblk = map_end;
 			len = ex_end - map_end;
 			pblk = ext4_ext_pblock(ex) + (map_end - ee_block);
-			err = ext4_issue_zeroout(inode, lblk, pblk, len);
+			err = ext4_issue_zeroout(handle, inode, lblk, pblk,
+						 len);
 			if (err)
 				/* ZEROOUT failed, just return original error */
 				return err;
@@ -3377,7 +3380,7 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 		lblk = map->m_lblk;
 		len = map->m_len;
 		pblk = ext4_ext_pblock(ex) + (map->m_lblk - ee_block);
-		err = ext4_issue_zeroout(inode, lblk, pblk, len);
+		err = ext4_issue_zeroout(handle, inode, lblk, pblk, len);
 		if (err)
 			/* ZEROOUT failed, just return original error */
 			return err;
@@ -3754,7 +3757,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 			ext4_ext_store_pblock(&zero_ex1,
 				ext4_ext_pblock(ex) + split_map.m_lblk +
 				split_map.m_len - ee_block);
-			err = ext4_ext_zeroout(inode, &zero_ex1);
+			err = ext4_ext_zeroout(handle, inode, &zero_ex1);
 			if (err) {
 				zero_ex1.ee_len = 0;
 				goto fallback;
@@ -3770,7 +3773,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 							ee_block);
 				ext4_ext_store_pblock(&zero_ex2,
 						      ext4_ext_pblock(ex));
-				err = ext4_ext_zeroout(inode, &zero_ex2);
+				err = ext4_ext_zeroout(handle, inode, &zero_ex2);
 				if (err) {
 					zero_ex2.ee_len = 0;
 					goto fallback;
@@ -4693,8 +4696,14 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 			WARN_ON_ONCE(map.m_lblk + map.m_len >
 				EXT4_B_TO_LBLK(inode, new_size ?: old_size));
 
-			ret = ext4_issue_zeroout(inode, map.m_lblk, map.m_pblk,
-						 map.m_len);
+			/*
+			 * Zeroed out without a handle, see the comment
+			 * on EXT4_GET_BLOCKS_ZERO above. The conversion
+			 * below tracks the blocks it converts for fast
+			 * commit, and written blocks keep their mapping.
+			 */
+			ret = ext4_issue_zeroout(NULL, inode, map.m_lblk,
+						 map.m_pblk, map.m_len);
 			if (unlikely(ret))
 				break;
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 48ef9bb387..618ce271ba 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -406,23 +406,32 @@ static int __check_block_validity(struct inode *inode, const char *func,
 	return 0;
 }
 
-int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk, ext4_fsblk_t pblk,
-		       ext4_lblk_t len)
+/*
+ * The zeroes written here are file data. With a @handle the range is tracked
+ * for fast commit, because some callers zero out blocks outside the range that
+ * ext4_map_blocks() tracks and convert them to written. A caller without a
+ * handle must make sure that the conversion tracks the range.
+ */
+int ext4_issue_zeroout(handle_t *handle, struct inode *inode, ext4_lblk_t lblk,
+		       ext4_fsblk_t pblk, ext4_lblk_t len)
 {
 	int ret;
 
-	KUNIT_STATIC_STUB_REDIRECT(ext4_issue_zeroout, inode, lblk, pblk, len);
+	KUNIT_STATIC_STUB_REDIRECT(ext4_issue_zeroout, handle, inode, lblk, pblk,
+				   len);
 
 	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
-		return fscrypt_zeroout_range(inode,
+		ret = fscrypt_zeroout_range(inode,
 				(loff_t)lblk << inode->i_blkbits,
 				pblk << (inode->i_blkbits - SECTOR_SHIFT),
 				(u64)len << inode->i_blkbits);
-
-	ret = sb_issue_zeroout(inode->i_sb, pblk, len, GFP_NOFS);
+	else
+		ret = sb_issue_zeroout(inode->i_sb, pblk, len, GFP_NOFS);
 	if (ret > 0)
 		ret = 0;
 
+	if (!ret && handle)
+		ext4_fc_track_range(handle, inode, lblk, lblk + len - 1);
 	return ret;
 }
 
@@ -657,8 +666,8 @@ int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
 	 */
 	if (flags & EXT4_GET_BLOCKS_ZERO &&
 	    map->m_flags & EXT4_MAP_MAPPED && map->m_flags & EXT4_MAP_NEW) {
-		err = ext4_issue_zeroout(inode, map->m_lblk, map->m_pblk,
-					 map->m_len);
+		err = ext4_issue_zeroout(handle, inode, map->m_lblk,
+					 map->m_pblk, map->m_len);
 		if (err)
 			return err;
 	}

-- 
2.43.0

Re: [PATCH v2 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout

[sashiko-bot]

> From: Daejun Park <daejun7.park@samsung.com>
> 
> ext4_ext_convert_to_initialized() may zero out the blocks before and
> after the range being written and convert them to written together with
> it, instead of splitting them off the unwritten extent. If
> ext4_ext_zeroout() fails for one side, it falls back to splitting the
> extent, so the blocks on that side stay unwritten in the extent tree.
> But zero_ex1 or zero_ex2 keeps its length, and ext4_zeroout_es() still
> inserts those blocks into the extent status tree as written.
> 
> Until that entry goes away, a read of those blocks returns whatever was
> on the disk before the extent was allocated. A write to them is mapped
> from the cache and goes in place without converting the extent, so its
> data would read back as zeroes once the entry is dropped. To reproduce
> on 4 KiB blocks with -o nodelalloc: fallocate 32 KiB, map the last seven
> blocks of that extent to dm-error, write the first block, restore the
> mapping and read the second block. It returns the old disk contents
> instead of zeroes.
> 
> Clear the length of an extent that could not be zeroed out, so that
> ext4_zeroout_es() skips it, as the comment at the out label intends.
> 
> Fixes: 308c57ccf431 ("ext4: if zeroout fails fall back to splitting the extent node")
> Cc: stable@vger.kernel.org
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-ext4-fc-zeroout-v2-0-55cab1e24fa3@samsung.com?part=1

Re: [PATCH v2 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[sashiko-bot]

> From: Daejun Park <daejun7.park@samsung.com>
> 
> When ext4_ext_convert_to_initialized() converts part of an unwritten
> extent, it may zero out the blocks before and after that range (a side
> only if it fits together with the range in s_extent_max_zeroout_kb,
> 32 KiB by default, and the extent ends within i_disksize or the write)
> and convert them to written together with it, instead of splitting them
> off. ext4_split_extent() zeroes out the rest of the extent and converts
> all of it when a split fails with -ENOSPC, -EDQUOT or -ENOMEM. In both
> cases ext4_map_blocks() reports only the mapped range to fast commit, so
> the other converted blocks are not tracked.
> 
> A later write to one of those blocks overwrites a written block in
> place. That changes no mapping and is not tracked either, so the next
> fsync is a fast commit that logs the inode but no range for the block.
> [ ... ]
> 
> Fixes: aa75f4d3daae ("ext4: main fast-commit commit path")
> Cc: stable@vger.kernel.org
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-ext4-fc-zeroout-v2-0-55cab1e24fa3@samsung.com?part=2

Re: [PATCH v2 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout

[Ojaswin Mujoo]

On Thu, Oct 08, 2026 at 10:25:36AM +0900, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
> 
> ext4_ext_convert_to_initialized() may zero out the blocks before and
> after the range being written and convert them to written together with
> it, instead of splitting them off the unwritten extent. If
> ext4_ext_zeroout() fails for one side, it falls back to splitting the
> extent, so the blocks on that side stay unwritten in the extent tree.
> But zero_ex1 or zero_ex2 keeps its length, and ext4_zeroout_es() still
> inserts those blocks into the extent status tree as written.
> 
> Until that entry goes away, a read of those blocks returns whatever was
> on the disk before the extent was allocated. A write to them is mapped
> from the cache and goes in place without converting the extent, so its
> data would read back as zeroes once the entry is dropped. To reproduce
> on 4 KiB blocks with -o nodelalloc: fallocate 32 KiB, map the last seven
> blocks of that extent to dm-error, write the first block, restore the
> mapping and read the second block. It returns the old disk contents
> instead of zeroes.
> 
> Clear the length of an extent that could not be zeroed out, so that
> ext4_zeroout_es() skips it, as the comment at the out label intends.
> 
> Fixes: 308c57ccf431 ("ext4: if zeroout fails fall back to splitting the extent node")
> Cc: stable@vger.kernel.org
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Feel free to add:

Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Regards,
ojaswin

> ---
>  fs/ext4/extents.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 836396ea79..d65e30f3c1 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3755,8 +3755,10 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
>  				ext4_ext_pblock(ex) + split_map.m_lblk +
>  				split_map.m_len - ee_block);
>  			err = ext4_ext_zeroout(inode, &zero_ex1);
> -			if (err)
> +			if (err) {
> +				zero_ex1.ee_len = 0;
>  				goto fallback;
> +			}
>  			split_map.m_len = *allocated;
>  		}
>  		if (split_map.m_lblk - ee_block + split_map.m_len <
> @@ -3769,8 +3771,10 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
>  				ext4_ext_store_pblock(&zero_ex2,
>  						      ext4_ext_pblock(ex));
>  				err = ext4_ext_zeroout(inode, &zero_ex2);
> -				if (err)
> +				if (err) {
> +					zero_ex2.ee_len = 0;
>  					goto fallback;
> +				}
>  			}
>  
>  			split_map.m_len += split_map.m_lblk - ee_block;
> 
> -- 
> 2.43.0
> 
> 

Re: [PATCH v2 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[Jan Kara]

On Thu 08-10-26 10:25:37, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
> 
> When ext4_ext_convert_to_initialized() converts part of an unwritten
> extent, it may zero out the blocks before and after that range (a side
> only if it fits together with the range in s_extent_max_zeroout_kb,
> 32 KiB by default, and the extent ends within i_disksize or the write)
> and convert them to written together with it, instead of splitting them
> off. ext4_split_extent() zeroes out the rest of the extent and converts
> all of it when a split fails with -ENOSPC, -EDQUOT or -ENOMEM. In both
> cases ext4_map_blocks() reports only the mapped range to fast commit, so
> the other converted blocks are not tracked.
> 
> A later write to one of those blocks overwrites a written block in
> place. That changes no mapping and is not tracked either, so the next
> fsync is a fast commit that logs the inode but no range for the block.
> After a crash, replay starts from the last full commit, where the block
> is still part of an unwritten extent, and the fsynced data reads back
> as zeroes. Fast commit tracks one [min, max] range per inode, so this
> shows only when no other block beyond the zeroed ones was tracked in
> the same commit, which makes it easy to miss.
> 
> The first conversion runs when writes allocate through ext4_map_blocks()
> with EXT4_GET_BLOCKS_CREATE, for example with -o nodelalloc,
> -o dioread_lock or DAX. On a 4 KiB block file system made with
> -O fast_commit and mounted with -o nodelalloc,commit=60:
> 
>   fallocate -l 32k f; sync      # blocks 0-7 unwritten
>   write block 0; fsync f        # zeroes out 1-7, tracks only 0
>   write block 2; fsync f        # in place, nothing tracked
>   crash (kill the VM), mount    # fast commit replay
>   block 2 reads back as zeroes
> 
> Track the range in ext4_issue_zeroout(), where the zeroes are written,
> once the zeroout has succeeded. It takes the handle for that, and so
> does ext4_ext_zeroout(). The EXT4_GET_BLOCKS_CONVERT_UNWRITTEN zeroout
> in ext4_split_extent_zeroout() and the EXT4_GET_BLOCKS_ZERO one in
> ext4_map_create_blocks() zero out the mapped range, which
> ext4_map_blocks() tracks anyway.
> ext4_alloc_file_blocks() zeroes out without a handle and passes NULL:
> ext4_convert_unwritten_extents() right after it tracks the blocks it
> converts through ext4_map_blocks(), and blocks that were already
> written keep their mapping. The callers with a handle hold i_data_sem,
> as fast commit tracking already does when ext4_ext_dirty() changes an
> extent kept in the inode (through ext4_mark_inode_dirty()).
> 
> Fixes: aa75f4d3daae ("ext4: main fast-commit commit path")
> Cc: stable@vger.kernel.org
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Just one note: I agree with you ext4_alloc_file_blocks() is actually handled
correctly. But OTOH tracking a range twice isn't really an issue (only tiny
performance overhead compared to the extent tree operations and zeroing
itself). So I just wouldn't special-case ext4_alloc_file_blocks() and
always pass a proper handle to ext4_issue_zeroout(). That keeps the logic
slightly simpler: kernel zeroes out blocks => kernel tracks the range. But
I don't really insist, it's just a suggestion.

								Honza

> ---
>  fs/ext4/ext4.h         |  5 +++--
>  fs/ext4/ext4_extents.h |  3 ++-
>  fs/ext4/extents-test.c |  8 +++++---
>  fs/ext4/extents.c      | 31 ++++++++++++++++++++-----------
>  fs/ext4/inode.c        | 25 +++++++++++++++++--------
>  5 files changed, 47 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 7fd078de26..f469b3548b 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3209,8 +3209,9 @@ extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
>  extern void ext4_da_release_space(struct inode *inode, int to_free);
>  extern void ext4_da_update_reserve_space(struct inode *inode,
>  					int used, int quota_claim);
> -extern int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk,
> -			      ext4_fsblk_t pblk, ext4_lblk_t len);
> +extern int ext4_issue_zeroout(handle_t *handle, struct inode *inode,
> +			      ext4_lblk_t lblk, ext4_fsblk_t pblk,
> +			      ext4_lblk_t len);
>  
>  static inline bool is_special_ino(struct super_block *sb, unsigned long ino)
>  {
> diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
> index ebaf7cc424..f4218ff084 100644
> --- a/fs/ext4/ext4_extents.h
> +++ b/fs/ext4/ext4_extents.h
> @@ -267,7 +267,8 @@ static inline void ext4_idx_store_pblock(struct ext4_extent_idx *ix,
>  extern int __ext4_ext_dirty(const char *where, unsigned int line,
>  			    handle_t *handle, struct inode *inode,
>  			    struct ext4_ext_path *path);
> -extern int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex);
> +extern int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
> +			    struct ext4_extent *ex);
>  #if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS)
>  extern int ext4_ext_space_root_idx_test(struct inode *inode, int check);
>  extern struct ext4_ext_path *ext4_split_convert_extents_test(
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index bd7795a826..4e205de575 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -180,7 +180,8 @@ ext4_ext_insert_extent_stub(handle_t *handle, struct inode *inode,
>  /*
>   * We will zeroout the equivalent range in the data area
>   */
> -static int ext4_ext_zeroout_stub(struct inode *inode, struct ext4_extent *ex)
> +static int ext4_ext_zeroout_stub(handle_t *handle, struct inode *inode,
> +				 struct ext4_extent *ex)
>  {
>  	ext4_lblk_t ee_block, off_blk;
>  	loff_t ee_len;
> @@ -203,8 +204,9 @@ static int ext4_ext_zeroout_stub(struct inode *inode, struct ext4_extent *ex)
>  	return 0;
>  }
>  
> -static int ext4_issue_zeroout_stub(struct inode *inode, ext4_lblk_t lblk,
> -				   ext4_fsblk_t pblk, ext4_lblk_t len)
> +static int ext4_issue_zeroout_stub(handle_t *handle, struct inode *inode,
> +				   ext4_lblk_t lblk, ext4_fsblk_t pblk,
> +				   ext4_lblk_t len)
>  {
>  	ext4_lblk_t off_blk;
>  	loff_t off_bytes;
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index d65e30f3c1..aa55e7cf26 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -3167,17 +3167,18 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
>  }
>  
>  /* FIXME!! we need to try to merge to left or right after zero-out  */
> -int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
> +int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
> +		     struct ext4_extent *ex)
>  {
>  	ext4_fsblk_t ee_pblock;
>  	unsigned int ee_len;
>  
> -	KUNIT_STATIC_STUB_REDIRECT(ext4_ext_zeroout, inode, ex);
> +	KUNIT_STATIC_STUB_REDIRECT(ext4_ext_zeroout, handle, inode, ex);
>  
>  	ee_len    = ext4_ext_get_actual_len(ex);
>  	ee_pblock = ext4_ext_pblock(ex);
> -	return ext4_issue_zeroout(inode, le32_to_cpu(ex->ee_block), ee_pblock,
> -				  ee_len);
> +	return ext4_issue_zeroout(handle, inode, le32_to_cpu(ex->ee_block),
> +				  ee_pblock, ee_len);
>  }
>  
>  /*
> @@ -3347,7 +3348,8 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
>  			lblk = ee_block;
>  			len = map->m_lblk - ee_block;
>  			pblk = ext4_ext_pblock(ex);
> -			err = ext4_issue_zeroout(inode, lblk, pblk, len);
> +			err = ext4_issue_zeroout(handle, inode, lblk, pblk,
> +						 len);
>  			if (err)
>  				/* ZEROOUT failed, just return original error */
>  				return err;
> @@ -3358,7 +3360,8 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
>  			lblk = map_end;
>  			len = ex_end - map_end;
>  			pblk = ext4_ext_pblock(ex) + (map_end - ee_block);
> -			err = ext4_issue_zeroout(inode, lblk, pblk, len);
> +			err = ext4_issue_zeroout(handle, inode, lblk, pblk,
> +						 len);
>  			if (err)
>  				/* ZEROOUT failed, just return original error */
>  				return err;
> @@ -3377,7 +3380,7 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
>  		lblk = map->m_lblk;
>  		len = map->m_len;
>  		pblk = ext4_ext_pblock(ex) + (map->m_lblk - ee_block);
> -		err = ext4_issue_zeroout(inode, lblk, pblk, len);
> +		err = ext4_issue_zeroout(handle, inode, lblk, pblk, len);
>  		if (err)
>  			/* ZEROOUT failed, just return original error */
>  			return err;
> @@ -3754,7 +3757,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
>  			ext4_ext_store_pblock(&zero_ex1,
>  				ext4_ext_pblock(ex) + split_map.m_lblk +
>  				split_map.m_len - ee_block);
> -			err = ext4_ext_zeroout(inode, &zero_ex1);
> +			err = ext4_ext_zeroout(handle, inode, &zero_ex1);
>  			if (err) {
>  				zero_ex1.ee_len = 0;
>  				goto fallback;
> @@ -3770,7 +3773,7 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
>  							ee_block);
>  				ext4_ext_store_pblock(&zero_ex2,
>  						      ext4_ext_pblock(ex));
> -				err = ext4_ext_zeroout(inode, &zero_ex2);
> +				err = ext4_ext_zeroout(handle, inode, &zero_ex2);
>  				if (err) {
>  					zero_ex2.ee_len = 0;
>  					goto fallback;
> @@ -4693,8 +4696,14 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>  			WARN_ON_ONCE(map.m_lblk + map.m_len >
>  				EXT4_B_TO_LBLK(inode, new_size ?: old_size));
>  
> -			ret = ext4_issue_zeroout(inode, map.m_lblk, map.m_pblk,
> -						 map.m_len);
> +			/*
> +			 * Zeroed out without a handle, see the comment
> +			 * on EXT4_GET_BLOCKS_ZERO above. The conversion
> +			 * below tracks the blocks it converts for fast
> +			 * commit, and written blocks keep their mapping.
> +			 */
> +			ret = ext4_issue_zeroout(NULL, inode, map.m_lblk,
> +						 map.m_pblk, map.m_len);
>  			if (unlikely(ret))
>  				break;
>  
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 48ef9bb387..618ce271ba 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -406,23 +406,32 @@ static int __check_block_validity(struct inode *inode, const char *func,
>  	return 0;
>  }
>  
> -int ext4_issue_zeroout(struct inode *inode, ext4_lblk_t lblk, ext4_fsblk_t pblk,
> -		       ext4_lblk_t len)
> +/*
> + * The zeroes written here are file data. With a @handle the range is tracked
> + * for fast commit, because some callers zero out blocks outside the range that
> + * ext4_map_blocks() tracks and convert them to written. A caller without a
> + * handle must make sure that the conversion tracks the range.
> + */
> +int ext4_issue_zeroout(handle_t *handle, struct inode *inode, ext4_lblk_t lblk,
> +		       ext4_fsblk_t pblk, ext4_lblk_t len)
>  {
>  	int ret;
>  
> -	KUNIT_STATIC_STUB_REDIRECT(ext4_issue_zeroout, inode, lblk, pblk, len);
> +	KUNIT_STATIC_STUB_REDIRECT(ext4_issue_zeroout, handle, inode, lblk, pblk,
> +				   len);
>  
>  	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
> -		return fscrypt_zeroout_range(inode,
> +		ret = fscrypt_zeroout_range(inode,
>  				(loff_t)lblk << inode->i_blkbits,
>  				pblk << (inode->i_blkbits - SECTOR_SHIFT),
>  				(u64)len << inode->i_blkbits);
> -
> -	ret = sb_issue_zeroout(inode->i_sb, pblk, len, GFP_NOFS);
> +	else
> +		ret = sb_issue_zeroout(inode->i_sb, pblk, len, GFP_NOFS);
>  	if (ret > 0)
>  		ret = 0;
>  
> +	if (!ret && handle)
> +		ext4_fc_track_range(handle, inode, lblk, lblk + len - 1);
>  	return ret;
>  }
>  
> @@ -657,8 +666,8 @@ int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
>  	 */
>  	if (flags & EXT4_GET_BLOCKS_ZERO &&
>  	    map->m_flags & EXT4_MAP_MAPPED && map->m_flags & EXT4_MAP_NEW) {
> -		err = ext4_issue_zeroout(inode, map->m_lblk, map->m_pblk,
> -					 map->m_len);
> +		err = ext4_issue_zeroout(handle, inode, map->m_lblk,
> +					 map->m_pblk, map->m_len);
>  		if (err)
>  			return err;
>  	}
> 
> -- 
> 2.43.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

RE:(2) [PATCH v2 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[Daejun Park]

On Thu, Oct 08, 2026 at 11:55:02 +0200, Jan Kara wrote:
> Just one note: I agree with you ext4_alloc_file_blocks() is actually handled
> correctly. But OTOH tracking a range twice isn't really an issue (only tiny
> performance overhead compared to the extent tree operations and zeroing
> itself). So I just wouldn't special-case ext4_alloc_file_blocks() and
> always pass a proper handle to ext4_issue_zeroout(). That keeps the logic
> slightly simpler: kernel zeroes out blocks => kernel tracks the range. But
> I don't really insist, it's just a suggestion.

Thanks for the review. I wasn't sure I should do that in
ext4_alloc_file_blocks(): f4265b8d32c4 ("ext4: add
FALLOC_FL_WRITE_ZEROES support") issues the zeroout outside of the
running transaction on purpose, and passing a handle there would put it
under the conversion handle, so I left it as it is.

Re: (2) [PATCH v2 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit

[Jan Kara]

On Thu 08-10-26 19:26:15, Daejun Park wrote:
> On Thu, Oct 08, 2026 at 11:55:02 +0200, Jan Kara wrote:
> > Just one note: I agree with you ext4_alloc_file_blocks() is actually handled
> > correctly. But OTOH tracking a range twice isn't really an issue (only tiny
> > performance overhead compared to the extent tree operations and zeroing
> > itself). So I just wouldn't special-case ext4_alloc_file_blocks() and
> > always pass a proper handle to ext4_issue_zeroout(). That keeps the logic
> > slightly simpler: kernel zeroes out blocks => kernel tracks the range. But
> > I don't really insist, it's just a suggestion.
> 
> Thanks for the review. I wasn't sure I should do that in
> ext4_alloc_file_blocks(): f4265b8d32c4 ("ext4: add
> FALLOC_FL_WRITE_ZEROES support") issues the zeroout outside of the
> running transaction on purpose, and passing a handle there would put it
> under the conversion handle, so I left it as it is.

Ah, I see. OK, makes sense. Thanks for having a look!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR