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

Boris Burkov <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
On Tue, Aug 18, 2026 at 05:40:10PM -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: [email protected] # 7.0+

Reviewed-by: Boris Burkov <[email protected]>

> Signed-off-by: Leo Martins <[email protected]>
> ---
>  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;
>  	}
> -- 
> 2.53.0-Meta
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.