Messages in this thread | | | Date | Fri, 14 Apr 2023 12:10:02 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH printk v1 16/18] kernel/panic: Add atomic write enforcement to warn/panic |
| |
On Thu 2023-04-13 14:19:13, John Ogness wrote: > On 2023-04-13, Petr Mladek <pmladek@suse.com> wrote: > >> --- a/kernel/panic.c > >> +++ b/kernel/panic.c > >> @@ -329,6 +332,8 @@ void panic(const char *fmt, ...) > >> if (_crash_kexec_post_notifiers) > >> __crash_kexec(NULL); > >> > >> + cons_atomic_flush(NULL, true); > > > > Do we need to explicitly flush the messages here? > > This is where the atomic printing actually starts (after the full dump > has been inserted into the ringbuffer). > > > cons_atomic_flush() is called also from vprintk_emit(). And there are > > many messages printed with the PANIC priority above. > > vprintk_emit() does not print in this case. From cons_atomic_flush(): > > /* > * When in an elevated priority, the printk() calls are not > * individually flushed. This is to allow the full output to > * be dumped to the ringbuffer before starting with printing > * the backlog. > */ > if (cpu_state->prio > NBCON_PRIO_NORMAL && printk_caller_wctxt) > return;
OK, what is the motivation for this behavior, please? Does it has any advantages?
> > > This makes an assumption that either printk() in PANIC context > > does not try to show the messages immediately or that this > > explicit console_atomic_flush() tries harder. I think > > that both assumptions are wrong. > > Both assumptions are correct, because until this point there has been no > effort to print.
Honestly, this makes me nervous. It means that panic() messages will not reach the console unless they are explicitly flushed.
First, it is error-prone because it requires calling console_atomic_flush() in all relevant code paths on the right locations.
Second, it expects that panic() code could never fail between the explicit console_atomic_flush() calls. If it failed, it might be pretty useful to see the last printed message.
Third, messages might get lost when there are too many. And it is realistic. For example, see panic_print_sys_info() it might add quite long reports.
I would really prefer to flush atomic consoles with every printk() unless there is a really good reason not to do it.
Best Regards, Petr
| |