* [PATCH v2] ext4: propagate errors from fast commit i_blocks replay
@ 2026-09-14 12:05 lty
2026-09-14 12:22 ` sashiko-bot
2026-09-14 14:15 ` Jan Kara
0 siblings, 2 replies; 3+ messages in thread
From: lty @ 2026-09-14 12:05 UTC (permalink / raw)
To: tytso
Cc: adilger.kernel, libaokun, jack, ojaswin, ritesh.list, yi.zhang,
harshadshirwadkar, linux-ext4, linux-kernel, lty
ext4_ext_replay_set_iblocks() breaks out of its mapping loop when
ext4_map_blocks() fails, but then continues with a partial block count
and unconditionally returns success. The fast commit replay caller also
ignores the helper return value.
Stop the count immediately on mapping errors, leave i_blocks unchanged,
and propagate the failure to abort inode replay.
Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Signed-off-by: lty <781735889@qq.com>
Changes in v2:
- Propagate errors from all ext4_find_extent() and skip_hole() calls in
ext4_ext_replay_set_iblocks(), as requested by Jan Kara.
- Keep i_blocks unchanged when any replay lookup fails.
---
fs/ext4/extents.c | 26 ++++++++++++++++----------
fs/ext4/fast_commit.c | 7 +++++--
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 76038b6c3655..f8c805d80c33 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -6275,7 +6275,7 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
map.m_len = end - cur;
ret = ext4_map_blocks(NULL, inode, &map, 0);
if (ret < 0)
- break;
+ goto cleanup;
if (ret > 0)
numblks += ret;
cur = cur + map.m_len;
@@ -6291,15 +6291,19 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
cur = 0;
ret = skip_hole(inode, &cur);
if (ret < 0)
- goto out;
+ goto cleanup;
path = ext4_find_extent(inode, cur, path, 0);
- if (IS_ERR(path))
- goto out;
+ if (IS_ERR(path)) {
+ ret = PTR_ERR(path);
+ goto cleanup;
+ }
numblks += path->p_depth;
while (cur < end) {
path = ext4_find_extent(inode, cur, path, 0);
- if (IS_ERR(path))
- break;
+ if (IS_ERR(path)) {
+ ret = PTR_ERR(path);
+ goto cleanup;
+ }
ex = path[path->p_depth].p_ext;
if (!ex)
goto cleanup;
@@ -6308,11 +6312,13 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
ext4_ext_get_actual_len(ex));
ret = skip_hole(inode, &cur);
if (ret < 0)
- break;
+ goto cleanup;
path2 = ext4_find_extent(inode, cur, path2, 0);
- if (IS_ERR(path2))
- break;
+ if (IS_ERR(path2)) {
+ ret = PTR_ERR(path2);
+ goto cleanup;
+ }
for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) {
cmp1 = cmp2 = 0;
@@ -6333,7 +6339,7 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
cleanup:
ext4_free_ext_path(path);
ext4_free_ext_path(path2);
- return 0;
+ return ret;
}
int ext4_ext_clear_bb(struct inode *inode)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 0cac890cf370..8110b01e36c1 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -2002,8 +2002,11 @@ static int ext4_fc_replay_inode(struct super_block *sb,
* crashing. This should be fixed but until then, we calculate
* the number of blocks the inode.
*/
- if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
- ext4_ext_replay_set_iblocks(inode);
+ if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA)) {
+ ret = ext4_ext_replay_set_iblocks(inode);
+ if (ret)
+ goto out_brelse;
+ }
inode->i_generation = le32_to_cpu(ext4_raw_inode(&iloc)->i_generation);
ext4_reset_inode_seed(inode);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ext4: propagate errors from fast commit i_blocks replay
2026-09-14 12:05 [PATCH v2] ext4: propagate errors from fast commit i_blocks replay lty
@ 2026-09-14 12:22 ` sashiko-bot
2026-09-14 14:15 ` Jan Kara
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-14 12:22 UTC (permalink / raw)
To: lty; +Cc: linux-ext4, tytso
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete error propagation in `ext4_ext_replay_set_iblocks` when encountering an empty extent leaf.
--
commit d0296b8bc160aac911c99a12a53e4f06189bb5cd
Author: lty <781735889@qq.com>
ext4: propagate errors from fast commit i_blocks replay
This commit modifies ext4_ext_replay_set_iblocks() to stop calculating
blocks and propagate errors when mapping fails. It ensures failures from
extent lookups abort the fast commit inode replay.
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
[ ... ]
> @@ -6291,15 +6291,19 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> numblks += path->p_depth;
> while (cur < end) {
> path = ext4_find_extent(inode, cur, path, 0);
> - if (IS_ERR(path))
> - break;
> + if (IS_ERR(path)) {
> + ret = PTR_ERR(path);
> + goto cleanup;
> + }
> ex = path[path->p_depth].p_ext;
> if (!ex)
> goto cleanup;
[Severity: High]
Since ret is 0 on a successful extent lookup, does this branch silently return
success when it encounters an empty extent leaf?
If an empty leaf (!ex) represents a corrupted extent tree, should this set an
error code like -EFSCORRUPTED before jumping to cleanup, so the fast commit
replay can properly abort?
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_BCDA824359279D18963B838B15B01BC1CD06@qq.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ext4: propagate errors from fast commit i_blocks replay
2026-09-14 12:05 [PATCH v2] ext4: propagate errors from fast commit i_blocks replay lty
2026-09-14 12:22 ` sashiko-bot
@ 2026-09-14 14:15 ` Jan Kara
1 sibling, 0 replies; 3+ messages in thread
From: Jan Kara @ 2026-09-14 14:15 UTC (permalink / raw)
To: lty
Cc: tytso, adilger.kernel, libaokun, jack, ojaswin, ritesh.list,
yi.zhang, harshadshirwadkar, linux-ext4, linux-kernel
On Mon 14-09-26 20:05:56, lty wrote:
> ext4_ext_replay_set_iblocks() breaks out of its mapping loop when
> ext4_map_blocks() fails, but then continues with a partial block count
> and unconditionally returns success. The fast commit replay caller also
> ignores the helper return value.
>
> Stop the count immediately on mapping errors, leave i_blocks unchanged,
> and propagate the failure to abort inode replay.
>
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
>
> Signed-off-by: lty <781735889@qq.com>
>
> Changes in v2:
> - Propagate errors from all ext4_find_extent() and skip_hole() calls in
> ext4_ext_replay_set_iblocks(), as requested by Jan Kara.
> - Keep i_blocks unchanged when any replay lookup fails.
The 'Changes' paragraph belongs after the --- separator below so that it
doesn't get into the pernament changelog. Otherwise the fix looks good to
me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/extents.c | 26 ++++++++++++++++----------
> fs/ext4/fast_commit.c | 7 +++++--
> 2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 76038b6c3655..f8c805d80c33 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -6275,7 +6275,7 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> map.m_len = end - cur;
> ret = ext4_map_blocks(NULL, inode, &map, 0);
> if (ret < 0)
> - break;
> + goto cleanup;
> if (ret > 0)
> numblks += ret;
> cur = cur + map.m_len;
> @@ -6291,15 +6291,19 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> cur = 0;
> ret = skip_hole(inode, &cur);
> if (ret < 0)
> - goto out;
> + goto cleanup;
> path = ext4_find_extent(inode, cur, path, 0);
> - if (IS_ERR(path))
> - goto out;
> + if (IS_ERR(path)) {
> + ret = PTR_ERR(path);
> + goto cleanup;
> + }
> numblks += path->p_depth;
> while (cur < end) {
> path = ext4_find_extent(inode, cur, path, 0);
> - if (IS_ERR(path))
> - break;
> + if (IS_ERR(path)) {
> + ret = PTR_ERR(path);
> + goto cleanup;
> + }
> ex = path[path->p_depth].p_ext;
> if (!ex)
> goto cleanup;
> @@ -6308,11 +6312,13 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> ext4_ext_get_actual_len(ex));
> ret = skip_hole(inode, &cur);
> if (ret < 0)
> - break;
> + goto cleanup;
>
> path2 = ext4_find_extent(inode, cur, path2, 0);
> - if (IS_ERR(path2))
> - break;
> + if (IS_ERR(path2)) {
> + ret = PTR_ERR(path2);
> + goto cleanup;
> + }
>
> for (i = 0; i <= max(path->p_depth, path2->p_depth); i++) {
> cmp1 = cmp2 = 0;
> @@ -6333,7 +6339,7 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> cleanup:
> ext4_free_ext_path(path);
> ext4_free_ext_path(path2);
> - return 0;
> + return ret;
> }
>
> int ext4_ext_clear_bb(struct inode *inode)
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0cac890cf370..8110b01e36c1 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2002,8 +2002,11 @@ static int ext4_fc_replay_inode(struct super_block *sb,
> * crashing. This should be fixed but until then, we calculate
> * the number of blocks the inode.
> */
> - if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
> - ext4_ext_replay_set_iblocks(inode);
> + if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA)) {
> + ret = ext4_ext_replay_set_iblocks(inode);
> + if (ret)
> + goto out_brelse;
> + }
>
> inode->i_generation = le32_to_cpu(ext4_raw_inode(&iloc)->i_generation);
> ext4_reset_inode_seed(inode);
> --
> 2.34.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-14 14:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14 12:05 [PATCH v2] ext4: propagate errors from fast commit i_blocks replay lty
2026-09-14 12:22 ` sashiko-bot
2026-09-14 14:15 ` Jan Kara
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.