Re: [PATCH RFC] btrfs: abort transaction directly on errors in commit path

Bartosz Chronowski <[email protected]> Thu, 23 Jul 2026 16:18:45 +0200
Newsgroups dev.linux.lists.syzbot
Message-ID <n6vtyhyk7dcb7zf3tko5grqqcfod7co6uxvzyyy47u5bffevzo@7d6qolowq4gh>
Aim for minimal patch:
btrfs_relocate_block_group() should call btrfs_commit_current_transaction(extent_root) 
after relocation starts and after the relocation control has the target block 
group, but before btrfs_inc_block_group_ro() marks that target block group read-only.

Check return value for the out: cleanup.
Iterate on patch development until the patch passes CI pipeline resulting in "Correct" 
status.

On Thu, Jul 09, 2026 at 07:08:55PM +0000, syzbot wrote:
> When a transaction commit fails due to filesystem corruption (-EUCLEAN),
> some functions like btrfs_qgroup_account_extents() and btrfs_update_root()
> return the error without explicitly calling btrfs_abort_transaction(). This
> forces the generic cleanup_transaction() fallback to abort the transaction,
> which masks the actual location of the failure in the stack trace.
> 
> This results in confusing warnings that do not point to the actual failure:
> 
> BTRFS warning (device loop0): Skipping commit of aborted transaction.
> [ cut here ]
> btrfs_abort_should_print_stack(__error)
> WARNING: fs/btrfs/transaction.c:2068 at cleanup_transaction+0x727/0x7c0
> ...
> Call Trace:
>  <TASK>
>  btrfs_commit_transaction+0x262c/0x30b0
>  prepare_to_relocate+0x3dd/0x4e0
>  relocate_block_group+0x141/0xe90
>  do_nonremap_reloc+0xa7/0x560
>  btrfs_relocate_block_group+0x6e2/0xaf0
>  btrfs_relocate_chunk+0x114/0x830
>  __btrfs_balance+0x1b6e/0x29e0
>  btrfs_balance+0xaa6/0x1180
>  btrfs_ioctl_balance+0x3dd/0x640
> 
> Fix this by calling btrfs_abort_transaction() directly in these functions
> when they encounter an error. For btrfs_update_root(), -ENOSPC is a normal
> error when updating the log root tree (e.g., during btrfs_sync_log()), so
> we only abort if it is not the log root tree or the error is not -ENOSPC.
> 
> Additionally, btrfs_abort_transaction() triggers a WARN_ON for -EUCLEAN and
> -ENOSPC because they are not in the ignore list of
> btrfs_abort_should_print_stack(). WARN_ON must not be used for conditions
> that can legitimately happen, such as encountering a corrupted filesystem
> image (-EUCLEAN) or running out of space (-ENOSPC). To prevent unnecessary
> warnings, add -EUCLEAN and -ENOSPC to the ignore list. To ensure we still
> get meaningful stack traces for debugging, add manual dump_stack() calls in
> __btrfs_abort_transaction() for these specific errors.
> 
> Fixes: 72bd2323ec87 ("Btrfs: do not abort transaction at btrfs_update_root() after failure to COW path")
> Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview syzbot
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=021d10c4d4edc87daa03
> Link: https://syzkaller.appspot.com/ai_job?id=36e1fafd-13a6-44c6-a988-98fb6981cb2b
> To: "Chris Mason" <[email protected]>
> To: "David Sterba" <[email protected]>
> To: <[email protected]>
> To: "Filipe Manana" <[email protected]>
> Cc: <[email protected]>
> 
> ---
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 502fb4a55..1870595f1 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -3068,8 +3068,10 @@ int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans)
>  			if (!record->old_roots) {
>  				/* Search commit root to find old_roots */
>  				ret = btrfs_find_all_roots(&ctx, false);
> -				if (ret < 0)
> +				if (ret < 0) {
> +					btrfs_abort_transaction(trans, ret);
>  					goto cleanup;
> +				}
>  				record->old_roots = ctx.roots;
>  				ctx.roots = NULL;
>  			}
> @@ -3082,8 +3084,10 @@ int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans)
>  			ctx.trans = trans;
>  			ctx.time_seq = BTRFS_SEQ_LAST;
>  			ret = btrfs_find_all_roots(&ctx, false);
> -			if (ret < 0)
> +			if (ret < 0) {
> +				btrfs_abort_transaction(trans, ret);
>  				goto cleanup;
> +			}
>  			new_roots = ctx.roots;
>  			if (qgroup_to_skip) {
>  				ulist_del(new_roots, qgroup_to_skip, 0);
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 90659b287..13695e4f4 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -142,8 +142,17 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
>  		return -ENOMEM;
>  
>  	ret = btrfs_search_slot(trans, root, key, path, 0, 1);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		/*
> +		 * Abort the transaction if we are not updating the log root tree.
> +		 * In btrfs_sync_log, -ENOSPC is a normal error and should not
> +		 * abort the transaction.
> +		 */
> +		if (ret != -ENOSPC ||
> +		    root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID)
> +			btrfs_abort_transaction(trans, ret);
>  		return ret;
> +	}
>  
>  	if (unlikely(ret > 0)) {
>  		btrfs_crit(fs_info,
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 8f9419728..aa53eb831 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -2751,8 +2751,16 @@ void __cold __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
>  	if (first_hit) {
>  		btrfs_err(fs_info, "Transaction %llu aborted (error %d)",
>  			  trans->transid, error);
> -		if (error == -ENOSPC)
> +		if (error == -ENOSPC) {
>  			btrfs_dump_space_info_for_trans_abort(fs_info);
> +			dump_stack();
> +		}
> +		if (error == -EUCLEAN) {
> +			btrfs_err(
> +				fs_info,
> +				"Aborting transaction due to filesystem corruption");
> +			dump_stack();
> +		}
>  	}
>  	/* Wake up anybody who may be waiting on this transaction */
>  	wake_up(&fs_info->transaction_wait);
> diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
> index 5e4b1106f..ec66a60cc 100644
> --- a/fs/btrfs/transaction.h
> +++ b/fs/btrfs/transaction.h
> @@ -237,6 +237,8 @@ static inline bool btrfs_abort_should_print_stack(int error)
>  	case -EIO:
>  	case -EROFS:
>  	case -ENOMEM:
> +	case -EUCLEAN:
> +	case -ENOSPC:
>  		return false;
>  	}
>  	return true;
> 
> 
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
> -- 
> This is an AI-generated patch subject to moderation.
> Reply with '#syz upstream' to Sign-off the patch as a human author
> and send it to the upstream kernel mailing lists.
> Reply with '#syz reject' to reject it ('#syz unreject' to undo).
> 
> See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
> You can comment on the patch as usual, syzbot will try to address
> the comments and send a new version of the patch if necessary.
> syzbot engineers can be reached at [email protected].