Messages in this thread | | | Date | Thu, 31 Mar 2016 13:52:50 +0200 | From | Jan Kara <> | Subject | Re: [RFC][PATCH v8 1/2] printk: Make printk() completely async |
| |
On Thu 31-03-16 13:12:29, Petr Mladek wrote: > > #if defined CONFIG_PRINTK > > +static int printk_kthread_func(void *data) > > +{ > > + while (1) { > > + set_current_state(TASK_INTERRUPTIBLE); > > + if (!need_flush_console) > > + schedule(); > > + > > + __set_current_state(TASK_RUNNING); > > > We still must do here: > > need_flush_console = false; > > Othrerwise, we might start "busy" cycling. cosole_unlock() > sometimes returns earlly, e.g. when console_suspended is set > or !can_use_console() returns true. > > Sigh, the handling of "need_flush_console" is a bit strange. > Part of the logic depends on logbuf_lock and the other part > must be lockless.
Frankly, I think we are overcomplicating this. What we really need to detect in printk_kthread_func() is whether someone appended something to the console since we woken up. Sure, console_unlock() may have already printed that and we would unnecessarily make one more loop over console_lock() and console_unlock() but who cares...
So what about having printk_kthread_func() like:
while (1) { set_current_state(TASK_INTERRUPTIBLE); if (!need_flush_console) schedule(); __set_current_state(TASK_RUNNING); need_flush_console = false; console_lock(); console_unlock(); }
In vprintk_emit() we do:
if (!in_panic && printk_kthread) { /* Offload printing to a schedulable context. */ need_flush_console = true; wake_up_process(printk_kthread); } else { ...
This guarantees that after message was appended to the buffer in vprintk_emit(), the message got either printed by console_unlock() or printk_kthread is in TASK_RUNNING state and will call console_unlock() once scheduled. It also guarantees that printk_kthread_func() won't loop forever when there's nothing to print. And that is all we need...
I think the simplicity of this is worth the possible extra loops in printk_kthread_func().
Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
| |