| Message ID | 20201126133338.125608-1-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] riscv: Using printk directly in __show_regs | expand |
On Thu, 26 Nov 2020 05:33:38 PST (-0800), wangkefeng.wang@huawei.com wrote: > Covert bulk of pr_cont("...\n") to separate printk calls. > > Note that a pr_cont() message with a trailing newline will > "terminate" the continous line so that it can no longer be > continued, also it is possible to sort out simultaneous dumps > from multiple CPUs with CONFIG_PRINTK_CALLER enabled. > > And align the printing formats. > > Suggested-by: John Ogness <john.ogness@linutronix.de> > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > arch/riscv/kernel/process.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c > index e41b733abeaa..2119d49feea5 100644 > --- a/arch/riscv/kernel/process.c > +++ b/arch/riscv/kernel/process.c > @@ -44,30 +44,30 @@ void __show_regs(struct pt_regs *regs) > { > show_regs_print_info(KERN_DEFAULT); > > - pr_cont("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", > + printk("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", > regs->epc, regs->ra, regs->sp); > - pr_cont(" gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", > + printk("gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", > regs->gp, regs->tp, regs->t0); > - pr_cont(" t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", > + printk("t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", > regs->t1, regs->t2, regs->s0); > - pr_cont(" s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", > + printk("s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", > regs->s1, regs->a0, regs->a1); > - pr_cont(" a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", > + printk("a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", > regs->a2, regs->a3, regs->a4); > - pr_cont(" a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", > + printk("a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", > regs->a5, regs->a6, regs->a7); > - pr_cont(" s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", > + printk("s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", > regs->s2, regs->s3, regs->s4); > - pr_cont(" s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", > + printk("s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", > regs->s5, regs->s6, regs->s7); > - pr_cont(" s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", > + printk("s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", > regs->s8, regs->s9, regs->s10); > - pr_cont(" s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", > + printk("s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", > regs->s11, regs->t3, regs->t4); > - pr_cont(" t5 : " REG_FMT " t6 : " REG_FMT "\n", > + printk("t5 : " REG_FMT " t6 : " REG_FMT "\n", > regs->t5, regs->t6); > > - pr_cont("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", > + printk("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", > regs->status, regs->badaddr, regs->cause); > } > void show_regs(struct pt_regs *regs) For some reason I remember having these as printk()s originally, but I don't see that in the git history so maybe it was from before we merged the port? I don't really understand the difference here, and I thought using printk() directly wasn't recommended?
On 2020-12-10, Palmer Dabbelt <palmerdabbelt@google.com> wrote: > On Thu, 26 Nov 2020 05:33:38 PST (-0800), wangkefeng.wang@huawei.com wrote: >> Covert bulk of pr_cont("...\n") to separate printk calls. >> >> Note that a pr_cont() message with a trailing newline will >> "terminate" the continous line so that it can no longer be >> continued, also it is possible to sort out simultaneous dumps >> from multiple CPUs with CONFIG_PRINTK_CALLER enabled. >> >> And align the printing formats. >> >> Suggested-by: John Ogness <john.ogness@linutronix.de> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >> --- >> arch/riscv/kernel/process.c | 24 ++++++++++++------------ >> 1 file changed, 12 insertions(+), 12 deletions(-) >> >> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c >> index e41b733abeaa..2119d49feea5 100644 >> --- a/arch/riscv/kernel/process.c >> +++ b/arch/riscv/kernel/process.c >> @@ -44,30 +44,30 @@ void __show_regs(struct pt_regs *regs) >> { >> show_regs_print_info(KERN_DEFAULT); >> >> - pr_cont("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", >> + printk("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", >> regs->epc, regs->ra, regs->sp); >> - pr_cont(" gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", >> + printk("gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", >> regs->gp, regs->tp, regs->t0); >> - pr_cont(" t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", >> + printk("t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", >> regs->t1, regs->t2, regs->s0); >> - pr_cont(" s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", >> + printk("s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", >> regs->s1, regs->a0, regs->a1); >> - pr_cont(" a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", >> + printk("a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", >> regs->a2, regs->a3, regs->a4); >> - pr_cont(" a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", >> + printk("a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", >> regs->a5, regs->a6, regs->a7); >> - pr_cont(" s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", >> + printk("s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", >> regs->s2, regs->s3, regs->s4); >> - pr_cont(" s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", >> + printk("s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", >> regs->s5, regs->s6, regs->s7); >> - pr_cont(" s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", >> + printk("s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", >> regs->s8, regs->s9, regs->s10); >> - pr_cont(" s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", >> + printk("s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", >> regs->s11, regs->t3, regs->t4); >> - pr_cont(" t5 : " REG_FMT " t6 : " REG_FMT "\n", >> + printk("t5 : " REG_FMT " t6 : " REG_FMT "\n", >> regs->t5, regs->t6); >> >> - pr_cont("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", >> + printk("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", >> regs->status, regs->badaddr, regs->cause); >> } >> void show_regs(struct pt_regs *regs) > > For some reason I remember having these as printk()s originally, but I > don't see that in the git history so maybe it was from before we > merged the port? I don't really understand the difference here, and I > thought using printk() directly wasn't recommended? There no difference between: pr_cont("...\n"); and printk("...\n"); except for the very first call, which will try to continue from whatever was printed before. (But even that only makes a difference if the previous call did not have a newline at the end.) Note that the printk() being used does not specify the loglevel. Perhaps it is intentional that the default loglevel (which can be specified by the user) is used? Otherwise, if a particular loglevel _is_ desired, then the appropriate pr_info/pr_warn/... should be used. Also, this patch is also sneaking in some formatting changes. Previously the columns were not aligned correctly. That spacing is also fixed here. The spacing change _is_ a real improvement. John Ogness
On 2020/12/11 16:48, John Ogness wrote: > On 2020-12-10, Palmer Dabbelt <palmerdabbelt@google.com> wrote: >> On Thu, 26 Nov 2020 05:33:38 PST (-0800), wangkefeng.wang@huawei.com wrote: >>> Covert bulk of pr_cont("...\n") to separate printk calls. >>> >>> Note that a pr_cont() message with a trailing newline will >>> "terminate" the continous line so that it can no longer be >>> continued, also it is possible to sort out simultaneous dumps >>> from multiple CPUs with CONFIG_PRINTK_CALLER enabled. >>> >>> And align the printing formats. >>> >>> Suggested-by: John Ogness <john.ogness@linutronix.de> >>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >>> --- >>> arch/riscv/kernel/process.c | 24 ++++++++++++------------ >>> 1 file changed, 12 insertions(+), 12 deletions(-) >>> >>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c >>> index e41b733abeaa..2119d49feea5 100644 >>> --- a/arch/riscv/kernel/process.c >>> +++ b/arch/riscv/kernel/process.c >>> @@ -44,30 +44,30 @@ void __show_regs(struct pt_regs *regs) >>> { >>> show_regs_print_info(KERN_DEFAULT); >>> >>> - pr_cont("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", >>> + printk("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", >>> regs->epc, regs->ra, regs->sp); >>> - pr_cont(" gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", >>> + printk("gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", >>> regs->gp, regs->tp, regs->t0); >>> - pr_cont(" t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", >>> + printk("t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", >>> regs->t1, regs->t2, regs->s0); >>> - pr_cont(" s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", >>> + printk("s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", >>> regs->s1, regs->a0, regs->a1); >>> - pr_cont(" a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", >>> + printk("a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", >>> regs->a2, regs->a3, regs->a4); >>> - pr_cont(" a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", >>> + printk("a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", >>> regs->a5, regs->a6, regs->a7); >>> - pr_cont(" s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", >>> + printk("s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", >>> regs->s2, regs->s3, regs->s4); >>> - pr_cont(" s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", >>> + printk("s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", >>> regs->s5, regs->s6, regs->s7); >>> - pr_cont(" s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", >>> + printk("s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", >>> regs->s8, regs->s9, regs->s10); >>> - pr_cont(" s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", >>> + printk("s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", >>> regs->s11, regs->t3, regs->t4); >>> - pr_cont(" t5 : " REG_FMT " t6 : " REG_FMT "\n", >>> + printk("t5 : " REG_FMT " t6 : " REG_FMT "\n", >>> regs->t5, regs->t6); >>> >>> - pr_cont("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", >>> + printk("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", >>> regs->status, regs->badaddr, regs->cause); >>> } >>> void show_regs(struct pt_regs *regs) >> For some reason I remember having these as printk()s originally, but I >> don't see that in the git history so maybe it was from before we >> merged the port? I don't really understand the difference here, and I >> thought using printk() directly wasn't recommended? The original reason which I changed to printk is the strange out-print, see v1 patch [1], but it's found that there is a regression in v5.10 printk rework, and being fixed by commit 4ad9921af4f1 ("printk: finalize records with trailing newlines"). The v2 patch major change is align the printing (not a big change) and in case of rearrangement of print when CONFIG_PRINTK_CALLER enabled [1] https://patchwork.kernel.org/project/linux-riscv/patch/20201124112054.106960-2-wangkefeng.wang@huawei.com/ > There no difference between: > > pr_cont("...\n"); > and > printk("...\n"); > > except for the very first call, which will try to continue from whatever > was printed before. (But even that only makes a difference if the > previous call did not have a newline at the end.) > > Note that the printk() being used does not specify the loglevel. Perhaps > it is intentional that the default loglevel (which can be specified by > the user) is used? Otherwise, if a particular loglevel _is_ desired, > then the appropriate pr_info/pr_warn/... should be used. > > Also, this patch is also sneaking in some formatting changes. Previously > the columns were not aligned correctly. That spacing is also fixed > here. The spacing change _is_ a real improvement. > > John Ogness > . >
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index e41b733abeaa..2119d49feea5 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -44,30 +44,30 @@ void __show_regs(struct pt_regs *regs) { show_regs_print_info(KERN_DEFAULT); - pr_cont("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", + printk("epc: " REG_FMT " ra : " REG_FMT " sp : " REG_FMT "\n", regs->epc, regs->ra, regs->sp); - pr_cont(" gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", + printk("gp : " REG_FMT " tp : " REG_FMT " t0 : " REG_FMT "\n", regs->gp, regs->tp, regs->t0); - pr_cont(" t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", + printk("t1 : " REG_FMT " t2 : " REG_FMT " s0 : " REG_FMT "\n", regs->t1, regs->t2, regs->s0); - pr_cont(" s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", + printk("s1 : " REG_FMT " a0 : " REG_FMT " a1 : " REG_FMT "\n", regs->s1, regs->a0, regs->a1); - pr_cont(" a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", + printk("a2 : " REG_FMT " a3 : " REG_FMT " a4 : " REG_FMT "\n", regs->a2, regs->a3, regs->a4); - pr_cont(" a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", + printk("a5 : " REG_FMT " a6 : " REG_FMT " a7 : " REG_FMT "\n", regs->a5, regs->a6, regs->a7); - pr_cont(" s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", + printk("s2 : " REG_FMT " s3 : " REG_FMT " s4 : " REG_FMT "\n", regs->s2, regs->s3, regs->s4); - pr_cont(" s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", + printk("s5 : " REG_FMT " s6 : " REG_FMT " s7 : " REG_FMT "\n", regs->s5, regs->s6, regs->s7); - pr_cont(" s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", + printk("s8 : " REG_FMT " s9 : " REG_FMT " s10: " REG_FMT "\n", regs->s8, regs->s9, regs->s10); - pr_cont(" s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", + printk("s11: " REG_FMT " t3 : " REG_FMT " t4 : " REG_FMT "\n", regs->s11, regs->t3, regs->t4); - pr_cont(" t5 : " REG_FMT " t6 : " REG_FMT "\n", + printk("t5 : " REG_FMT " t6 : " REG_FMT "\n", regs->t5, regs->t6); - pr_cont("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", + printk("status: " REG_FMT " badaddr: " REG_FMT " cause: " REG_FMT "\n", regs->status, regs->badaddr, regs->cause); } void show_regs(struct pt_regs *regs)
Covert bulk of pr_cont("...\n") to separate printk calls. Note that a pr_cont() message with a trailing newline will "terminate" the continous line so that it can no longer be continued, also it is possible to sort out simultaneous dumps from multiple CPUs with CONFIG_PRINTK_CALLER enabled. And align the printing formats. Suggested-by: John Ogness <john.ogness@linutronix.de> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- arch/riscv/kernel/process.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)