From: Petr Mladek <pmladek@suse.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Dmitry Vyukov <dvyukov@google.com>,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] printk: Add caller information to printk() output.
Date: Mon, 3 Dec 2018 16:06:03 +0100 [thread overview]
Message-ID: <20181203150603.cdqii263e4kmmibo@pathway.suse.cz> (raw)
In-Reply-To: <f224bd68-7394-ff70-cad6-d3fbbb3b5f7d@i-love.sakura.ne.jp>
On Sat 2018-12-01 23:44:37, Tetsuo Handa wrote:
> On 2018/12/01 0:40, Petr Mladek wrote:
> >> Some examples for console output:
> >>
> >> [ 0.293000] [T1] smpboot: CPU0: Intel(R) Core(TM) i5-4440S CPU @ 2.80GHz (family: 0x6, model: 0x3c, stepping: 0x3)
> >> [ 0.299733] [T1] Performance Events: Haswell events, core PMU driver.
> >> [ 2.813808] [T35] clocksource: Switched to clocksource tsc
> >> [ 2.893984] [C0] random: fast init done
> > ^
> >
> > Please, remove the space between the timestamp and the from field.
>
> This space was emitted by print_time(). Do we want to modify print_time()
> not to emit this space if the from field is printed?
Exactly. This is what I thought about.
> If we modify print_time(), I think that the leading spaces inserted by "%5lu"
> makes little sense, for "%5lu" is too small for systems with uptime >= 1.16 days
> and parsers after all cannot assume fixed length for the timestamp field. Then,
> we could change from "%5lu.%06lu" to "%lu.%06lu" so that parsers (like /bin/awk)
> can get prefix part using white spaces as a delimiter.
My primary concern was a human readability. The different header columns
are separated by brackets and the message itself is separated by the space.
awk could easily use \[ as the separator.
But you made a good point about the column width. The text might be
hard to read when every line of text starts on a different column. And
the might be bigger differences for the task id. It might be useful to
add some reasonable default width also for the for the "from_id" column.
> If we want to reduce space, do we want to do like
>
> [0.293000@T1] smpboot: CPU0: Intel(R) Core(TM) i5-4440S CPU @ 2.80GHz (family: 0x6, model: 0x3c, stepping: 0x3)
> [0.299733@T1] Performance Events: Haswell events, core PMU driver.
> [2.813808@T35] clocksource: Switched to clocksource tsc
> [2.893984@C0] random: fast init done
Hmm, this is pretty hard to parse by my eyes. Also it changes the format
of the timestamp column.
I think that the following might give the best human user experience:
[ 0.293000][ T1] smpboot: CPU0: Intel(R) Core(TM) i5-4440S CPU @ 2.80GHz (family: 0x6, model: 0x3c, stepping: 0x3)
[ 0.299733][ T1] Performance Events: Haswell events, core PMU driver.
[ 2.813808][ T35] clocksource: Switched to clocksource tsc
[ 2.893984][ C0] random: fast init done
> >> @@ -1037,6 +1054,9 @@ void log_buf_vmcoreinfo_setup(void)
> >> VMCOREINFO_OFFSET(printk_log, len);
> >> VMCOREINFO_OFFSET(printk_log, text_len);
> >> VMCOREINFO_OFFSET(printk_log, dict_len);
> >> +#ifdef CONFIG_PRINTK_FROM
> >> + VMCOREINFO_OFFSET(printk_log, from_id);
> >> +#endif
> >
> > The crash tool would need to be updated if anyone wanted to read
> > the log from the extended structure. Well, it might be done later
> > if people start using it more widely.
>
> Since syzbot can utilize output from only normal consoles, I can
> keep extended records unmodified for now.
Please, add VMCOREINFO_OFFSET(printk_log, from_id) so that crashdump
can be updated when necessary.
> >
> > I think about adding one more filed "u8 version". It would help
> > to solve the external compatibility in the long term.
>
> /dev/kmsg format allows adding more fields, but that format did not define
> how to tell what fields are there. If fields are conditionally added by
> kernel config options, I don't think that "u8 version" field helps.
> Unless we add fields unconditionally, we will need to use $name=$value
> (where $name and $value must not contain ',' and ';') representation.
/dev/kmsg uses key=value notation. It does not need any version. The
version filed was intended for crashdump. It would make the life
easier for its maintainers.
Well, I do not resist on it. Let's put the version field aside for now.
Best Regards,
Petr
next prev parent reply other threads:[~2018-12-03 15:06 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-24 7:37 [PATCH] printk: Add caller information to printk() output Tetsuo Handa
2018-11-30 15:40 ` Petr Mladek
2018-12-01 14:44 ` Tetsuo Handa
2018-12-02 11:23 ` Tetsuo Handa
2018-12-04 2:02 ` Sergey Senozhatsky
2018-12-04 10:16 ` Tetsuo Handa
2018-12-04 10:38 ` Sergey Senozhatsky
2018-12-04 15:31 ` Petr Mladek
2018-12-03 15:06 ` Petr Mladek [this message]
2018-12-03 21:10 ` Tetsuo Handa
2018-12-04 15:27 ` Petr Mladek
2018-12-05 10:42 ` Tetsuo Handa
2018-12-05 11:50 ` Sergey Senozhatsky
2018-12-07 4:58 ` Tetsuo Handa
2018-12-07 5:31 ` Sergey Senozhatsky
2018-12-10 13:09 ` Petr Mladek
2018-12-10 14:01 ` Tetsuo Handa
2018-12-11 10:26 ` Tetsuo Handa
2018-12-12 2:25 ` Sergey Senozhatsky
2018-12-12 2:29 ` Sergey Senozhatsky
2018-12-13 12:18 ` Petr Mladek
2018-12-13 12:42 ` Sergey Senozhatsky
2018-12-17 14:54 ` Petr Mladek
2018-12-17 15:40 ` Sergey Senozhatsky
2018-12-17 21:05 ` Tetsuo Handa
2018-12-18 8:39 ` Petr Mladek
2018-12-18 8:58 ` Sergey Senozhatsky
2019-01-02 16:09 ` Dmitry Vyukov
2019-01-03 18:27 ` Dmitry Vyukov
2019-01-04 7:33 ` Fengguang Wu
2019-01-11 19:34 ` Kevin Hilman
2019-01-10 11:27 ` Tetsuo Handa
2018-12-18 8:55 ` Sergey Senozhatsky
2018-12-18 10:01 ` Petr Mladek
2018-12-18 10:10 ` Sergey Senozhatsky
2019-03-21 2:59 ` Michael Ellerman
2019-03-21 10:20 ` Petr Mladek
2019-03-22 0:48 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181203150603.cdqii263e4kmmibo@pathway.suse.cz \
--to=pmladek@suse.com \
--cc=akpm@linux-foundation.org \
--cc=dvyukov@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=sergey.senozhatsky@gmail.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).