Messages in this thread |  | | Date | Wed, 19 Oct 2016 16:41:40 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH 1/3] printk: Fix kdb_trap_printk placement |
| |
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 <jason.wessel@windriver.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Otherwise, your patch makes sense:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards, Petr
|  |