Re: [PATCH] btrfs: include root in error message when unlinking inode

[Qu Wenruo <wqu@suse.com>]

在 2025/5/23 00:49, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> To help debugging include the root number in the error message, and since
> this is a critical error that implies a metadata inconsistency and results
> in a transaction abort change the log message level from "info" to
> "critical", which is a much better fit.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Just a small nitpick which can be addressed at merge time.> ---
>   fs/btrfs/inode.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 7d27875600d6..161a19316dfa 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4250,9 +4250,9 @@ static int __btrfs_unlink_inode(struct btrfs_trans_handle *trans,
>   
>   	ret = btrfs_del_inode_ref(trans, root, name, ino, dir_ino, &index);
>   	if (ret) {
> -		btrfs_info(fs_info,
> -			"failed to delete reference to %.*s, inode %llu parent %llu",
> -			name->len, name->name, ino, dir_ino);
> +		btrfs_crit(fs_info,
> +	   "failed to delete reference to %.*s, inode %llu parent %llu root %llu",

For most error messages involving both root and ino, we put root id 
before inode.

E.g. checksum verification in inode.c, zstd.c, lzo.c etc.

Thanks,
Qu

> +			   name->len, name->name, ino, dir_ino, btrfs_root_id(root));
>   		btrfs_abort_transaction(trans, ret);
>   		goto err;
>   	}