mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out
@ 2026-10-07  0:41 Daejun Park via B4 Relay
  2026-10-07  0:41 ` [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout Daejun Park via B4 Relay
  2026-10-07  0:41 ` [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit Daejun Park via B4 Relay
  0 siblings, 2 replies; 9+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-07  0:41 UTC (permalink / raw)
  To: Theodore Ts'o, linux-ext4
  Cc: Jan Kara, Andreas Dilger, Baokun Li, Ojaswin Mujoo,
	Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable, 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.

2/2 depends on 1/2. With 2/2 alone, the blocks that failed to zero out
are tracked as well, and fast commit replay converts them to written on
disk, so the old contents survive the crash: with the dm-error
reproducer described in 1/2, block 1 reads back as old data after
replay on a kernel with only 2/2, and as zeroes with both patches.

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;
 - 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);
 - the ext4 KUnit tests, 72 of 72, including the split cases that zero
   out. They are the only ones to reach the second hunk of 2/2, and they
   run without fast commit, so its tracking is not exercised.

For stable: 2/2 needs ext4_split_extent_zeroout(), which came in 7.0,
hence "# 7.0.x". Before that only its ext4_zeroout_es() part applies.

---
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/extents.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
---
base-commit: 9091c97be34083587a75db174aab51551d8e8543
change-id: 20261006-ext4-fc-zeroout-e429007c72a0

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout
  2026-10-07  0:41 [PATCH 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out Daejun Park via B4 Relay
@ 2026-10-07  0:41 ` Daejun Park via B4 Relay
  2026-10-07 12:51   ` Jan Kara
                     ` (2 more replies)
  2026-10-07  0:41 ` [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit Daejun Park via B4 Relay
  1 sibling, 3 replies; 9+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-07  0:41 UTC (permalink / raw)
  To: Theodore Ts'o, linux-ext4
  Cc: Jan Kara, Andreas Dilger, Baokun Li, Ojaswin Mujoo,
	Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable, Daejun Park

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
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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit
  2026-10-07  0:41 [PATCH 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out Daejun Park via B4 Relay
  2026-10-07  0:41 ` [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout Daejun Park via B4 Relay
@ 2026-10-07  0:41 ` Daejun Park via B4 Relay
  2026-10-07 13:41   ` Jan Kara
       [not found]   ` <CGME20261007134122epcas2p261c7ea484933b17c10e745ec844b0aff@epcms2p1>
  1 sibling, 2 replies; 9+ messages in thread
From: Daejun Park via B4 Relay @ 2026-10-07  0:41 UTC (permalink / raw)
  To: Theodore Ts'o, linux-ext4
  Cc: Jan Kara, Andreas Dilger, Baokun Li, Ojaswin Mujoo,
	Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable, Daejun Park

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 lies within i_size) and convert them
to written together with it, instead of splitting them off.
ext4_split_extent() zeroes out and converts the whole extent 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 zeroed out blocks in ext4_zeroout_es(), and the whole extent
in ext4_split_extent_zeroout() when it converts one to written. Both run
under 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 # 7.0.x
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/ext4/extents.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d65e30f3c1..3b45b55931 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3149,7 +3149,8 @@ void ext4_ext_release(struct super_block *sb)
 #endif
 }
 
-static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
+static void ext4_zeroout_es(handle_t *handle, struct inode *inode,
+			    struct ext4_extent *ex)
 {
 	ext4_lblk_t  ee_block;
 	ext4_fsblk_t ee_pblock;
@@ -3164,6 +3165,12 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
 
 	ext4_es_insert_extent(inode, ee_block, ee_len, ee_pblock,
 			      EXTENT_STATUS_WRITTEN, false);
+	/*
+	 * The zeroed out blocks became written together with the mapped
+	 * range, but ext4_map_blocks() reports only the mapped range to fast
+	 * commit.
+	 */
+	ext4_fc_track_range(handle, inode, ee_block, ee_block + ee_len - 1);
 }
 
 /* FIXME!! we need to try to merge to left or right after zero-out  */
@@ -3400,6 +3407,14 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
 	if (err)
 		return err;
 
+	/*
+	 * The whole extent is written now, not only the range in @map that
+	 * ext4_map_blocks() reports to fast commit.
+	 */
+	if (flags & EXT4_GET_BLOCKS_CONVERT)
+		ext4_fc_track_range(handle, inode, ee_block,
+				    ee_block + ee_len - 1);
+
 	return 0;
 }
 
@@ -3790,8 +3805,8 @@ ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
 		return path;
 out:
 	/* If we have gotten a failure, don't zero out status tree */
-	ext4_zeroout_es(inode, &zero_ex1);
-	ext4_zeroout_es(inode, &zero_ex2);
+	ext4_zeroout_es(handle, inode, &zero_ex1);
+	ext4_zeroout_es(handle, inode, &zero_ex2);
 	return path;
 
 errout:

-- 
2.43.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout
  2026-10-07  0:41 ` [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout Daejun Park via B4 Relay
@ 2026-10-07 12:51   ` Jan Kara
  2026-10-08  6:19   ` Ojaswin Mujoo
       [not found]   ` <CGME20261008062023epcas2p17ab2f934eeb6ba157103b01c9cb59f5b@epcms2p2>
  2 siblings, 0 replies; 9+ messages in thread
From: Jan Kara @ 2026-10-07 12:51 UTC (permalink / raw)
  To: daejun7.park
  Cc: Theodore Ts'o, linux-ext4, Jan Kara, Andreas Dilger,
	Baokun Li, Ojaswin Mujoo, Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable

On Wed 07-10-26 09:41:20, 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
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  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
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit
  2026-10-07  0:41 ` [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit Daejun Park via B4 Relay
@ 2026-10-07 13:41   ` Jan Kara
       [not found]   ` <CGME20261007134122epcas2p261c7ea484933b17c10e745ec844b0aff@epcms2p1>
  1 sibling, 0 replies; 9+ messages in thread
From: Jan Kara @ 2026-10-07 13:41 UTC (permalink / raw)
  To: daejun7.park
  Cc: Theodore Ts'o, linux-ext4, Jan Kara, Andreas Dilger,
	Baokun Li, Ojaswin Mujoo, Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable

On Wed 07-10-26 09:41:21, 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 lies within i_size) and convert them
> to written together with it, instead of splitting them off.
> ext4_split_extent() zeroes out and converts the whole extent 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 zeroed out blocks in ext4_zeroout_es(), and the whole extent
> in ext4_split_extent_zeroout() when it converts one to written. Both run
> under 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 # 7.0.x
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Thanks for catching this. Some comments below.

> -static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
> +static void ext4_zeroout_es(handle_t *handle, struct inode *inode,
> +			    struct ext4_extent *ex)
>  {
>  	ext4_lblk_t  ee_block;
>  	ext4_fsblk_t ee_pblock;
> @@ -3164,6 +3165,12 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
>  
>  	ext4_es_insert_extent(inode, ee_block, ee_len, ee_pblock,
>  			      EXTENT_STATUS_WRITTEN, false);
> +	/*
> +	 * The zeroed out blocks became written together with the mapped
> +	 * range, but ext4_map_blocks() reports only the mapped range to fast
> +	 * commit.
> +	 */
> +	ext4_fc_track_range(handle, inode, ee_block, ee_block + ee_len - 1);
>  }

It looks a bit odd to hide ext4_fc_track_range() inside a function for
extent status tree tracking. Also see below.

> @@ -3400,6 +3407,14 @@ static int ext4_split_extent_zeroout(handle_t *handle, struct inode *inode,
>  	if (err)
>  		return err;
>  
> +	/*
> +	 * The whole extent is written now, not only the range in @map that
> +	 * ext4_map_blocks() reports to fast commit.
> +	 */
> +	if (flags & EXT4_GET_BLOCKS_CONVERT)
> +		ext4_fc_track_range(handle, inode, ee_block,
> +				    ee_block + ee_len - 1);
> +
>  	return 0;
>  }

Also putting this into ext4_split_extent_zeroout() looks a bit too easy to
miss. In fact I think placing ext4_fc_track_range() into
ext4_issue_zeroout() would make sense because that is where the writing of
"data" really happens. This will fix the use in
ext4_split_extent_zeroout() as well as ext4_ext_convert_to_initialized().
And it will also fix the same class of problem which I think we have in
ext4_alloc_file_blocks()...

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE:(2) [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit
       [not found]   ` <CGME20261007134122epcas2p261c7ea484933b17c10e745ec844b0aff@epcms2p1>
@ 2026-10-08  1:24     ` Daejun Park
  0 siblings, 0 replies; 9+ messages in thread
From: Daejun Park @ 2026-10-08  1:24 UTC (permalink / raw)
  To: Jan Kara
  Cc: Theodore Ts'o, linux-ext4, Andreas Dilger, Baokun Li,
	Ojaswin Mujoo, Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable, Daejun Park

Hi Jan,

On Wed, Oct 07, 2026 at 15:41:17 +0200, Jan Kara wrote:
> Also putting this into ext4_split_extent_zeroout() looks a bit too easy to
> miss. In fact I think placing ext4_fc_track_range() into
> ext4_issue_zeroout() would make sense because that is where the writing of
> "data" really happens. This will fix the use in
> ext4_split_extent_zeroout() as well as ext4_ext_convert_to_initialized().
> And it will also fix the same class of problem which I think we have in
> ext4_alloc_file_blocks()...

Thanks, that makes sense. In v2, ext4_issue_zeroout() and
ext4_ext_zeroout() take the handle, and the range is tracked there once
the zeroout has succeeded.

ext4_alloc_file_blocks() zeroes out without a handle, so it passes NULL.
As far as I can tell there is no gap there:
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. I also tried FALLOC_FL_WRITE_ZEROES on scsi_debug
(lbpws=1 lbprz=1) over an 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 on the base kernel
as well, and the tracepoint shows the range tracked twice, at allocation
and at conversion. If you had a different path in mind, please let me
know.

I'll send v2 shortly.

Daejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout
  2026-10-07  0:41 ` [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout Daejun Park via B4 Relay
  2026-10-07 12:51   ` Jan Kara
@ 2026-10-08  6:19   ` Ojaswin Mujoo
       [not found]   ` <CGME20261008062023epcas2p17ab2f934eeb6ba157103b01c9cb59f5b@epcms2p2>
  2 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-10-08  6:19 UTC (permalink / raw)
  To: daejun7.park
  Cc: Theodore Ts'o, linux-ext4, Jan Kara, Andreas Dilger,
	Baokun Li, Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable

On Wed, Oct 07, 2026 at 09:41:20AM +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
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Hi Daejun,

Feel free to add:

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

btw, did you catch this with some fstests? If not I think it would be
good to add a test for this. 

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
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE:(2) [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout
       [not found]   ` <CGME20261008062023epcas2p17ab2f934eeb6ba157103b01c9cb59f5b@epcms2p2>
@ 2026-10-08  7:47     ` Daejun Park
  2026-10-08  9:29       ` (2) " Ojaswin Mujoo
  0 siblings, 1 reply; 9+ messages in thread
From: Daejun Park @ 2026-10-08  7:47 UTC (permalink / raw)
  To: Ojaswin Mujoo
  Cc: Theodore Ts'o, linux-ext4, Jan Kara, Andreas Dilger,
	Baokun Li, Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable, Daejun Park

Hi Ojaswin,

On Thu, Oct 08, 2026 at 11:49:48 +0530, Ojaswin Mujoo wrote:
> Feel free to add:
>
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
>
> btw, did you catch this with some fstests? If not I think it would be
> good to add a test for this.

Thanks for the review.

No, it wasn't fstests. I ran into it while working on 2/2: when adding
the fast commit tracking next to ext4_zeroout_es(), I checked what it
caches when the zeroout fails, and saw that zero_ex1/zero_ex2 keep their
length on that path. To confirm it, I put dm-error under the file system
and failed only the zeroout.

I turned that into an fstests case [2]. The existing dm-error tests
don't reach this path: they write with direct I/O, which leaves the
extent unwritten until the I/O completes, and so does a buffered write
with the default dioread_nolock.

v2 [1] keeps this patch unchanged, so I'll carry your tag if there is a
v3.

[1] https://lore.kernel.org/r/20261008-ext4-fc-zeroout-v2-0-55cab1e24fa3@samsung.com
[2] https://lore.kernel.org/r/20261008-ext4-zeroout-eio-test-v1-1-9bb66deaf646@samsung.com

Daejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: (2) [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout
  2026-10-08  7:47     ` Daejun Park
@ 2026-10-08  9:29       ` Ojaswin Mujoo
  0 siblings, 0 replies; 9+ messages in thread
From: Ojaswin Mujoo @ 2026-10-08  9:29 UTC (permalink / raw)
  To: Daejun Park
  Cc: Theodore Ts'o, linux-ext4, Jan Kara, Andreas Dilger,
	Baokun Li, Ritesh Harjani (IBM),
	Zhang Yi, Zhang Yi, Harshad Shirwadkar, Li Chen, linux-kernel,
	stable

On Thu, Oct 08, 2026 at 04:47:14PM +0900, Daejun Park wrote:
> Hi Ojaswin,
> 
> On Thu, Oct 08, 2026 at 11:49:48 +0530, Ojaswin Mujoo wrote:
> > Feel free to add:
> >
> > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> >
> > btw, did you catch this with some fstests? If not I think it would be
> > good to add a test for this.
> 
> Thanks for the review.
> 
> No, it wasn't fstests. I ran into it while working on 2/2: when adding
> the fast commit tracking next to ext4_zeroout_es(), I checked what it
> caches when the zeroout fails, and saw that zero_ex1/zero_ex2 keep their
> length on that path. To confirm it, I put dm-error under the file system
> and failed only the zeroout.

Yeah, its a bit tricky to hit this in kunit test cause it needs one
zeroout to pass and one to fail, so good to have a test for this case.
Thanks for sending that, I'll take a look!
> 
> I turned that into an fstests case [2]. The existing dm-error tests
> don't reach this path: they write with direct I/O, which leaves the
> extent unwritten until the I/O completes, and so does a buffered write
> with the default dioread_nolock.
> 
> v2 [1] keeps this patch unchanged, so I'll carry your tag if there is a
> v3.

Ohh, i missed the v2, I'll send it there as well no worries.
> 
> [1] https://lore.kernel.org/r/20261008-ext4-fc-zeroout-v2-0-55cab1e24fa3@samsung.com
> [2] https://lore.kernel.org/r/20261008-ext4-zeroout-eio-test-v1-1-9bb66deaf646@samsung.com
> 
> Daejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-08  9:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07  0:41 [PATCH 0/2] ext4: fix fast commit and extent status after unwritten extent zero-out Daejun Park via B4 Relay
2026-10-07  0:41 ` [PATCH 1/2] ext4: don't cache unzeroed blocks as written after a failed zeroout Daejun Park via B4 Relay
2026-10-07 12:51   ` Jan Kara
2026-10-08  6:19   ` Ojaswin Mujoo
     [not found]   ` <CGME20261008062023epcas2p17ab2f934eeb6ba157103b01c9cb59f5b@epcms2p2>
2026-10-08  7:47     ` Daejun Park
2026-10-08  9:29       ` (2) " Ojaswin Mujoo
2026-10-07  0:41 ` [PATCH 2/2] ext4: track zeroed out blocks of unwritten extents for fast commit Daejun Park via B4 Relay
2026-10-07 13:41   ` Jan Kara
     [not found]   ` <CGME20261007134122epcas2p261c7ea484933b17c10e745ec844b0aff@epcms2p1>
2026-10-08  1:24     ` Daejun Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®