[PATCH v2 0/4] ext4: assorted fixes related to the buffered iomap conversion

[PATCH v2 0/4] ext4: assorted fixes related to the buffered iomap conversion

[Zhang Yi]

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.

Changes since v1:
 - In patch 3, when converting unwritten extents, always update
   map.m_len even if no conversion was actually performed, to prevent
   map.m_len from being 0 on ENOSPC retry. [Sashiko]

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 | 47 ++++++++++++++++++++++++++++++++---------------
 fs/ext4/inline.c  |  8 ++++++--
 fs/ext4/inode.c   |  3 ++-
 3 files changed, 40 insertions(+), 18 deletions(-)

-- 
2.52.0

[PATCH v2 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages()

[Zhang Yi]

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

[PATCH v2 2/4] ext4: fix extent status cache clearing when truncating inline data

[Zhang Yi]

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 '244392197bb9 ("ext4: skip
block allocation for holes in the data submission path")' 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.

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

[PATCH v2 3/4] ext4: add inode to orphan list before extent conversion

[Zhang Yi]

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 | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ad0d80e33f609..9f80beeaa1f29 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,21 @@ 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;
 		}
 
+		/*
+		 * The conversion may have failed without converting
+		 * anything. Only move epos over the blocks that were
+		 * actually converted, so that a failure updates
+		 * i_disksize just far enough to cover them.
+		 */
+		if (map.m_len)
+			epos = EXT4_LBLK_TO_B(inode, map.m_lblk + 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 +4747,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 +4771,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

[PATCH v2 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks()

[Zhang Yi]

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

Re: [PATCH v2 1/4] ext4: return 0 instead of stale ret in ext4_iomap_writepages()

[sashiko-bot]

> 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/20261030021338.1768292-1-yi.zhang@huaweicloud.com?part=1

Re: [PATCH v2 4/4] ext4: initialize map->m_pblk in ext4_da_map_blocks()

[sashiko-bot]

> 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/20261030021338.1768292-1-yi.zhang@huaweicloud.com?part=4

Re: [PATCH v2 3/4] ext4: add inode to orphan list before extent conversion

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues.

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

Re: [PATCH v2 2/4] ext4: fix extent status cache clearing when truncating inline data

[sashiko-bot]

> 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/20261030021338.1768292-1-yi.zhang@huaweicloud.com?part=2