Hi Matsumoto-san,

> -----Original Message-----
> Hi Lianbo,
> 
> 
> 
> >I would recommend packing them into two patches as below:
> 
> >BTW: If there is a better way, you could rearrange them when merging. Thanks.
> 
> >And with the warning fix, otherwise:
> 
> 
> 
> Thank you for your review. I will send a new patch with these fixes.

There is no need to resend for now, I will rearrange them when merging.
(probably Lianbo said it to me.)

Thanks,
Kazu


> 
> 
> 
> Thanks,
> 
> Shogo Matsumoto
> 
> 
> 
> 
> 
> Thank you for the patch, Shogo.
> 
> 
> 
> On Thu, Feb 10, 2022 at 4:25 PM <[email protected]
> <mailto:[email protected]> > wrote:
> 
>       Date: Thu, 10 Feb 2022 06:38:32 +0000
>       From: HAGIO KAZUHITO(?????)     <[email protected] 
> <mailto:[email protected]> >
>       To: "[email protected] <mailto:[email protected]> "
> <[email protected] <mailto:[email protected]> >
>       Cc: "Discussion list for crash utility usage,   maintenance and
>               development" <[email protected] 
> <mailto:[email protected]> >
>       Subject: Re: [Crash-utility] [PATCH v3 0/7] log: output logs of printk
>               safe buffers
>       Message-ID:
>               
> <tyypr01mb6777fe775f5cd6438fc2e448dd...@tyypr01mb6777.jpnprd01.prod.outlook.com
> <mailto:tyypr01mb6777fe775f5cd6438fc2e448dd...@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 <[email protected] <mailto:[email protected]> >
> 
> 
> 
>       patchset and the output of the commands look nice to me!
> 
>       Acked-by: Kazuhito Hagio <[email protected] 
> <mailto:[email protected]> >
> 
> 
>       $ 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
> <https://listman.redhat.com/archives/crash-utility/2021-December/msg00031.html>
>       > [v2]: 
> https://listman.redhat.com/archives/crash-utility/2022-January/msg00004.html
> <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
>       > [email protected] <mailto:[email protected]>
>       > https://listman.redhat.com/mailman/listinfo/crash-utility
> <https://listman.redhat.com/mailman/listinfo/crash-utility>
> 
> 
> 
> 
>       ------------------------------
> 
>       Message: 2
>       Date: Thu, 10 Feb 2022 08:21:52 +0000
>       From: HAGIO KAZUHITO(?????)     <[email protected] 
> <mailto:[email protected]> >
>       To: "Discussion list for crash utility usage,   maintenance and
>               development"    <[email protected] 
> <mailto:[email protected]> >
>       Cc: "[email protected] <mailto:[email protected]> " 
> <[email protected]
> <mailto:[email protected]> >,
>               "[email protected] 
> <mailto:[email protected]> "
> <[email protected] <mailto:[email protected]> >,
>               "[email protected] <mailto:[email protected]> " 
> <[email protected]
> <mailto:[email protected]> >,
>               "[email protected] 
> <mailto:[email protected]> "
> <[email protected] <mailto:[email protected]> >
>       Subject: Re: [Crash-utility] [PATCH] arm64: Use CONFIG_ARM64_VA_BITS
>               to      initialize VA_BITS_ACTUAL
>       Message-ID:
>               
> <tyypr01mb6777479d629cd1f2a18a7a6ddd...@tyypr01mb6777.jpnprd01.prod.outlook.com
> <mailto:tyypr01mb6777479d629cd1f2a18a7a6ddd...@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 <[email protected]
> <mailto:[email protected]> >
>       > ---
>       >  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
>       > [email protected] <mailto:[email protected]>
>       > https://listman.redhat.com/mailman/listinfo/crash-utility
> <https://listman.redhat.com/mailman/listinfo/crash-utility>
> 
> 
> 
> 
>       ------------------------------
> 
>       --
>       Crash-utility mailing list
>       [email protected] <mailto:[email protected]>
>       https://listman.redhat.com/mailman/listinfo/crash-utility
> 
>       End of Crash-utility Digest, Vol 197, Issue 7
>       *********************************************


--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility

Reply via email to