[Crash-utility] [PATCH v3 0/7] log: output logs of printk safe buffers
lijiang
lijiang at redhat.com
Mon Feb 14 08:42:29 UTC 2022
Thank you for the patch, Shogo.
On Thu, Feb 10, 2022 at 4:25 PM <crash-utility-request at redhat.com> wrote:
> Date: Thu, 10 Feb 2022 06:38:32 +0000
> From: HAGIO KAZUHITO(?????) <k-hagio-ab at nec.com>
> To: "shogo.matsumoto at fujitsu.com" <shogo.matsumoto at fujitsu.com>
> Cc: "Discussion list for crash utility usage, maintenance and
> development" <crash-utility at redhat.com>
> Subject: Re: [Crash-utility] [PATCH v3 0/7] log: output logs of printk
> safe buffers
> Message-ID:
> <
> TYYPR01MB6777FE775F5CD6438FC2E448DD2F9 at TYYPR01MB6777.jpnprd01.prod.outlook.com
> >
>
> Content-Type: text/plain; charset="iso-2022-jp"
>
>
> -----Original Message-----
> > This patch set introduces -s option for log builtin command to display
> > printk safe buffers (safe_print_seq/nmi_print_seq) as follows:
> >
> > ===
> > crash> log -s
> > PRINTK_SAFE_SEQ_BUF: nmi_print_seq
> > CPU: 0 ADDR: ffff969d7bc19ce0 LEN: 150 MESSAGE_LOST: 0
> > Uhhuh. NMI received for unknown reason 20 on CPU 0.
> > Do you have a strange power saving mode enabled?
> > Dazed and confused, but trying to continue
> > ...
> > ===
> >
> > The printk safe buffers are also displayed at the bottom of
> > 'log' output so as not to overlook them.
> >
> > ===
> > crash> log
> > ...
> > [nmi_print_seq] Uhhuh. NMI received for unknown reason 20 on CPU 0.
> > [nmi_print_seq] Do you have a strange power saving mode enabled?
> > [nmi_print_seq] Dazed and confused, but trying to continue
> > ===
> >
> > -m and -t options are also supported.
> >
> > Note that the safe buffer (struct printk_safe_seq_buf) was introduced
> > in kernel-4.11 (Merge commit 7d91de74436a69c2b78a7a72f1e7f97f8b4396fa)
> > and removed in kernel-5.15 (93d102f094be9beab28e5afb656c188b16a3793b).
> >
> > Changes since v2:
> > - Add support new options -s, -t, -m (Kazu)
> > - Add help text (Kazu)
>
> Thank you for the update.
>
> Maybe I will join the patches into two or three and the following warning
> is emitted, so I will adjust a little when merging, but otherwise the
>
I would recommend packing them into two patches as below:
[PATCH v3 1/7] log: introduce -s option
[PATCH v3 2/7] log: adjust indent and line breaks for log -s
[PATCH v3 3/7] log: append printk safe buffer output to 'log'
[PATCH v3 6/7] symbols: add support 'help -o' for printk safe buffers
[PATCH v3 7/7] log: add help text for printk safe buffers
Another one:
[PATCH v3 4/7] log: add support -t option for output of printk safe buffers
[PATCH v3 5/7] log: add support -m for output of printk safe buffers
BTW: If there is a better way, you could rearrange them when merging.
Thanks.
And with the warning fix, otherwise:
Acked-by: Lianbo Jiang <lijiang at redhat.com>
> patchset and the output of the commands look nice to me!
>
> Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>
>
>
> $ make clean ; make warn
> ...
> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_10_2 kernel.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> -Wformat-security
> kernel.c: In function ?__dump_printk_safe_seq_buf?:
> kernel.c:11623:7: warning: format not a string literal and no format
> arguments [-Wformat-security]
> fprintf(fp, space(PRINTK_SAFE_SEQ_BUF_INDENT));
> ^~~~~~~
>
> Will add "%s".
>
> Thanks,
> Kazu
>
> >
> > [v1]:
> https://listman.redhat.com/archives/crash-utility/2021-December/msg00031.html
> > [v2]:
> https://listman.redhat.com/archives/crash-utility/2022-January/msg00004.html
> >
> > Test program is attached in the above v2 patch e-mail.
> >
> > Shogo Matsumoto (7):
> > log: introduce -s option
> > log: adjust indent and line breaks for log -s
> > log: append printk safe buffer output to 'log'
> > log: add support -t option for output of printk safe buffers
> > log: add support -m for output of printk safe buffers
> > symbols: add support 'help -o' for printk safe buffers
> > log: add help text for printk safe buffers
> >
> > defs.h | 5 ++
> > help.c | 25 ++++++++-
> > kernel.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > symbols.c | 5 ++
> > 4 files changed, 192 insertions(+), 2 deletions(-)
> >
> > --
> > 2.29.2
> >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility at redhat.com
> > https://listman.redhat.com/mailman/listinfo/crash-utility
>
>
>
>
> ------------------------------
>
> Message: 2
> Date: Thu, 10 Feb 2022 08:21:52 +0000
> From: HAGIO KAZUHITO(?????) <k-hagio-ab at nec.com>
> To: "Discussion list for crash utility usage, maintenance and
> development" <crash-utility at redhat.com>
> Cc: "zwang at amperecomputing.com" <zwang at amperecomputing.com>,
> "patches at amperecomputing.com" <patches at amperecomputing.com>,
> "lijiang at redhat.com" <lijiang at redhat.com>,
> "darren at os.amperecomputing.com" <darren at os.amperecomputing.com>
> Subject: Re: [Crash-utility] [PATCH] arm64: Use CONFIG_ARM64_VA_BITS
> to initialize VA_BITS_ACTUAL
> Message-ID:
> <
> TYYPR01MB6777479D629CD1F2A18A7A6DDD2F9 at TYYPR01MB6777.jpnprd01.prod.outlook.com
> >
>
> Content-Type: text/plain; charset="iso-2022-jp"
>
> Hi Huang,
>
> thanks for the patch.
>
> -----Original Message-----
> > For DISKDUMP case, we can get VA_BITS_ACTUAL from CONFIG_ARM64_VA_BITS.
>
> I could not understand this, there is a case where CONFIG_ARM64_VA_BITS
> is different from VA_BITS_ACTUAL and why is this only for DISKDUMP case?
>
> If the patch intends to guess the value of VA_BITS_ACTUAL to be the same as
> CONFIG_ARM64_VA_BITS when no NUMBER(TCR_EL1_T1SZ), I think that DISKDUMP
> check is not needed and it would be better to write such a commit log and
> a comment e.g. "/* guess */" on the else if block.
>
> Thanks,
> Kazu
>
> > Without this patch, we may have to use "--machdep vabits_actual=48" to
> > set the VA_BITS_ACTUAL.
> >
> > Signed-off-by: Huang Shijie <shijie at os.amperecomputing.com>
> > ---
> > arm64.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/arm64.c b/arm64.c
> > index 4f2c2b5..2b3ec02 100644
> > --- a/arm64.c
> > +++ b/arm64.c
> > @@ -4170,6 +4170,12 @@ arm64_calc_VA_BITS(void)
> > } else if (machdep->machspec->VA_BITS_ACTUAL) {
> > machdep->machspec->VA_BITS =
> machdep->machspec->VA_BITS_ACTUAL;
> > machdep->machspec->VA_START =
> _VA_START(machdep->machspec->VA_BITS_ACTUAL);
> > + } else if (pc->flags & DISKDUMP) {
> > + if
> (machdep->machspec->CONFIG_ARM64_VA_BITS) {
> > + machdep->machspec->VA_BITS_ACTUAL =
> > machdep->machspec->CONFIG_ARM64_VA_BITS;
> > + machdep->machspec->VA_BITS =
> > machdep->machspec->CONFIG_ARM64_VA_BITS;
> > + machdep->machspec->VA_START =
> > _VA_START(machdep->machspec->VA_BITS_ACTUAL);
> > + }
> > } else
> > error(FATAL, "cannot determine
> VA_BITS_ACTUAL\n");
> > }
> > --
> > 2.30.2
> >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility at redhat.com
> > https://listman.redhat.com/mailman/listinfo/crash-utility
>
>
>
>
> ------------------------------
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility
>
> End of Crash-utility Digest, Vol 197, Issue 7
> *********************************************
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20220214/18e69b41/attachment.htm>
More information about the Crash-utility
mailing list