lkml.org 
[lkml]   [2018]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH] printk: do not call console drivers from printk_safe context
On Tue, 24 Apr 2018 11:28:02 +0900
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> wrote:

> Calling console drivers from printk_safe() context does not really
> make call_console_drivers() any safer, because printk_safe() has
> nothing to do with console drivers or the underlying code. At the
> same time printk()-s from console drivers are fine, they don't
> deadlock the system. We need printk_safe() because of the way
> vprintk_emit() works -- we protect logbuf lock, console_owner_lock
> and console_sem spin_lock with printk_safe, -- not because of the
> console drivers (which don't deal with logbuf, console_owner_lock
> or console_sem locks). Hence we can call console drivers outside
> of printk_safe() context.
>
> Another thing to notice is that,
> printk_safe() introduces unneeded complexity, since any printk()
> message from console drivers has to be stored in per-CPU printk_safe()
> buffer first, then be flushed via IRQ work:
> call_console_drivers()
> printk()
> printk_safe_log_store()
> IRQ_work()
> printk_safe_flush_buffer()
> printk_deferred()
> log_store()
> irq_work_queue() *
> wake_up_klogd_work_func() *
>
> Note that this also costs us extra IRQ work [along with the IRQ work
> that flushes printk_safe() buffer] - we flush per-CPU printk_safe()
> buffers via printk_deferred().
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
> kernel/printk/internal.h | 7 ++++++-
> kernel/printk/printk.c | 2 ++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
> index 2a7d04049af4..f3ba1bf08590 100644
> --- a/kernel/printk/internal.h
> +++ b/kernel/printk/internal.h
> @@ -55,8 +55,13 @@ void __printk_safe_exit(void);
> } while (0)
>
> #else
> +static void __printk_safe_enter(void) {}
> +static void __printk_safe_exit(void) {}
>
> -__printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
> +static __printf(1, 0) int vprintk_func(const char *fmt, va_list args)
> +{
> + return 0;
> +}
>
> /*
> * In !PRINTK builds we still export logbuf_lock spin_lock, console_sem
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2f4af216bd6e..9acb25ce6081 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2391,9 +2391,11 @@ void console_unlock(void)
> */
> console_lock_spinning_enable();
>
> + __printk_safe_exit();
> stop_critical_timings(); /* don't trace print latency */
> call_console_drivers(ext_text, ext_len, text, len);
> start_critical_timings();
> + __printk_safe_enter();
>

OK, I'm still confused (It's been that kind of week)

So, if we do this, and the consoles do a printk(), doesn't that fill
the logbuf? And then the loop this is in will just continue to perform
that loop? That is, we have:

for (;;) {
if (console_seq == log_next_seq)
break;
console_seq++;
call_console_drives() {
printk() {
log_next_seq++;
}
}
}

That looks like an infinite loop to me. Whereas the printk_safe keeps
from adding to the logbuf?

-- Steve


> if (console_lock_spinning_disable_and_check()) {
> printk_safe_exit_irqrestore(flags);

\
 
 \ /
  Last update: 2018-04-24 16:52    [W:0.364 / U:0.836 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site