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

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

On 7/26/22 21:44, Qu Wenruo wrote:
> 
> 
> On 2022/7/27 09:21, Sweet Tea Dorminy wrote:
>>
>>
>> On 7/26/22 19:21, Boris Burkov wrote:
>>> On Tue, Jul 26, 2022 at 11:39:28PM +0200, David Sterba wrote:
>>>> On Tue, Jul 26, 2022 at 01:53:33PM -0700, Boris Burkov wrote:
>>>>>> Yes we shold care about readability but kernel printk output lines 
>>>>>> can
>>>>>> be interleaved, single line is much easier to grep for and all the
>>>>>> values are from one event. The format where it's a series of
>>>>>> "key=value"
>>>>>> is common and I think we're used to it from tracepoints too. There 
>>>>>> are
>>>>>> lines that do not put "=" between keys and values we could unify that
>>>>>> eventually.
>>>>>
>>>>> Agreed that a long line is OK, and preferable to full on splitting.
>>>>>
>>>>> What about making some btrfs printing macros that use KERN_CONT? I
>>>>> think
>>>>> that would do what Qu wants without splitting the lines or being bad
>>>>> for
>>>>> ratelimiting.
>>>>
>>>> IIRC I've read some discussions about KERN_CONT suggesting not to use
>>>> it, I'll ask what's the status.
>>>
>>> I just saw a comment at its definition that reads:
>>>
>>> /*
>>>   * Annotation for a "continued" line of log printout (only done after a
>>>   * line that had no enclosing \n). Only to be used by core/arch code
>>>   * during early bootup (a continued line is not SMP-safe otherwise).
>>>   */
>>> #define KERN_CONT       KERN_SOH "c"
>>>
>>> So that's not an encouraging sign. OTOH, I found some code in
>>> ext4/super.c that prints its errors with KERN_CONT here:
>>> 'ext4: super.c: Update logging style using KERN_CONT
>>
>> Some other log message from somewhere else could be emitted to the
>> printk ringbuffer between the original and the continued message. In
>> such a case, the continued message instead gets treated as its own
>> message of loglevel default. (kernel/printk/printk.c:2173ish) Using
>> KERN_CONT seems like it has a lot of potential for confusion, especially
>> if the default message level has been changed to be different from the
>> original messages' level.
> 
> Thanks for all the discussion, it looks like the current long single
> line is the way to go (in fact, the space info dumping itself is still
> two lines, and we may want to fix it).
> 
> Although it's not that human readable, the racy nature of message output
> is indeed a concern.
> This also means the old DUMP_BLOCK_RSV() function calls are not safe 
> either.
> 
> 
> But on the other hand, what if we output one line with multiple '\n'?
> Would it keep things readable while still count as one single line?

Yes, it would count as only one message, but it would break scripts that 
assume dmesg etc outputs a timestamp for every line; I think many kernel 
log scrapers make that assumption.

sample dmesg -T

[Wed Jul 27 10:59:54 2022] test three
                            line message

[Wed Jul 27 11:03:50 2022] next message.

sample journalctl:

Jul 27 10:59:54 vmcentral unknown: test three
                            line message
Jul 27 11:03:50 vmcentral unknown: next message.