lkml.org 
[lkml]   [2018]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] printk/nmi: Prevent deadlock when serializing NMI backtraces
On (05/17/18 16:39), Petr Mladek wrote:
>
> CPU0 CPU1 CPU2
>
> printk()
> vprintk_emit()
> spin_lock(&logbuf_lock)
>
> trigger_all_cpu_backtrace()
> raise()
>
> nmi_enter()
> printk_nmi_enter()
> if (this_cpu_read(printk_context)
> & PRINTK_SAFE_CONTEXT_MASK)
> // false
> else
> // looks safe to use printk_deferred()
> this_cpu_or(printk_context,
> PRINTK_NMI_DEFERRED_CONTEXT_MASK);
>
> nmi_cpu_backtrace()
> arch_spin_lock(&lock);
> show_regs()
>
> nmi_enter()
> nmi_cpu_backtrace()
> arch_spin_lock(&lock);
>
> printk()
> vprintk_func()
> vprintk_deferred()
> vprintk_emit()
> spin_lock(&logbuf_lock)
>
> DEADLOCK: between &logbuf_lock from vprintk_emit() and
> &lock from nmi_cpu_backtrace().
>
> CPU0 CPU1
> lock(logbuf_lock) lock(lock)
> lock(lock) lock(logbuf_lock)
>
[..]
> Signed-off-by: Petr Mladek <pmladek@suse.com>

This is a pretty cool find!

Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

> - if ((this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK) &&
> - raw_spin_is_locked(&logbuf_lock)) {
> + if (raw_spin_is_locked(&logbuf_lock))
> this_cpu_or(printk_context, PRINTK_NMI_CONTEXT_MASK);
> - } else {
> + else
> this_cpu_or(printk_context, PRINTK_NMI_DEFERRED_CONTEXT_MASK);
> - }

A question - can we switch to a bitwise OR?

if (this_cpu_read(printk_context) & PRINTK_SAFE_CONTEXT_MASK) ||
raw_spin_is_locked(&logbuf_lock)

just to check per-CPU `printk_context' first and only afterwards
access the global `logbuf_lock'. printk_nmi_enter() happens on
every CPU, so maybe we can avoid some overhead by checking the
local per-CPU data first.

-ss

\
 
 \ /
  Last update: 2018-05-18 04:08    [W:0.112 / U:0.016 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site