mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] ext4: don't fail journal recovery on an incomplete fast commit
@ 2026-10-07  4:08 Hsiu-Hsien Lee
  2026-10-07  6:56 ` Jan Kara
  0 siblings, 1 reply; 2+ messages in thread
From: Hsiu-Hsien Lee @ 2026-10-07  4:08 UTC (permalink / raw)
  To: Theodore Ts'o, linux-ext4
  Cc: Jan Kara, Andreas Dilger, Harshad Shirwadkar, Baokun Li,
	Ojaswin Mujoo, Ritesh Harjani, Zhang Yi, linux-kernel,
	Hsiu-Hsien Lee

If the system crashes while the first fast commit after a full commit is
being written, the fast commit area can hold the head of that fast
commit without a valid tail.  ext4_fc_replay_scan() treats this as an
error as long as no valid tail has been seen: an invalid tag length or
an unknown tag returns -ECANCELED, a tail with a wrong tid or checksum
returns -EFSBADCRC.  jbd2_journal_recover() then fails, the file system
can be mounted neither read-write nor read-only, and the regular journal
transactions committed before the fast commit are not replayed either:

  JBD2: journal recovery failed
  EXT4-fs (sdb): error loading journal

The tail is written last and fsync() does not return before it has
completed, so an incomplete fast commit was never reported as durable.
Handle it like jbd2 handles a transaction without a valid commit block:
stop the scan and replay what is valid.  This is already what happens
when an earlier fast commit in the area has a valid tail.

Reproducer, with the power cut emulated by copying the device while it
is mounted:

  mkfs.ext4 -O fast_commit /dev/sdb
  mount /dev/sdb /mnt; mkdir /mnt/d
  create 20 files in /mnt/d; sync
  write a fragmented file and modify the 20 files, fsync one file
      (one fast commit spanning several blocks)
  copy /dev/sdb while still mounted, then zero the block holding the
  fast commit tail in the copy
  mount the copy

Without this patch the mount fails as above.  With it, the mount
succeeds, the state after sync is recovered and e2fsck finds no errors.
The same holds when a full commit precedes the torn fast commit, in
which case the full commit is now replayed as well.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Assisted-by: Claude Opus 5.5
Signed-off-by: Hsiu-Hsien Lee <swinds24@gmail.com>
---
Changes in v2:
- Drop the warning and the helper, use ext4_debug() and set the return
  value inline (Jan Kara)
- v1: https://lore.kernel.org/linux-ext4/20261006070005.1209234-1-swinds24@gmail.com/

Testing: re-tested on 6.6.y with the same crash images used for v1 and
for the unpatched baseline (identical files).  Without the patch, all
torn first-fast-commit variants fail to mount rw and ro with "JBD2:
journal recovery failed".  With v2, all of them mount rw and ro, the
state of the last full commit is recovered and e2fsck -fn is clean.
Images with intact fast commits, or with a torn fast commit after a
valid one, behave the same with and without the patch.  The tail was
torn by zeroing whole blocks, so only the invalid tag length path was
exercised.

 fs/ext4/fast_commit.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 0cac890cf370..165950e2b7d9 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -2443,6 +2443,12 @@ static bool ext4_fc_value_len_isvalid(struct ext4_sb_info *sbi,
  * It returns a negative error to indicate that there was an error. At the end
  * of a successful scan phase, sbi->s_fc_replay_state.fc_replay_num_tags is set
  * to indicate the number of tags that need to replayed during the replay phase.
+ *
+ * An invalid tag or a tail that does not match ends the scan without an error.
+ * This is what a fast commit that was only partially written before a crash
+ * looks like.  Its tail is written last and fsync() does not return before
+ * the tail is on disk, so such a fast commit was never reported as durable and
+ * is simply dropped, like jbd2 drops a transaction without a commit block.
  */
 static int ext4_fc_replay_scan(journal_t *journal,
 				struct buffer_head *bh, int off,
@@ -2489,8 +2495,9 @@ static int ext4_fc_replay_scan(journal_t *journal,
 		val = cur + EXT4_FC_TAG_BASE_LEN;
 		if (tl.fc_len > end - val ||
 		    !ext4_fc_value_len_isvalid(sbi, tl.fc_tag, tl.fc_len)) {
-			ret = state->fc_replay_num_tags ?
-				JBD2_FC_REPLAY_STOP : -ECANCELED;
+			ext4_debug("Scan phase, invalid tag length, blk %lld\n",
+				   bh->b_blocknr);
+			ret = JBD2_FC_REPLAY_STOP;
 			goto out_err;
 		}
 		ext4_debug("Scan phase, tag:%s, blk %lld\n",
@@ -2530,8 +2537,9 @@ static int ext4_fc_replay_scan(journal_t *journal,
 				state->fc_regions_valid =
 					state->fc_regions_used;
 			} else {
-				ret = state->fc_replay_num_tags ?
-					JBD2_FC_REPLAY_STOP : -EFSBADCRC;
+				ext4_debug("Scan phase, invalid tail, blk %lld\n",
+					   bh->b_blocknr);
+				ret = JBD2_FC_REPLAY_STOP;
 			}
 			state->fc_crc = 0;
 			break;
@@ -2551,8 +2559,9 @@ static int ext4_fc_replay_scan(journal_t *journal,
 				EXT4_FC_TAG_BASE_LEN + tl.fc_len);
 			break;
 		default:
-			ret = state->fc_replay_num_tags ?
-				JBD2_FC_REPLAY_STOP : -ECANCELED;
+			ext4_debug("Scan phase, unknown tag %d, blk %lld\n",
+				   tl.fc_tag, bh->b_blocknr);
+			ret = JBD2_FC_REPLAY_STOP;
 		}
 		if (ret < 0 || ret == JBD2_FC_REPLAY_STOP)
 			break;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-07  6:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-07  4:08 [PATCH v2] ext4: don't fail journal recovery on an incomplete fast commit Hsiu-Hsien Lee
2026-10-07  6:56 ` Jan Kara

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®