Re: [PATCH] btrfs: abort transaction before releasing tree_log_mutex on commit failure

[jlayton@xxxxxxxx]

On Tue, 2026-08-18 at 17:40 -0700, Leo Martins wrote:
> When transaction metadata writeout fails in btrfs_commit_transaction(),
> the current code only logs the error, drops tree_log_mutex and then goes
> through cleanup_transaction(), which aborts the transaction and records
> the fs error.
> 
> That is too late for the tree log side. A log sync can already be
> waiting on tree_log_mutex, because the committing transaction is moved
> to TRANS_STATE_UNBLOCKED while that mutex is held, which lets fsyncs
> join the next transaction and queue up in btrfs_sync_log(). Once the
> failed commit drops tree_log_mutex, such a log sync acquires it, sees
> BTRFS_FS_ERROR() still clear, and writes super_for_commit. That
> superblock holds the roots prepared for the transaction that has just
> failed to write out its metadata, so it can point at tree blocks that
> never reached the disk, and the next mount fails with a parent transid
> mismatch.
> 
> Commit 165ea85f1483 ("btrfs: do not write supers if we have an fs
> error") fixed this class of problem by making btrfs_sync_log() check for
> an fs error right after taking tree_log_mutex. That check only works if
> the commit path publishes the fs error before it releases the same
> mutex, and commit 68d4ece9c30e ("btrfs: don't call
> btrfs_handle_fs_error() in btrfs_commit_transaction()") removed the only
> thing that did so.
> 
> Restore the ordering by aborting the transaction while tree_log_mutex is
> still held. We have a transaction handle here, so this does not need to
> bring back the btrfs_handle_fs_error() call: __btrfs_abort_transaction()
> records the fs error itself, which is all btrfs_sync_log() looks at, and
> the error message put in its place is kept.
> 
> This is what commit 3810ab40afa5 ("btrfs: abort transaction on error in
> write_all_supers()") already does for the next call in this function.
> 
> This is reproducible on an unmodified kernel by failing the first
> couple of bios of a transaction commit with fail_make_request while a
> concurrent fsync workload keeps log syncs queued on tree_log_mutex.
> 
> Fixes: 68d4ece9c30e ("btrfs: don't call btrfs_handle_fs_error() in btrfs_commit_transaction()")
> Cc: stable@xxxxxxxxxxxxxxx # 7.0+
> Signed-off-by: Leo Martins <loemra.dev@xxxxxxxxx>
> ---
>  fs/btrfs/transaction.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 13a7e5f4e08c..ed850bf0546a 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -2583,6 +2583,12 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
>  	ret = btrfs_write_and_wait_transaction(trans);
>  	if (unlikely(ret)) {
>  		btrfs_err(fs_info, "error while writing out transaction: %pe", ERR_PTR(ret));
> +		/*
> +		 * Abort before releasing tree_log_mutex, so a log sync waiting
> +		 * on it sees the fs error and skips writing super_for_commit
> +		 * for this failed transaction. See btrfs_sync_log().
> +		 */
> +		btrfs_abort_transaction(trans, ret);
>  		mutex_unlock(&fs_info->tree_log_mutex);
>  		goto scrub_continue;
>  	}

Reviewed-by: jlayton@xxxxxxxx <jlayton@xxxxxxxx>