On Tue 2016-10-18 19:08:31, Peter Zijlstra wrote:
> Some people figured vprintk_emit() makes for a nice API and exported
> it, bypassing the kdb trap.
>
> This still leaves vprintk_nmi() outside of the kbd reach, should that
> be fixed too?
Good question! vkdb_printf() tries to avoid a deadlock but the code is racy:
int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap)
{
[...]
/* Serialize kdb_printf if multiple cpus try to write at once.
* But if any cpu goes recursive in kdb, just print the output,
* even if it is interleaved with any other text.
*/
if (!KDB_STATE(PRINTF_LOCK)) {
KDB_STATE_SET(PRINTF_LOCK);
spin_lock_irqsave(&kdb_printf_lock, flags);
got_printf_lock = 1;
atomic_inc(&kdb_event);
} else {
__acquire(kdb_printf_lock);
}
Let's have the following situation:
CPU1 CPU2
if (!KDB_STATE(PRINTF_LOCK)) {
KDB_STATE_SET(PRINTF_LOCK);
if (!KDB_STATE(PRINTF_LOCK)) {
} else {
__acquire(kdb_printf_lock);
}
Now, both CPUs are in the critical section and happily writing over each
other, e.g. in
vsnprintf(next_avail, size_avail, fmt, ap);
I quess that we want to fix this race. But I am not sure if it will
be done an NMI-safe way. I am going to send a patch for this.
Well, vkdb_printf() is called later when the messages are pushed
to the main logbuffer by printk_nmi_flush_line(). It is not perfect
but...
> Cc: Jason Wessel <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Otherwise, your patch makes sense:
Reviewed-by: Petr Mladek <[email protected]>
Best Regards,
Petr
