Linux EXT4 FS development
 help / color / mirror / Atom feed
* [PATCH 0/3] ext4: fix counts left by fast commit replay of a mkdir
@ 2026-11-11 23:02 Mani Tofigh via B4 Relay
  2026-11-11 23:02 ` [PATCH 1/3] ext4: fix directory count after fast commit replay Mani Tofigh via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-11 23:02 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Andreas Dilger, Baokun Li, Jan Kara, Ojaswin Mujoo,
	Ritesh Harjani (IBM), Zhang Yi, Harshad Shirwadkar, linux-ext4,
	linux-kernel, tz2294, stable, Mani Tofigh

On ext4 with fast_commit, a crash after an fsync of a file in a new
directory leaves counts that e2fsck has to fix.  The mount works, but
e2fsck -fn reports:

  Inode 14, i_blocks is 0, should be 8.  Fix? no
  Inode 2 ref count is 3, should be 4.  Fix? no
  Inode 14 ref count is 1, should be 2.  Fix? no
  Directories count wrong for group #0 (2, counted=3).

All it takes is

  mkdir a
  fd = creat a/f0
  fsync(fd)
  <crash>

Fast commits don't track directories, so the replay of a CREAT tag has
to redo what ext4_mkdir() does and misses each of these counts.  The
replay of an rmdir has the opposite problem with the link counts.
There's one patch per count.  Patch 1 has to go before patch 3, since
with patch 3 the replay of an rmdir frees the directory and lowers the
directory count.

Baokun has the same fix as patch 1 in his LBS RFC [1], plus a flex_bg
update that never runs during replay since s_log_groups_per_flex is
still 0 then.  Baokun, happy to drop patch 1 if you'd rather send yours.

This bug was identified and confirmed with the help of a deterministic
hypervisor currently under development.  The C reproducer below however
hits it every time.

Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/ext4fcmkdir.c

[1] https://lore.kernel.org/linux-ext4/20260508121539.4174601-12-libaokun@linux.alibaba.com/

---
Mani Tofigh (3):
      ext4: fix directory count after fast commit replay
      ext4: fix directory i_blocks after fast commit replay
      ext4: fix directory link counts after fast commit replay

 fs/ext4/ext4.h        |  4 +++-
 fs/ext4/fast_commit.c | 34 ++++++++++++++++++++++++++--------
 fs/ext4/ialloc.c      |  4 +++-
 fs/ext4/namei.c       |  4 ++--
 4 files changed, 34 insertions(+), 12 deletions(-)
---
base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620
change-id: 20261111-ext4-fc-replay-mkdir-3c0adf845a19

Best regards,
--  
Mani Tofigh <mt4022@columbia.edu>



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

* [PATCH 1/3] ext4: fix directory count after fast commit replay
  2026-11-11 23:02 [PATCH 0/3] ext4: fix counts left by fast commit replay of a mkdir Mani Tofigh via B4 Relay
@ 2026-11-11 23:02 ` Mani Tofigh via B4 Relay
  2026-11-12  0:13   ` sashiko-bot
  2026-11-11 23:02 ` [PATCH 2/3] ext4: fix directory i_blocks " Mani Tofigh via B4 Relay
  2026-11-11 23:02 ` [PATCH 3/3] ext4: fix directory link counts " Mani Tofigh via B4 Relay
  2 siblings, 1 reply; 7+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-11 23:02 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Andreas Dilger, Baokun Li, Jan Kara, Ojaswin Mujoo,
	Ritesh Harjani (IBM), Zhang Yi, Harshad Shirwadkar, linux-ext4,
	linux-kernel, tz2294, stable, Mani Tofigh

From: Mani Tofigh <mt4022@columbia.edu>

After a crash, the fast commit replay of a mkdir leaves the group's
directory count one short.  e2fsck reports:

  Directories count wrong for group #0 (2, counted=3).

The replay marks the inode of an INODE or CREAT tag used with
ext4_mark_inode_used().  That lowers the group's free inode count but
never raises bg_used_dirs_count since the function doesn't know the
inode's mode.

Pass the mode to ext4_mark_inode_used() and count directories there.
The INODE tag of a new directory comes first and does the counting.
ext4_fc_replay_create() now looks up the inode before marking it used
to get its mode.

With the fix, the directory count is right after the replay.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
 fs/ext4/ext4.h        |  2 +-
 fs/ext4/fast_commit.c | 12 ++++++------
 fs/ext4/ialloc.c      |  4 +++-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bf0325ecbec7..c5044aaddb69 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3059,7 +3059,7 @@ extern int ext4fs_dirhash(const struct inode *dir, const char *name, int len,
 			  struct dx_hash_info *hinfo);
 
 /* ialloc.c */
-extern int ext4_mark_inode_used(struct super_block *sb, int ino);
+int ext4_mark_inode_used(struct super_block *sb, int ino, umode_t mode);
 extern struct inode *__ext4_new_inode(const struct mnt_idmap *, handle_t *,
 				      struct inode *, umode_t,
 				      const struct qstr *qstr, __u32 goal,
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 0cac890cf370..0eac7be3f58c 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1984,7 +1984,7 @@ static int ext4_fc_replay_inode(struct super_block *sb,
 	ret = sync_dirty_buffer(iloc.bh);
 	if (ret)
 		goto out_brelse;
-	ret = ext4_mark_inode_used(sb, ino);
+	ret = ext4_mark_inode_used(sb, ino, le16_to_cpu(raw_fc_inode->i_mode));
 	if (ret)
 		goto out_brelse;
 
@@ -2041,11 +2041,6 @@ static int ext4_fc_replay_create(struct super_block *sb,
 	trace_ext4_fc_replay(sb, EXT4_FC_TAG_CREAT, darg.ino,
 			darg.parent_ino, darg.dname_len);
 
-	/* This takes care of update group descriptor and other metadata */
-	ret = ext4_mark_inode_used(sb, darg.ino);
-	if (ret)
-		goto out;
-
 	inode = ext4_iget(sb, darg.ino, EXT4_IGET_NORMAL);
 	if (IS_ERR(inode)) {
 		ext4_debug("inode %d not found.", darg.ino);
@@ -2054,6 +2049,11 @@ static int ext4_fc_replay_create(struct super_block *sb,
 		goto out;
 	}
 
+	/* This takes care of update group descriptor and other metadata */
+	ret = ext4_mark_inode_used(sb, darg.ino, inode->i_mode);
+	if (ret)
+		goto out;
+
 	if (S_ISDIR(inode->i_mode)) {
 		/*
 		 * If we are creating a directory, we need to make sure that the
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 527f1f703c3e..e29faa8590c6 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -758,7 +758,7 @@ static int find_inode_bit(struct super_block *sb, ext4_group_t group,
 	return 1;
 }
 
-int ext4_mark_inode_used(struct super_block *sb, int ino)
+int ext4_mark_inode_used(struct super_block *sb, int ino, umode_t mode)
 {
 	unsigned long max_ino = le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count);
 	struct buffer_head *inode_bitmap_bh = NULL, *group_desc_bh = NULL;
@@ -860,6 +860,8 @@ int ext4_mark_inode_used(struct super_block *sb, int ino)
 	}
 
 	ext4_free_inodes_set(sb, gdp, ext4_free_inodes_count(sb, gdp) - 1);
+	if (S_ISDIR(mode))
+		ext4_used_dirs_set(sb, gdp, ext4_used_dirs_count(sb, gdp) + 1);
 	if (ext4_has_group_desc_csum(sb)) {
 		ext4_inode_bitmap_csum_set(sb, gdp, inode_bitmap_bh);
 		ext4_group_desc_csum_set(sb, group, gdp);

-- 
2.53.0



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

* [PATCH 2/3] ext4: fix directory i_blocks after fast commit replay
  2026-11-11 23:02 [PATCH 0/3] ext4: fix counts left by fast commit replay of a mkdir Mani Tofigh via B4 Relay
  2026-11-11 23:02 ` [PATCH 1/3] ext4: fix directory count after fast commit replay Mani Tofigh via B4 Relay
@ 2026-11-11 23:02 ` Mani Tofigh via B4 Relay
  2026-11-12  0:26   ` sashiko-bot
  2026-11-11 23:02 ` [PATCH 3/3] ext4: fix directory link counts " Mani Tofigh via B4 Relay
  2 siblings, 1 reply; 7+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-11 23:02 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Andreas Dilger, Baokun Li, Jan Kara, Ojaswin Mujoo,
	Ritesh Harjani (IBM), Zhang Yi, Harshad Shirwadkar, linux-ext4,
	linux-kernel, tz2294, stable, Mani Tofigh

From: Mani Tofigh <mt4022@columbia.edu>

After a crash, a directory created by the fast commit replay has an
i_blocks of 0.  e2fsck reports:

  Inode 14, i_blocks is 0, should be 8.

The same happens to a directory that gets new blocks while the replay
adds entries to it, e.g., a new directory that got many files in the
same fast commit:

  Inode 14, i_blocks is 8, should be 24.

Fast commits don't track directories.  The replay allocates their
blocks in ext4_init_new_dir() and __ext4_link() through
ext4_mb_new_blocks_simple(), which leaves i_blocks alone.  For regular
files ext4_fc_replay_inode() recounts i_blocks with
ext4_ext_replay_set_iblocks().  Nothing does that for directories.

Recount i_blocks of the new directory after ext4_init_new_dir() and of
the parent after a new entry is added.

With the fix, e2fsck doesn't find a wrong i_blocks after the replay
anymore.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
 fs/ext4/fast_commit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 0eac7be3f58c..9c170986f0b2 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1858,6 +1858,9 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
 		goto out;
 	}
 
+	/* The replay allocator doesn't count blocks added to the dir */
+	if (!ret && !ext4_test_inode_flag(dir, EXT4_INODE_INLINE_DATA))
+		ext4_ext_replay_set_iblocks(dir);
 	ret = 0;
 out:
 	if (dir)
@@ -2070,6 +2073,9 @@ static int ext4_fc_replay_create(struct super_block *sb,
 			ret = 0;
 			goto out;
 		}
+		/* The replay allocator doesn't count the new dir block */
+		if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
+			ext4_ext_replay_set_iblocks(inode);
 	}
 	ret = ext4_fc_replay_link_internal(sb, &darg, inode);
 	if (ret)

-- 
2.53.0



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

* [PATCH 3/3] ext4: fix directory link counts after fast commit replay
  2026-11-11 23:02 [PATCH 0/3] ext4: fix counts left by fast commit replay of a mkdir Mani Tofigh via B4 Relay
  2026-11-11 23:02 ` [PATCH 1/3] ext4: fix directory count after fast commit replay Mani Tofigh via B4 Relay
  2026-11-11 23:02 ` [PATCH 2/3] ext4: fix directory i_blocks " Mani Tofigh via B4 Relay
@ 2026-11-11 23:02 ` Mani Tofigh via B4 Relay
  2026-11-12  0:16   ` sashiko-bot
  2 siblings, 1 reply; 7+ messages in thread
From: Mani Tofigh via B4 Relay @ 2026-11-11 23:02 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Andreas Dilger, Baokun Li, Jan Kara, Ojaswin Mujoo,
	Ritesh Harjani (IBM), Zhang Yi, Harshad Shirwadkar, linux-ext4,
	linux-kernel, tz2294, stable, Mani Tofigh

From: Mani Tofigh <mt4022@columbia.edu>

After a crash, the fast commit replay of a mkdir leaves the new
directory with one link and its parent with one link too few.  e2fsck
then reports:

  Inode 2 ref count is 3, should be 4.
  Inode 14 ref count is 1, should be 2.

Fast commits don't track directories, so the replay of a CREAT tag has
to redo what ext4_mkdir() does.  ext4_fc_replay_create() sets the
link count of the new inode to 1 even for a directory, and nothing
counts the new ".." in the parent.  The replay of an rmdir has the
opposite problem.  __ext4_unlink() drops only one link of the directory
and the parent keeps the link of the removed "..":

  Inode 2 ref count is 4, should be 3.

Give a new directory 2 links and count it in its parent once its entry
is added.  When an UNLINK tag removes a directory, drop all its links
and one link of the parent like ext4_rmdir() does.  Both halves have to
go together since today a mkdir and an rmdir replayed in sequence
cancel out.

With the fix, e2fsck doesn't find wrong link counts after the replay
anymore.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Cc: stable@vger.kernel.org
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
 fs/ext4/ext4.h        |  2 ++
 fs/ext4/fast_commit.c | 16 ++++++++++++++--
 fs/ext4/namei.c       |  4 ++--
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index c5044aaddb69..bdba38062e83 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -3897,6 +3897,8 @@ extern int __ext4_unlink(struct inode *dir, const struct qstr *d_name,
 			 struct inode *inode, struct dentry *dentry);
 extern int __ext4_link(struct inode *dir, struct inode *inode,
 		       const struct qstr *d_name, struct dentry *dentry);
+void ext4_inc_count(struct inode *inode);
+void ext4_dec_count(struct inode *inode);
 
 #define S_SHIFT 12
 static const unsigned char ext4_type_by_mode[(S_IFMT >> S_SHIFT) + 1] = {
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 9c170986f0b2..3f864ebab1c8 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1824,8 +1824,15 @@ static int ext4_fc_replay_unlink(struct super_block *sb,
 
 	ret = __ext4_unlink(old_parent, &entry, inode, NULL);
 	/* -ENOENT ok coz it might not exist anymore. */
-	if (ret == -ENOENT)
+	if (ret == -ENOENT) {
 		ret = 0;
+	} else if (!ret && S_ISDIR(inode->i_mode)) {
+		/* Like ext4_rmdir(), drop all links of the dir and its ".." */
+		clear_nlink(inode);
+		ext4_mark_inode_dirty(NULL, inode);
+		ext4_dec_count(old_parent);
+		ext4_mark_inode_dirty(NULL, old_parent);
+	}
 	iput(old_parent);
 	iput(inode);
 	return ret;
@@ -1858,6 +1865,11 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
 		goto out;
 	}
 
+	/* Like ext4_mkdir(), count the new dir's ".." in its parent */
+	if (!ret && S_ISDIR(inode->i_mode)) {
+		ext4_inc_count(dir);
+		ext4_mark_inode_dirty(NULL, dir);
+	}
 	/* The replay allocator doesn't count blocks added to the dir */
 	if (!ret && !ext4_test_inode_flag(dir, EXT4_INODE_INLINE_DATA))
 		ext4_ext_replay_set_iblocks(dir);
@@ -2080,7 +2092,7 @@ static int ext4_fc_replay_create(struct super_block *sb,
 	ret = ext4_fc_replay_link_internal(sb, &darg, inode);
 	if (ret)
 		goto out;
-	set_nlink(inode, 1);
+	set_nlink(inode, S_ISDIR(inode->i_mode) ? 2 : 1);
 	ext4_mark_inode_dirty(NULL, inode);
 out:
 	iput(inode);
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 354fc2d22df1..7e58f3c609e1 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2758,7 +2758,7 @@ static int ext4_delete_entry(handle_t *handle,
  * for checking S_ISDIR(inode) (since the INODE_INDEX feature will not be set
  * on regular files) and to avoid creating huge/slow non-HTREE directories.
  */
-static void ext4_inc_count(struct inode *inode)
+void ext4_inc_count(struct inode *inode)
 {
 	inc_nlink(inode);
 	if (is_dx(inode) &&
@@ -2770,7 +2770,7 @@ static void ext4_inc_count(struct inode *inode)
  * If a directory had nlink == 1, then we should let it be 1. This indicates
  * directory has >EXT4_LINK_MAX subdirs.
  */
-static void ext4_dec_count(struct inode *inode)
+void ext4_dec_count(struct inode *inode)
 {
 	if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2)
 		drop_nlink(inode);

-- 
2.53.0



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

* Re: [PATCH 1/3] ext4: fix directory count after fast commit replay
  2026-11-11 23:02 ` [PATCH 1/3] ext4: fix directory count after fast commit replay Mani Tofigh via B4 Relay
@ 2026-11-12  0:13   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-12  0:13 UTC (permalink / raw)
  To: Mani Tofigh; +Cc: linux-ext4, tytso

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ext4: unaligned memory access on raw_fc_inode in
  ext4_fc_replay_inode()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] ext4: unpropagated parent lookup error in
  ext4_fc_replay_link_internal() [1]
--

commit da3b7bd74825f337c491fafdca323cce067546e9
Author: Mani Tofigh <mt4022@columbia.edu>

ext4: fix directory count after fast commit replay

This commit passes the inode mode to ext4_mark_inode_used() so that fast
commit replay increments bg_used_dirs_count when marking a directory inode
in use, and moves the ext4_iget() lookup in ext4_fc_replay_create() ahead
of ext4_mark_inode_used().

> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0cac890cf3702..0eac7be3f58c2 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1984,7 +1984,7 @@ static int ext4_fc_replay_inode(struct super_block *sb,
>  	ret = sync_dirty_buffer(iloc.bh);
>  	if (ret)
>  		goto out_brelse;
> -	ret = ext4_mark_inode_used(sb, ino);
> +	ret = ext4_mark_inode_used(sb, ino, le16_to_cpu(raw_fc_inode->i_mode));
>  	if (ret)
>  		goto out_brelse;
>  

[Severity: Medium]
Can dereferencing raw_fc_inode->i_mode trigger an unaligned memory access
fault on strict-alignment architectures?

Fast commit TLVs are packed contiguously without alignment padding, so an
earlier dentry tag with an odd-length filename places subsequent
EXT4_FC_TAG_INODE payloads and raw_fc_inode at an odd byte offset.
ext4_fc_replay_inode() already copies raw_fc_inode into the aligned
raw_inode buffer earlier in the function, so reading raw_inode->i_mode
instead avoids the unaligned load.

> @@ -2041,11 +2041,6 @@ static int ext4_fc_replay_create(struct super_block *sb,
>  	trace_ext4_fc_replay(sb, EXT4_FC_TAG_CREAT, darg.ino,
>  			darg.parent_ino, darg.dname_len);
>  
> -	/* This takes care of update group descriptor and other metadata */
> -	ret = ext4_mark_inode_used(sb, darg.ino);
> -	if (ret)
> -		goto out;
> -
>  	inode = ext4_iget(sb, darg.ino, EXT4_IGET_NORMAL);
>  	if (IS_ERR(inode)) {
>  		ext4_debug("inode %d not found.", darg.ino);
> @@ -2054,6 +2049,11 @@ static int ext4_fc_replay_create(struct super_block *sb,
>  		goto out;
>  	}
>  
> +	/* This takes care of update group descriptor and other metadata */
> +	ret = ext4_mark_inode_used(sb, darg.ino, inode->i_mode);
> +	if (ret)
> +		goto out;
> +
>  	if (S_ISDIR(inode->i_mode)) {
>  		/*
>  		 * If we are creating a directory, we need to make sure that the

[Severity: Critical]
This problem wasn't introduced by this patch, but does
ext4_fc_replay_link_internal() [1] silently return 0 when looking up
the parent directory fails?

fs/ext4/fast_commit.c:ext4_fc_replay_link_internal() {
	...
	int ret = 0;

	dir = ext4_iget(sb, darg->parent_ino, EXT4_IGET_NORMAL);
	if (IS_ERR(dir)) {
		ext4_debug("Dir with inode %d not found.", darg->parent_ino);
		dir = NULL;
		goto out;
	}
	...
out:
	if (dir)
		iput(dir);

	return ret;
}

When ext4_fc_replay_create() calls ext4_fc_replay_link_internal() and the
parent lookup fails, it receives 0 and proceeds to set i_nlink to 1 and
mark the inode dirty, leaving an unlinked inode and inconsistent metadata
instead of aborting recovery.

[1] https://sashiko.dev/#/bug/linux-f99925c6-c893-41e7-947b-8df2ac40e92c

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ext4-fc-replay-mkdir-v1-0-4b20be29aed8@columbia.edu?part=1

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

* Re: [PATCH 3/3] ext4: fix directory link counts after fast commit replay
  2026-11-11 23:02 ` [PATCH 3/3] ext4: fix directory link counts " Mani Tofigh via B4 Relay
@ 2026-11-12  0:16   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-12  0:16 UTC (permalink / raw)
  To: Mani Tofigh; +Cc: linux-ext4, tytso

> From: Mani Tofigh <mt4022@columbia.edu>
> 
> After a crash, the fast commit replay of a mkdir leaves the new
> directory with one link and its parent with one link too few.  e2fsck
> then reports:
> 
>   Inode 2 ref count is 3, should be 4.
>   Inode 14 ref count is 1, should be 2.
> 
> Fast commits don't track directories, so the replay of a CREAT tag has
> to redo what ext4_mkdir() does.  ext4_fc_replay_create() sets the
> link count of the new inode to 1 even for a directory, and nothing
> counts the new ".." in the parent.  The replay of an rmdir has the
> opposite problem.  __ext4_unlink() drops only one link of the directory
> and the parent keeps the link of the removed "..":
> 
>   Inode 2 ref count is 4, should be 3.
> 
> Give a new directory 2 links and count it in its parent once its entry
> is added.  When an UNLINK tag removes a directory, drop all its links
> and one link of the parent like ext4_rmdir() does.  Both halves have to
> go together since today a mkdir and an rmdir replayed in sequence
> cancel out.
> 
> With the fix, e2fsck doesn't find wrong link counts after the replay
> anymore.
> 
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ext4-fc-replay-mkdir-v1-0-4b20be29aed8@columbia.edu?part=3


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

* Re: [PATCH 2/3] ext4: fix directory i_blocks after fast commit replay
  2026-11-11 23:02 ` [PATCH 2/3] ext4: fix directory i_blocks " Mani Tofigh via B4 Relay
@ 2026-11-12  0:26   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-12  0:26 UTC (permalink / raw)
  To: Mani Tofigh; +Cc: linux-ext4, tytso

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] ext4: missing EXT4_INODE_EXTENTS check before
  ext4_ext_replay_set_iblocks()
- [High] ext4: ext4_ext_replay_set_iblocks() drops directory xattr
  blocks in i_blocks
- [Medium] ext4: skipped dir i_blocks recount on -EEXIST in
  ext4_fc_replay_link_internal()
--

commit bd9227cad3ab9ec91535ca3b97b88447eed97729
Author: Mani Tofigh <mt4022@columbia.edu>

ext4: fix directory i_blocks after fast commit replay

This commit recounts i_blocks via ext4_ext_replay_set_iblocks() for newly
initialized directories and parent directories modified during fast commit
replay.

> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0eac7be3f58c2..9c170986f0b21 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1858,6 +1858,9 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
>  		goto out;
>  	}
>  
> +	/* The replay allocator doesn't count blocks added to the dir */
> +	if (!ret && !ext4_test_inode_flag(dir, EXT4_INODE_INLINE_DATA))
> +		ext4_ext_replay_set_iblocks(dir);

[Severity: High]
Does calling ext4_ext_replay_set_iblocks(dir) lose external xattr block
accounting on dir->i_blocks?

ext4_ext_replay_set_iblocks() overwrites inode->i_blocks with only the
extent data and tree block count:

fs/ext4/extents.c:ext4_ext_replay_set_iblocks() {
    ...
out:
	inode->i_blocks = numblks << (inode->i_sb->s_blocksize_bits - 9);
	ext4_mark_inode_dirty(NULL, inode);
    ...
}

If dir already has an external xattr block (EXT4_I(dir)->i_file_acl) or
ea_inode blocks from an earlier transaction, replaying any link or file
creation in dir drops those blocks from dir->i_blocks.  Besides corrupting
on-disk i_blocks, later removing the directory or its xattrs frees
i_file_acl and underflows dir->i_blocks.

[Severity: Medium]
Does gating ext4_ext_replay_set_iblocks(dir) on !ret skip the i_blocks
recount if a previous replay crashed after __ext4_link() persisted the new
directory block and dirent, when the re-run of __ext4_link() returns
-EEXIST?

>  	ret = 0;
>  out:
>  	if (dir)
> @@ -2070,6 +2073,9 @@ static int ext4_fc_replay_create(struct super_block *sb,
>  			ret = 0;
>  			goto out;
>  		}
> +		/* The replay allocator doesn't count the new dir block */
> +		if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
> +			ext4_ext_replay_set_iblocks(inode);

[Severity: High]
Can this (and the matching check in ext4_fc_replay_link_internal()) call
ext4_ext_replay_set_iblocks() on an indirect-mapped directory that does not
have EXT4_INODE_EXTENTS set?

A directory will lack EXT4_INODE_EXTENTS if the filesystem was formatted
without the extents feature or if EXT4_EXTENTS_FL was cleared on the
directory via FS_IOC_SETFLAGS in an earlier transaction.  During fast
commit replay, __ext4_iget() skips extent header validation, and
ext4_ext_replay_set_iblocks() calls ext4_find_extent(..., EXT4_EX_NOCACHE),
which reinterprets the direct block numbers in EXT4_I(inode)->i_data[] as a
struct ext4_extent_header and binary-searches past i_data[] into adjacent
slab memory or corrupts i_blocks.

Checking ext4_test_inode_flag(..., EXT4_INODE_EXTENTS) as
ext4_fc_replay_inode() does would avoid calling into the extent tree
walker on non-extent directories.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-ext4-fc-replay-mkdir-v1-0-4b20be29aed8@columbia.edu?part=2

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

end of thread, other threads:[~2026-11-12  0:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-11 23:02 [PATCH 0/3] ext4: fix counts left by fast commit replay of a mkdir Mani Tofigh via B4 Relay
2026-11-11 23:02 ` [PATCH 1/3] ext4: fix directory count after fast commit replay Mani Tofigh via B4 Relay
2026-11-12  0:13   ` sashiko-bot
2026-11-11 23:02 ` [PATCH 2/3] ext4: fix directory i_blocks " Mani Tofigh via B4 Relay
2026-11-12  0:26   ` sashiko-bot
2026-11-11 23:02 ` [PATCH 3/3] ext4: fix directory link counts " Mani Tofigh via B4 Relay
2026-11-12  0:16   ` sashiko-bot

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