Re: [PATCH v2 1/4] btrfs: output human readable space info flag

[Sweet Tea Dorminy <sweettea-kernel@dorminy.me>]

On 7/19/22 01:11, Qu Wenruo wrote:
> For btrfs_space_info, its flags has only 4 possible values:
> - BTRFS_BLOCK_GROUP_SYSTEM
> - BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_DATA
> - BTRFS_BLOCK_GROUP_METADATA
> - BTRFS_BLOCK_GROUP_DATA
> 
> Thus do debuggers a favor by output a human readable flags in
> __btrfs_dump_space_info().
> 
> Now the summary line of __btrfs_dump_space_info() looks like:
> 
>   BTRFS info (device dm-1: state A): space_info META has 251494400 free, is not full
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>   fs/btrfs/space-info.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 62d25112310d..36b466525318 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -450,14 +450,29 @@ do {									\
>   	spin_unlock(&__rsv->lock);					\
>   } while (0)
>   
> +static const char *space_info_flag_to_str(struct btrfs_space_info *space_info)
> +{
> +	if (space_info->flags == BTRFS_BLOCK_GROUP_SYSTEM)
> +		return "SYS";
> +
> +	/* Handle mixed data+metadata first. */
> +	if (space_info->flags == (BTRFS_BLOCK_GROUP_METADATA |
> +				  BTRFS_BLOCK_GROUP_DATA))
> +		return "DATA+META";
> +	if (space_info->flags == BTRFS_BLOCK_GROUP_DATA)
> +		return "DATA";
> +	return "META";
> +}

I think this would be more elegant as a switch statement; and would 
prefer the default to be "UNKNOWN" just in case there were a flag 
bitflip.   +
>   static void __btrfs_dump_space_info(struct btrfs_fs_info *fs_info,
>   				    struct btrfs_space_info *info)
>   {
> +	const char *flag_str = space_info_flag_to_str(info);
>   	lockdep_assert_held(&info->lock);
>   
>   	/* The free space could be negative in case of overcommit */
> -	btrfs_info(fs_info, "space_info %llu has %lld free, is %sfull",
> -		   info->flags,
> +	btrfs_info(fs_info, "space_info %s has %lld free, is %sfull",
> +		   flag_str,
>   		   (s64)(info->total_bytes - btrfs_space_info_used(info, true)),
>   		   info->full ? "" : "not ");
>   	btrfs_info(fs_info,