lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/4] printk: Avoid livelock with heavy printk during panic
On Wed 2022-01-26 15:02:35, Stephen Brennan wrote:
> During panic(), if another CPU is writing heavily the kernel log (e.g.
> via /dev/kmsg), then the panic CPU may livelock writing out its messages
> to the console. Note when too many messages are dropped during panic and
> suppress further printk, except from the panic CPU. This could result in
> some important messages being dropped. However, messages are already
> being dropped, so this approach at least prevents a livelock.
>
> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> ---
>
> Notes:
> v2: Add pr_warn when we suppress printk on non-panic CPU
>
> kernel/printk/printk.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 20b4b71a1a07..18107db118d4 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -93,6 +93,12 @@ EXPORT_SYMBOL_GPL(console_drivers);
> */
> int __read_mostly suppress_printk;
>
> +/*
> + * During panic, heavy printk by other CPUs can delay the
> + * panic and risk deadlock on console resources.
> + */
> +int __read_mostly suppress_panic_printk;
> +
> #ifdef CONFIG_LOCKDEP
> static struct lockdep_map console_lock_dep_map = {
> .name = "console_lock"
> @@ -2228,6 +2234,10 @@ asmlinkage int vprintk_emit(int facility, int level,
> if (unlikely(suppress_printk))
> return 0;
>
> + if (unlikely(suppress_panic_printk) &&
> + atomic_read(&panic_cpu) != raw_smp_processor_id())
> + return 0;
> +
> if (level == LOGLEVEL_SCHED) {
> level = LOGLEVEL_DEFAULT;
> in_sched = true;
> @@ -2613,6 +2623,7 @@ void console_unlock(void)
> {
> static char ext_text[CONSOLE_EXT_LOG_MAX];
> static char text[CONSOLE_LOG_MAX];
> + static int panic_console_dropped;
> unsigned long flags;
> bool do_cond_resched, retry;
> struct printk_info info;
> @@ -2667,6 +2678,11 @@ void console_unlock(void)
> if (console_seq != r.info->seq) {
> console_dropped += r.info->seq - console_seq;
> console_seq = r.info->seq;
> + if (panic_in_progress() && panic_console_dropped++ > 10) {
> + suppress_panic_printk = 1;
> + pr_warn("Too many dropped messages. "
> + "Supress messages on non-panic CPUs to prevent livelock.\n");

It looks like the message might be printed more times when
panic_console_dropped++ > 10.

In theory, no message can be lost after we disable printk on another
CPUs. But the code might evolve in the future. Let's make it
more error-proof.

We could use (panic_console_dropped++ == 10) or pr_warn_once() or
both.

I prefer using pr_warn_once() because it looks the most error-proof.


Nit: printk() has exceptions from the 80 chars/line rule.
The message string should be on a single line. It helps
to find it using "git grep". I think that even checkpatch.pl
handles this correctly.


With pr_warn_once() and message in single line:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr


PS: I could fix the two problems when pushing to git. But there
is still time to send v3. I have vacation the following week
with limited internet access. I am not going to rush it into
linux before I leave, ...


\
 
 \ /
  Last update: 2022-01-27 15:51    [W:0.106 / U:0.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site