From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Petr Mladek <pmladek@suse.com>, Jan Kara <jack@suse.cz>,
linux-kernel@vger.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH v2] printk/nmi: avoid direct printk()-s from __printk_nmi_flush()
Date: Wed, 31 Aug 2016 10:44:41 +0900 [thread overview]
Message-ID: <20160831014441.GA472@swordfish> (raw)
In-Reply-To: <20160830150315.93efc592aa631f474af760b5@linux-foundation.org>
Hello,
On (08/30/16 15:03), Andrew Morton wrote:
> > __printk_nmi_flush() can be called from nmi_panic(), therefore it has to
> > test whether it's executed in NMI context and thus must route the messages
> > through deferred printk() or via direct printk().
>
> Why? What misbehaviour does the current code cause?
the reasoning behind the `if in_nmi()' in print_nmi_seq_line()
if (in_nmi())
printk_deferred("%.*s", (end - start) + 1, buf);
else
printk("%.*s", (end - start) + 1, buf);
was as follows (per Petr's commit message)
: In NMI context, printk() messages are stored into per-CPU buffers to
: avoid a possible deadlock. They are normally flushed to the main ring
: buffer via an IRQ work. But the work is never called when the system
: calls panic() in the very same NMI handler.
:
: This patch tries to flush NMI buffers before the crash dump is
: generated. In this case it does not risk a double release and bails out
: when the logbuf_lock is already taken. The aim is to get the messages
: into the main ring buffer when possible. It makes them better
: accessible in the vmcore.
:
: Then the patch tries to flush the buffers second time when other CPUs
: are down. It might be more aggressive and reset logbuf_lock. The aim
: is to get the messages available for the consequent kmsg_dump() and
: console_flush_on_panic() calls.
:
: The patch causes vprintk_emit() to be called even in NMI context again.
: But it is done via printk_deferred() so that the console handling is
: skipped. Consoles use internal locks and we could not prevent a
: deadlock easily. They are explicitly called later when the crash dump
: is not generated, see console_flush_on_panic().
doing pr_err() and pr_cont() defeats the purpose of print_nmi_seq_line(),
because in the worst case it can do something like this:
vprintk_emit()
/* console_trylock() */
console_unlock()
call_console_drivers()
foo_write() // possibly locked
the tricky moment here is -- if the console semaphore is unlocked and
NMI can successfully console_trylock(), then how any of underlying serial
console driver's locks can be taken? one possibility is:
CPU0
SyS_ioctl
do_vfs_ioctl
tty_ioctl
n_tty_ioctl
tty_mode_ioctl
set_termios
tty_set_termios
uart_set_termios
uart_change_speed
FOO_serial_set_termios
spin_lock_irqsave(&port->lock) // lock the output port
...
--> NMI
nmi_panic()
printk_nmi_flush_on_panic()
__printk_nmi_flush()
pr_cont()
/* console_trylock() */
console_unlock()
call_console_drivers()
foo_write()
spin_lock_irqsave(&port->lock) // already locked
as far as I can see, tty_ioctl() path does not lock console semaphore,
but grabs some tty locks instead.
-ss
next prev parent reply other threads:[~2016-08-31 1:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-30 16:13 [PATCH v2] printk/nmi: avoid direct printk()-s from __printk_nmi_flush() Sergey Senozhatsky
2016-08-30 22:03 ` Andrew Morton
2016-08-31 1:44 ` Sergey Senozhatsky [this message]
2016-08-31 20:15 ` Andrew Morton
2016-09-01 0:55 ` Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160831014441.GA472@swordfish \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=jack@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=sergey.senozhatsky@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).