Re: [PATCH v2 2/4] btrfs: make __btrfs_dump_space_info() output better formatted

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

On 7/19/22 01:11, Qu Wenruo wrote:
> The format change includes:
> 
> - Output each bytes_* in a separate line
> 
> - All bytes_* output starts at the same vertical position
>    Do human a favor reading the numbers
> 
> - Skip zone specific numbers if zone is not enabled
> 
> Now one example of __btrfs_dump_space_info() looks like this for its
> bytes_* members.
> 
>   BTRFS info (device dm-1: state A): space_info META has 251494400 free, is not full
>   BTRFS info (device dm-1: state A):   total:         268435456
>   BTRFS info (device dm-1: state A):   used:          376832
>   BTRFS info (device dm-1: state A):   pinned:        229376
>   BTRFS info (device dm-1: state A):   reserved:      0
>   BTRFS info (device dm-1: state A):   may_use:       16269312
>   BTRFS info (device dm-1: state A):   read_only:     65536
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>   fs/btrfs/space-info.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 36b466525318..623fa0488545 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -475,11 +475,15 @@ static void __btrfs_dump_space_info(struct btrfs_fs_info *fs_info,
>   		   flag_str,
>   		   (s64)(info->total_bytes - btrfs_space_info_used(info, true)),
>   		   info->full ? "" : "not ");
> -	btrfs_info(fs_info,
> -		"space_info total=%llu, used=%llu, pinned=%llu, reserved=%llu, may_use=%llu, readonly=%llu zone_unusable=%llu",
> -		info->total_bytes, info->bytes_used, info->bytes_pinned,
> -		info->bytes_reserved, info->bytes_may_use,
> -		info->bytes_readonly, info->bytes_zone_unusable);
> +	btrfs_info(fs_info, "  total:         %llu", info->total_bytes);
> +	btrfs_info(fs_info, "  used:          %llu", info->bytes_used);
> +	btrfs_info(fs_info, "  pinned:        %llu", info->bytes_pinned);
> +	btrfs_info(fs_info, "  reserved:      %llu", info->bytes_reserved);
> +	btrfs_info(fs_info, "  may_use:       %llu", info->bytes_may_use);
> +	btrfs_info(fs_info, "  read_only:     %llu", info->bytes_readonly);
> +	if (btrfs_is_zoned(fs_info))
> +		btrfs_info(fs_info,
> +			    "  zone_unusable: %llu", info->bytes_zone_unusable);

I'm (perhaps needlessly) worried about splitting this up into six/seven 
messages, because of the ratelimiting rolled into btrfs_printk. The 
ratelimit is 100 messages per 5 * HZ, and it seems like it would be 
unfortunate if it kicked in during the middle of this dump and prevented 
later info from being dumped.

Maybe we should add a btrfs_dump_printk() helper that doesn't have a 
ratelimit built in, for exceptional cases like this where we really, 
really don't want anything ratelimited?


>   
>   	DUMP_BLOCK_RSV(fs_info, global_block_rsv);
>   	DUMP_BLOCK_RSV(fs_info, trans_block_rsv);