Re: [PATCH] jbd2: don't start a fast commit while the journal is marked empty

[Zhang Yi]

On 10/7/2026 8:41 AM, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@xxxxxxxxxxx>
> 
> jbd2_journal_flush() checkpoints everything, and
> jbd2_mark_journal_empty() then writes the journal superblock with
> s_start == 0 and the fast commit feature cleared. It sets JBD2_FLUSHED
> so that the next commit writes the log tail and the feature back, but
> only jbd2_journal_commit_transaction() looks at that flag. A fast commit
> started before the next full commit writes its blocks while the
> superblock on disk still says the log is empty: jbd2_journal_recover()
> returns at once and the fast commit is never replayed.
> 
> On ext4 with -O fast_commit this happens after ext4_freeze(), and after
> a remount to read-only, which flushes the journal in
> ext4_mark_recovery_complete(), once the file system is read-write again.
> EXT4_IOC_CHECKPOINT and online resize flush it as well. An fsync that a
> fast commit handles after one of them is lost if the system crashes
> before the next full commit, which can be up to the commit interval
> later:
> 
>   mount -o commit=60 (-O fast_commit file system)
>   echo first > a; sync
>   fsfreeze -f; fsfreeze -u      # or: remount ro, then rw
>   write b; fsync b              # one fast commit, s_start still 0
>   crash (kill the VM), mount
>   b is missing
> 
> Refuse to start a fast commit while JBD2_FLUSHED is set, as is already
> done before the first full commit. ext4 then falls back to a full
> commit, which writes the log tail, and the fast commits after it are
> recovered again. fc_info counts the refused fast commit as ineligible,
> as it does for the existing check.
> 
> Fixes: ff780b91efe9 ("jbd2: add fast commit machinery")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Daejun Park <daejun7.park@xxxxxxxxxxx>

This makes sense to me!

Reviewed-by: Zhang Yi <yi.zhang@xxxxxxxxxx>

> ---
> Tested in QEMU on ext4 dev 9091c97be340, crashing by killing QEMU: the
> sequence above loses b without this change and keeps it with it, both
> after fsfreeze and after a remount ro then rw. A second fsync after the
> fallback is a fast commit again and is recovered. xfstests ext4/044
> ext4/045 generic/455 generic/456 generic/482 with -O fast_commit, and
> generic/068 generic/085 generic/390, give the same results as without
> the change (generic/455 fails on both).
> ---
>  fs/jbd2/journal.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 00f5a98f3d..22576af8c8 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -695,7 +695,8 @@ int jbd2_log_wait_commit(journal_t *journal, tid_t tid)
>   * it to complete. Returns 0 if a new fast commit was started. Returns -EALREADY
>   * if a fast commit is not needed, either because there's an already a commit
>   * going on or this tid has already been committed. Returns -EINVAL if no jbd2
> - * commit has yet been performed.
> + * commit has yet been performed, or none since the journal was last marked
> + * empty.
>   */
>  int jbd2_fc_begin_commit(journal_t *journal, tid_t tid)
>  {
> @@ -714,6 +715,16 @@ int jbd2_fc_begin_commit(journal_t *journal, tid_t tid)
>  		return -EALREADY;
>  	}
>  
> +	/*
> +	 * The log was emptied and the superblock on disk says so, which makes
> +	 * recovery skip the fast commit area as well. Only a full commit
> +	 * records the log tail again.
> +	 */
> +	if (journal->j_flags & JBD2_FLUSHED) {
> +		write_unlock(&journal->j_state_lock);
> +		return -EINVAL;
> +	}
> +
>  	if (journal->j_flags & JBD2_FULL_COMMIT_ONGOING ||
>  	    (journal->j_flags & JBD2_FAST_COMMIT_ONGOING)) {
>  		DEFINE_WAIT(wait);
> 
> ---
> base-commit: 9091c97be34083587a75db174aab51551d8e8543
> change-id: 20261006-jbd2-fc-after-flush-2ef9bbc32cea
> 
> Best regards,