Messages in this thread | | | Subject | Re: [PATCH 2/3] x86/dumpstack: Add log_lvl to __show_regs() | From | Dmitry Safonov <> | Date | Thu, 25 Jun 2020 17:42:31 +0100 |
| |
On 6/23/20 5:45 PM, Jann Horn wrote: > On Tue, Jun 23, 2020 at 6:30 PM Dmitry Safonov <dima@arista.com> wrote: >> show_trace_log_lvl() provides x86 platform-specific way to unwind >> backtrace with a given log level. Unfortunately, registers dump(s) are >> not printed with the same log level - instead, KERN_DEFAULT is always >> used. >> >> Arista's switches uses quite common setup with rsyslog, where only >> urgent messages goes to console (console_log_level=KERN_ERR), everything >> else goes into /var/log/ as the console baud-rate often is indecently >> slow (9600 bps). >> >> Backtrace dumps without registers printed have proven to be as useful as >> morning standups. Furthermore, in order to introduce KERN_UNSUPPRESSED >> (which I believe is still the most elegant way to fix raciness of sysrq[1]) >> the log level should be passed down the stack to register dumping >> functions. Besides, I have a potential use-case for printing traces >> with KERN_DEBUG level [2] (where registers dump shouldn't appear with >> higher log level). >> >> Add log_lvl parameter to __show_regs(). >> Keep the used log level intact to separate visible change. > > This change seems like a good idea to me; just one small nit: > > [...] >> void show_regs(struct pt_regs *regs) >> { >> + int print_kernel_regs; >> + >> show_regs_print_info(KERN_DEFAULT); >> >> - __show_regs(regs, user_mode(regs) ? SHOW_REGS_USER : SHOW_REGS_ALL); >> + print_kernel_regs = user_mode(regs) ? SHOW_REGS_USER : SHOW_REGS_ALL; >> + __show_regs(regs, print_kernel_regs, KERN_DEFAULT); > > print_kernel_regs should probably have type "enum show_regs_mode"?
Makes sense, will fix in v2.
Thanks, Dmitry
| |