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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/7/20 05:38, David Sterba wrote:
> On Tue, Jul 19, 2022 at 04:56:00PM -0400, Sweet Tea Dorminy wrote:
>> On 7/19/22 01:11, Qu Wenruo wrote:
>>>
>>> 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?
>
> Splitting the message is IMHO wrong thing, there are other subysystems
> writing to the log so the lines can become scattered or interleaved with
> the same message from other threads.

But that one line output is really hard to read for human beings.

Or do you mean that, as long as it's debug info, we should not care
about readability at all?

Thanks,
Qu
>
> We should prefer single line messages if possible, if not only for
> better grep-ability, pretty printing can be done by any utility that
> parses the logs.
>
> I did not think about the rate limiting, but you're right that it could
> be problematic.