All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ext4: assorted fixes related to the buffered iomap conversion
@ 2026-10-26 11:51 Zhang Yi
  2026-10-26 11:51 ` [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages() Zhang Yi
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Zhang Yi @ 2026-10-26 11:51 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, libaokun,
	jack, ojaswin, ritesh.list, yi.zhang, yi.zhang, yizhang089,
	chengzhihao1, yangerkun, yukuai

From: Zhang Yi <yi.zhang@huawei.com>

Hi,

This series fixes some small issues introduced by the iomap
conversion [1] as well as some pre-existing issues found by Sashiko. The
first two patches were sent separately before [2]. Packaging them into
this series to facilitate merging.

Patch 1/4: fixes a complaint from smatch, making ext4_iomap_writepages()
           return 0 directly when it encounters an inode that doesn't go
           through the buffered iomap path during leftover writeback,
           instead of reusing the stale ret variable, even though it is
           also 0. Not a functional problem, it just makes the code
           clearer and silences the complaint.

Patch 2/4: fixes a possible extent status cache inconsistency when
           truncating an inode that is in the middle of a delayed inline
           data conversion.

Patch 3/4: fixes the issue reported by Sashiko where
           ext4_alloc_file_blocks() cannot guarantee atomicity when
           converting extents during fallocate WRITE_ZEROES, causing
           fsck to complain.

Patch 4/4: fixes the issue reported by Sashiko where the tracepoint on
           the iomap delalloc write path prints an uninitialized m_pblk
           when it encounters a delalloc-mapped extent.

Thanks,
Yi.

[1] https://lore.kernel.org/linux-ext4/20261009103033.2920530-1-yi.zhang@huaweicloud.com/
[2] https://lore.kernel.org/linux-ext4/20261014021714.2382353-1-yi.zhang@huaweicloud.com/
    https://lore.kernel.org/linux-ext4/20261016095927.1762656-1-yi.zhang@huaweicloud.com/


Zhang Yi (4):
  ext4: return 0 instead of stale ret in ext4_iomap_writepages()
  ext4: fix extent status cache clearing when truncating inline data
  ext4: add inode to orphan list before extent conversion
  ext4: initialize map->m_pblk in ext4_da_map_blocks()

 fs/ext4/extents.c | 49 +++++++++++++++++++++++++++++++----------------
 fs/ext4/inline.c  |  8 ++++++--
 fs/ext4/inode.c   |  3 ++-
 3 files changed, 40 insertions(+), 20 deletions(-)

-- 
2.52.0


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

* [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages()
  2026-10-26 11:51 [PATCH 0/4] ext4: assorted fixes related to the buffered iomap conversion Zhang Yi
@ 2026-10-26 11:51 ` Zhang Yi
  2026-10-26 12:13   ` sashiko-bot
  2026-10-26 11:51 ` [PATCH 2/4] ext4: fix extent status cache clearing when truncating inline data Zhang Yi
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Zhang Yi @ 2026-10-26 11:51 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, libaokun,
	jack, ojaswin, ritesh.list, yi.zhang, yi.zhang, yizhang089,
	chengzhihao1, yangerkun, yukuai

From: Zhang Yi <yi.zhang@huawei.com>

smatch reports a "missing error code" warning:

  fs/ext4/inode.c:4518 ext4_iomap_writepages() warn: missing error code? 'ret'

The 'ret' variable has already been consumed by the
ext4_emergency_state() check at the beginning of the function, so its
value is always zero at this point. Returning it is equivalent to
returning 0, but it is misleading because it looks like an error code is
being propagated.

This branch handles the race with ext4_change_inode_journal_flag(),
which flushes the dirty data and drops the page cache before switching
the inode to the buffer_head path, so there is genuinely nothing to
write back here. Return 0 explicitly to make the intent clear.

Fixes: bdcbb0f19e92 ("ext4: partially enable iomap for the buffered I/O path of regular files")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202610160314.PLMEXaOJ-lkp@intel.com/
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b4d581aa5ee1a..6b2f1b07f41e6 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4515,7 +4515,7 @@ static int ext4_iomap_writepages(struct address_space *mapping,
 	 */
 	if (!ext4_inode_buffered_iomap(inode)) {
 		ext4_writepages_up_read(sb, alloc_ctx);
-		return ret;
+		return 0;
 	}
 
 	trace_ext4_writepages(inode, wbc);
-- 
2.52.0


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

* [PATCH 2/4] ext4: fix extent status cache clearing when truncating inline data
  2026-10-26 11:51 [PATCH 0/4] ext4: assorted fixes related to the buffered iomap conversion Zhang Yi
  2026-10-26 11:51 ` [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages() Zhang Yi
@ 2026-10-26 11:51 ` Zhang Yi
  2026-10-26 12:47   ` sashiko-bot
  2026-10-26 11:51 ` [PATCH 3/4] ext4: add inode to orphan list before extent conversion Zhang Yi
  2026-10-26 11:51 ` [PATCH 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks() Zhang Yi
  3 siblings, 1 reply; 10+ messages in thread
From: Zhang Yi @ 2026-10-26 11:51 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, libaokun,
	jack, ojaswin, ritesh.list, yi.zhang, yi.zhang, yizhang089,
	chengzhihao1, yangerkun, yukuai

From: Zhang Yi <yi.zhang@huawei.com>

ext4_inline_data_truncate() clears the whole extent status tree of the
inode when it truncates the inline data, with

	ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);

It does that when the inode still carries inline data while the
EXT4_STATE_MAY_INLINE_DATA state bit is already cleared. This is the
case once an inline-to-extent conversion has been initiated, which is
triggered by a delalloc buffered write larger than the inline space in
ext4_da_convert_inline_data_to_extent().

The clear is meant to drop stale extent status entries of such an inode
to prevent stale extents and stale reserved space after the conversion,
but the range is too wide.  It also drops the entries of the blocks
below the new end of file, that is the blocks that stay in the file.

When writeback of these blocks starts later, the lookup of such a block
misses the extent status tree, falls back to the extent tree and caches
a hole entry for it.  Those blocks then have dirty, mapped and delayed
buffers together with a hole extent status entry.  This is an
unexpected inconsistency.

This problem has been hidden so far because we allocate blocks and new
extents for them during writeback.  But once [1] is merged during the
iomap conversion, the writeback behavior will be normalized, and it will
no longer allocate blocks for holes, thus exposing this problem.

Therefore, restrict the range to the blocks that are actually truncated
away, and pass EXT_MAX_BLOCKS minus lblk as the length.

[1] https://lore.kernel.org/linux-ext4/20261009103033.2920530-9-yi.zhang@huaweicloud.com/

Reported-by: syzbot ci <syzbot+ci46f64f7b6a2a188e@syzkaller.appspotmail.com>
Closes: https://lore.kernel.org/linux-ext4/6ac92a4c.70ed5d86.2ac85.0072.GAE@google.com/
Fixes: 0add491df4e5 ("ext4: remove extent cache entries when truncating inline data")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inline.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index ceee69a66482b..b630e56815bb5 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1905,8 +1905,12 @@ int ext4_inline_data_truncate(struct inode *inode, int *has_inline)
 		 * the extent status cache must be cleared to avoid leaving
 		 * behind stale delayed allocated extent entries
 		 */
-		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
-			ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
+		if (!ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
+			ext4_lblk_t lblk = EXT4_B_TO_LBLK(inode, i_size);
+
+			ext4_es_remove_extent(inode, lblk,
+					      EXT_MAX_BLOCKS - lblk);
+		}
 
 		/* Clear the content in the xattr space. */
 		if (inline_size > EXT4_MIN_INLINE_DATA_SIZE) {
-- 
2.52.0


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

* [PATCH 3/4] ext4: add inode to orphan list before extent conversion
  2026-10-26 11:51 [PATCH 0/4] ext4: assorted fixes related to the buffered iomap conversion Zhang Yi
  2026-10-26 11:51 ` [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages() Zhang Yi
  2026-10-26 11:51 ` [PATCH 2/4] ext4: fix extent status cache clearing when truncating inline data Zhang Yi
@ 2026-10-26 11:51 ` Zhang Yi
  2026-10-26 13:10   ` sashiko-bot
  2026-10-26 11:51 ` [PATCH 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks() Zhang Yi
  3 siblings, 1 reply; 10+ messages in thread
From: Zhang Yi @ 2026-10-26 11:51 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, libaokun,
	jack, ojaswin, ritesh.list, yi.zhang, yi.zhang, yizhang089,
	chengzhihao1, yangerkun, yukuai

From: Zhang Yi <yi.zhang@huawei.com>

ext4_alloc_file_blocks() zeroes the blocks it has just allocated and
converts their unwritten extents in a handle started with
ext4_chunk_trans_blocks(), which reserves credits for a single extent.
The range is allocated with ext4_map_blocks() first and converted
afterwards, and only the unwritten extent conversion which runs on I/O
completion can modify the extent tree of that range in between, since
this path holds i_rwsem. If such a conversion converts part of the
range first, it splits the extent, and converting the whole range then
needs more credits than the handle has reserved.

Before commit 10660a0ca7ed ("ext4: rework handle credit accounting for
unwritten extent conversion"), this race condition led to insufficient
credits, which made the conversion fail with -ENOSPC in the middle of
the range. After it, ext4_convert_unwritten_extents() can extend or
restart the transaction. Restarting commits everything modified so
far, so the extents converted by the first iterations reach the disk
before this function adds the inode to the orphan list. A crash in
between leaves written extents beyond i_disksize on disk with the
inode not on the orphan list, which fsck complains about.

Add the inode to the orphan list before the conversion instead and keep
it there until i_disksize has caught up. A restart then commits the
orphan entry together with the extents converted so far, and the entry
is only removed by the transaction which raises i_disksize and i_size.

Fixes: 10660a0ca7ed ("ext4: rework handle credit accounting for unwritten extent conversion")
Fixes: f7237a775c8f ("ext4: protect WRITE_ZEROES written extents with orphan list")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents.c | 49 +++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ad0d80e33f609..71dda0ffeb34a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4701,6 +4701,23 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 				break;
 			}
 
+			/*
+			 * Add the inode to the orphan list before the
+			 * conversion, as it may restart the transaction and
+			 * commit the extents converted so far.  Written
+			 * extents must stay covered by the orphan list until
+			 * i_disksize catches up.
+			 */
+			if (new_size) {
+				ret2 = ext4_orphan_add(handle, inode);
+				orphan = true;
+				if (unlikely(ret2)) {
+					ret = ret2;
+					ext4_journal_stop(handle);
+					break;
+				}
+			}
+
 			ret = ext4_convert_unwritten_extents(handle,
 					inode, (loff_t)map.m_lblk << blkbits,
 					(loff_t)map.m_len << blkbits,
@@ -4708,23 +4725,19 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 			if (ret)
 				map.m_len = converted;
 
-			/*
-			 * If blocks beyond i_disksize are converted, add
-			 * the inode to the orphan list and advance the epos.
-			 */
-			if (new_size && converted) {
-				ret2 = ext4_orphan_add(handle, inode);
-				ret = ret ? ret : ret2;
-				orphan = true;
-			}
-
 			ret3 = ext4_journal_stop(handle);
 			ret = ret ? ret : ret3;
 		}
 
-		map.m_lblk += map.m_len;
-		map.m_len = len_lblk = len_lblk - map.m_len;
-		epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
+		/*
+		 * The conversion may have failed without converting
+		 * anything.
+		 */
+		if (map.m_len) {
+			map.m_lblk += map.m_len;
+			map.m_len = len_lblk = len_lblk - map.m_len;
+			epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
+		}
 		if (ret)
 			break;
 	}
@@ -4732,12 +4745,13 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
 		goto retry;
 
-	if (!epos || !new_size)
+	if (!new_size || (!epos && !orphan))
 		return ret;
 
 	/*
-	 * Allocate blocks, update the file size to match the size of the
-	 * already successfully allocated blocks.
+	 * Update the file size to match the size of the already
+	 * successfully allocated blocks, and release the orphan entry
+	 * if we added one.
 	 */
 	if (epos > new_size)
 		epos = new_size;
@@ -4755,7 +4769,8 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
 		goto out;
 	}
 
-	ext4_update_inode_size(inode, epos);
+	if (epos)
+		ext4_update_inode_size(inode, epos);
 	if (orphan && inode->i_nlink)
 		ext4_orphan_del(handle, inode);
 
-- 
2.52.0


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

* [PATCH 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks()
  2026-10-26 11:51 [PATCH 0/4] ext4: assorted fixes related to the buffered iomap conversion Zhang Yi
                   ` (2 preceding siblings ...)
  2026-10-26 11:51 ` [PATCH 3/4] ext4: add inode to orphan list before extent conversion Zhang Yi
@ 2026-10-26 11:51 ` Zhang Yi
  2026-10-26 13:12   ` sashiko-bot
  3 siblings, 1 reply; 10+ messages in thread
From: Zhang Yi @ 2026-10-26 11:51 UTC (permalink / raw)
  To: linux-ext4
  Cc: linux-fsdevel, linux-kernel, tytso, adilger.kernel, libaokun,
	jack, ojaswin, ritesh.list, yi.zhang, yi.zhang, yizhang089,
	chengzhihao1, yangerkun, yukuai

From: Zhang Yi <yi.zhang@huawei.com>

ext4_da_map_blocks() returns a delalloc mapping through @map without
touching map->m_pblk at all. Callers that only set up m_lblk and m_len
of their local struct ext4_map_blocks and leave the rest to the mapping
helper, like ext4_iomap_map_blocks() does, end up with whatever happened
to be on their stack, so the trace_ext4_iomap_buffered_write_begin()
tracepoint prints an uninitialized pblk for a delalloc mapping.

This has been harmless so far, ext4_da_get_block_prep() and
ext4_set_iomap() both ignore m_pblk for a delalloc mapping, only the new
tracepoint dumps it.

Initialize m_pblk along with m_flags at the beginning of
ext4_da_map_blocks(), so that a delalloc mapping is always reported with
pblk 0, the same way ext4_map_blocks() reports it.

Fixes: 008f019ab559 ("ext4: add block mapping tracepoints for iomap buffered I/O path")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 6b2f1b07f41e6..c1c46e86bba3e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2041,6 +2041,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
 #endif
 
 	map->m_flags = 0;
+	map->m_pblk = 0;
 	ext_debug(inode, "max_blocks %u, logical block %lu\n", map->m_len,
 		  (unsigned long) map->m_lblk);
 
-- 
2.52.0


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

* Re: [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages()
  2026-10-26 11:51 ` [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages() Zhang Yi
@ 2026-10-26 12:13   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-26 12:13 UTC (permalink / raw)
  To: Zhang Yi, linux-ext4; +Cc: tytso

> From: Zhang Yi <yi.zhang@huawei.com>
> 
> smatch reports a "missing error code" warning:
> 
>   fs/ext4/inode.c:4518 ext4_iomap_writepages() warn: missing error code? 'ret'
> 
> The 'ret' variable has already been consumed by the
> ext4_emergency_state() check at the beginning of the function, so its
> value is always zero at this point. Returning it is equivalent to
> returning 0, but it is misleading because it looks like an error code is
> being propagated.
> 
> This branch handles the race with ext4_change_inode_journal_flag(),
> which flushes the dirty data and drops the page cache before switching
> the inode to the buffer_head path, so there is genuinely nothing to
> write back here. Return 0 explicitly to make the intent clear.
> 
> Fixes: bdcbb0f19e92 ("ext4: partially enable iomap for the buffered I/O path of regular files")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/r/202610160314.PLMEXaOJ-lkp@intel.com/
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026115131.1735014-1-yi.zhang@huaweicloud.com?part=1


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

* Re: [PATCH 2/4] ext4: fix extent status cache clearing when truncating inline data
  2026-10-26 11:51 ` [PATCH 2/4] ext4: fix extent status cache clearing when truncating inline data Zhang Yi
@ 2026-10-26 12:47   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-26 12:47 UTC (permalink / raw)
  To: Zhang Yi, linux-ext4; +Cc: tytso

> From: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4_inline_data_truncate() clears the whole extent status tree of the
> inode when it truncates the inline data, with
> 
> 	ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
> 
> It does that when the inode still carries inline data while the
> EXT4_STATE_MAY_INLINE_DATA state bit is already cleared. This is the
> case once an inline-to-extent conversion has been initiated, which is
> triggered by a delalloc buffered write larger than the inline space in
> ext4_da_convert_inline_data_to_extent().
> 
> The clear is meant to drop stale extent status entries of such an inode
> to prevent stale extents and stale reserved space after the conversion,
> [ ... ]
> 
> Reported-by: syzbot ci <syzbot+ci46f64f7b6a2a188e@syzkaller.appspotmail.com>
> Closes: https://lore.kernel.org/linux-ext4/6ac92a4c.70ed5d86.2ac85.0072.GAE@google.com/
> Fixes: 0add491df4e5 ("ext4: remove extent cache entries when truncating inline data")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026115131.1735014-1-yi.zhang@huaweicloud.com?part=2


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

* Re: [PATCH 3/4] ext4: add inode to orphan list before extent conversion
  2026-10-26 11:51 ` [PATCH 3/4] ext4: add inode to orphan list before extent conversion Zhang Yi
@ 2026-10-26 13:10   ` sashiko-bot
  2026-10-27  8:20     ` Zhang Yi
  0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-10-26 13:10 UTC (permalink / raw)
  To: Zhang Yi, linux-ext4; +Cc: tytso

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ext4: silent false success on ENOSPC retry in ext4_alloc_file_blocks()
--

commit af54a86e4faf105f27b9869fc053a46b111dbda1
Author: Zhang Yi <yi.zhang@huawei.com>

ext4: add inode to orphan list before extent conversion

This commit moves the ext4_orphan_add() call in ext4_alloc_file_blocks()
before ext4_convert_unwritten_extents() so that transaction restarts during
extent conversion do not leave written extents beyond i_disksize uncovered by
the orphan list. It also adjusts the post-conversion progress and cleanup
handling when conversion fails before converting any blocks.

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7193429a05e1f..677749f0c9c68 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4701,6 +4701,23 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>  				break;
>  			}
>  
> +			/*
> +			 * Add the inode to the orphan list before the
> +			 * conversion, as it may restart the transaction and
> +			 * commit the extents converted so far.  Written
> +			 * extents must stay covered by the orphan list until
> +			 * i_disksize catches up.
> +			 */
> +			if (new_size) {
> +				ret2 = ext4_orphan_add(handle, inode);
> +				orphan = true;
> +				if (unlikely(ret2)) {
> +					ret = ret2;
> +					ext4_journal_stop(handle);
> +					break;
> +				}
> +			}
> +
>  			ret = ext4_convert_unwritten_extents(handle,
>  					inode, (loff_t)map.m_lblk << blkbits,
>  					(loff_t)map.m_len << blkbits,
> @@ -4708,23 +4725,19 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>  			if (ret)
>  				map.m_len = converted;
>  
> -			/*
> -			 * If blocks beyond i_disksize are converted, add
> -			 * the inode to the orphan list and advance the epos.
> -			 */
> -			if (new_size && converted) {
> -				ret2 = ext4_orphan_add(handle, inode);
> -				ret = ret ? ret : ret2;
> -				orphan = true;
> -			}
> -
>  			ret3 = ext4_journal_stop(handle);
>  			ret = ret ? ret : ret3;
>  		}
>  
> -		map.m_lblk += map.m_len;
> -		map.m_len = len_lblk = len_lblk - map.m_len;
> -		epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
> +		/*
> +		 * The conversion may have failed without converting
> +		 * anything.
> +		 */
> +		if (map.m_len) {
> +			map.m_lblk += map.m_len;
> +			map.m_len = len_lblk = len_lblk - map.m_len;
> +			epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
> +		}
>  		if (ret)
>  			break;
>  	}
> @@ -4732,12 +4745,13 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>  	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry;

[Severity: High]
What happens on the goto retry path if ext4_convert_unwritten_extents() fails
with -ENOSPC and converted == 0?

When ret is non-zero and converted is 0, map.m_len is set to 0:

	if (ret)
		map.m_len = converted;

Previously, map.m_len = len_lblk = len_lblk - map.m_len ran unconditionally,
which restored map.m_len to len_lblk when map.m_len was 0. With that update
now guarded by if (map.m_len), map.m_len remains 0 while len_lblk > 0.

If ret == -ENOSPC and ext4_should_retry_alloc() returns true, jumping to
retry calls ext4_map_blocks() with map.m_len == 0:

fs/ext4/extents.c:ext4_alloc_file_blocks() {
	...
	while (len_lblk) {
		...
		ret = ext4_map_blocks(handle, inode, &map, flags);
		if (ret <= 0) {
			...
			ext4_mark_inode_dirty(handle, inode);
			ext4_journal_stop(handle);
			break;
		}
}

Because map.m_lblk already maps to the unwritten extent allocated before the
retry, both the extent status tree lookup in ext4_map_blocks() and
ext4_ext_handle_unwritten_extents() clamp the returned block count to
map.m_len (0). That causes ext4_map_blocks() to return 0, breaking out of the
loop with ret == 0 and silently returning success to the caller without
converting the unwritten extent or finishing the remaining allocation.

Should map.m_len be reset to len_lblk when map.m_len is 0 (or before jumping
to retry)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026115131.1735014-1-yi.zhang@huaweicloud.com?part=3

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

* Re: [PATCH 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks()
  2026-10-26 11:51 ` [PATCH 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks() Zhang Yi
@ 2026-10-26 13:12   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-26 13:12 UTC (permalink / raw)
  To: Zhang Yi, linux-ext4; +Cc: tytso

> From: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4_da_map_blocks() returns a delalloc mapping through @map without
> touching map->m_pblk at all. Callers that only set up m_lblk and m_len
> of their local struct ext4_map_blocks and leave the rest to the mapping
> helper, like ext4_iomap_map_blocks() does, end up with whatever happened
> to be on their stack, so the trace_ext4_iomap_buffered_write_begin()
> tracepoint prints an uninitialized pblk for a delalloc mapping.
> 
> This has been harmless so far, ext4_da_get_block_prep() and
> ext4_set_iomap() both ignore m_pblk for a delalloc mapping, only the new
> tracepoint dumps it.
> 
> Initialize m_pblk along with m_flags at the beginning of
> ext4_da_map_blocks(), so that a delalloc mapping is always reported with
> pblk 0, the same way ext4_map_blocks() reports it.
> 
> Fixes: 008f019ab559 ("ext4: add block mapping tracepoints for iomap buffered I/O path")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026115131.1735014-1-yi.zhang@huaweicloud.com?part=4


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

* Re: [PATCH 3/4] ext4: add inode to orphan list before extent conversion
  2026-10-26 13:10   ` sashiko-bot
@ 2026-10-27  8:20     ` Zhang Yi
  0 siblings, 0 replies; 10+ messages in thread
From: Zhang Yi @ 2026-10-27  8:20 UTC (permalink / raw)
  To: sashiko-reviews, linux-ext4; +Cc: tytso

On 10/26/2026 9:10 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] ext4: silent false success on ENOSPC retry in ext4_alloc_file_blocks()
> --
> 
> commit af54a86e4faf105f27b9869fc053a46b111dbda1
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> ext4: add inode to orphan list before extent conversion
> 
> This commit moves the ext4_orphan_add() call in ext4_alloc_file_blocks()
> before ext4_convert_unwritten_extents() so that transaction restarts during
> extent conversion do not leave written extents beyond i_disksize uncovered by
> the orphan list. It also adjusts the post-conversion progress and cleanup
> handling when conversion fails before converting any blocks.
> 
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 7193429a05e1f..677749f0c9c68 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -4701,6 +4701,23 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>>  				break;
>>  			}
>>  
>> +			/*
>> +			 * Add the inode to the orphan list before the
>> +			 * conversion, as it may restart the transaction and
>> +			 * commit the extents converted so far.  Written
>> +			 * extents must stay covered by the orphan list until
>> +			 * i_disksize catches up.
>> +			 */
>> +			if (new_size) {
>> +				ret2 = ext4_orphan_add(handle, inode);
>> +				orphan = true;
>> +				if (unlikely(ret2)) {
>> +					ret = ret2;
>> +					ext4_journal_stop(handle);
>> +					break;
>> +				}
>> +			}
>> +
>>  			ret = ext4_convert_unwritten_extents(handle,
>>  					inode, (loff_t)map.m_lblk << blkbits,
>>  					(loff_t)map.m_len << blkbits,
>> @@ -4708,23 +4725,19 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>>  			if (ret)
>>  				map.m_len = converted;
>>  
>> -			/*
>> -			 * If blocks beyond i_disksize are converted, add
>> -			 * the inode to the orphan list and advance the epos.
>> -			 */
>> -			if (new_size && converted) {
>> -				ret2 = ext4_orphan_add(handle, inode);
>> -				ret = ret ? ret : ret2;
>> -				orphan = true;
>> -			}
>> -
>>  			ret3 = ext4_journal_stop(handle);
>>  			ret = ret ? ret : ret3;
>>  		}
>>  
>> -		map.m_lblk += map.m_len;
>> -		map.m_len = len_lblk = len_lblk - map.m_len;
>> -		epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
>> +		/*
>> +		 * The conversion may have failed without converting
>> +		 * anything.
>> +		 */
>> +		if (map.m_len) {
>> +			map.m_lblk += map.m_len;
>> +			map.m_len = len_lblk = len_lblk - map.m_len;
>> +			epos = EXT4_LBLK_TO_B(inode, map.m_lblk);
>> +		}
>>  		if (ret)
>>  			break;
>>  	}
>> @@ -4732,12 +4745,13 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
>>  	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
>>  		goto retry;
> 
> [Severity: High]
> What happens on the goto retry path if ext4_convert_unwritten_extents() fails
> with -ENOSPC and converted == 0?
> 
> When ret is non-zero and converted is 0, map.m_len is set to 0:
> 
> 	if (ret)
> 		map.m_len = converted;
> 
> Previously, map.m_len = len_lblk = len_lblk - map.m_len ran unconditionally,
> which restored map.m_len to len_lblk when map.m_len was 0. With that update
> now guarded by if (map.m_len), map.m_len remains 0 while len_lblk > 0.
> 
> If ret == -ENOSPC and ext4_should_retry_alloc() returns true, jumping to
> retry calls ext4_map_blocks() with map.m_len == 0:
> 
> fs/ext4/extents.c:ext4_alloc_file_blocks() {
> 	...
> 	while (len_lblk) {
> 		...
> 		ret = ext4_map_blocks(handle, inode, &map, flags);
> 		if (ret <= 0) {
> 			...
> 			ext4_mark_inode_dirty(handle, inode);
> 			ext4_journal_stop(handle);
> 			break;
> 		}
> }
> 
> Because map.m_lblk already maps to the unwritten extent allocated before the
> retry, both the extent status tree lookup in ext4_map_blocks() and
> ext4_ext_handle_unwritten_extents() clamp the returned block count to
> map.m_len (0). That causes ext4_map_blocks() to return 0, breaking out of the
> loop with ret == 0 and silently returning success to the caller without
> converting the unwritten extent or finishing the remaining allocation.
> 
> Should map.m_len be reset to len_lblk when map.m_len is 0 (or before jumping
> to retry)?
> 

Yeah, sashiko is right. We should always update map.m_len as before.
I will send to fix this.

Thanks,
Yi.




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

end of thread, other threads:[~2026-10-27  8:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-26 11:51 [PATCH 0/4] ext4: assorted fixes related to the buffered iomap conversion Zhang Yi
2026-10-26 11:51 ` [PATCH 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages() Zhang Yi
2026-10-26 12:13   ` sashiko-bot
2026-10-26 11:51 ` [PATCH 2/4] ext4: fix extent status cache clearing when truncating inline data Zhang Yi
2026-10-26 12:47   ` sashiko-bot
2026-10-26 11:51 ` [PATCH 3/4] ext4: add inode to orphan list before extent conversion Zhang Yi
2026-10-26 13:10   ` sashiko-bot
2026-10-27  8:20     ` Zhang Yi
2026-10-26 11:51 ` [PATCH 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks() Zhang Yi
2026-10-26 13:12   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.