Re: [RFC][PATCH v4 1/2] printk: Make printk() completely async

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

On (03/16/16 19:34), Byungchul Park wrote:
[..]
> > -- if we are going to have wake_up_process() in wake_up_klogd_work_func(),
> > then we need `in_sched' message to potentially trigger a recursion chain
> > 
> > wake_up_klogd_work_func()->wake_up_process()->printk()->wake_up_process()->printk()...
> > 
> > to break this printk()->wake_up_process()->printk(), we need wake_up_process() to
> > be under the logbuf lock; so vprintk_emit()'s if (logbuf_cpu == this_cpu) will act.
> 
> I am curious about how you make the wake_up_process() call and I may want
> to talk about it at the next spin. Anyway, then we will lose the last
> message when "if (logbuf_cpu == this_cpu)" acts. Is it acceptible?

yes, this is how it is. "BUG: recent printk recursion!" will be printed
instead of the message.

> IMHO it's not a good choice to use wake_up() and friend within a printk()
> since it can additionally cause another recursion. Of course, it does not
> happen if the condition (logbuf_cpu == this_cpu) acts. But I don't think
> it's good to rely on the condition with losing a message. Anyway I really
> really want to see your next spin and talk.

the alternative is NOT significantly better. pending bit is checked in
IRQ, so one simply can do

	local_irq_save();
	while (xxx) printk();
	local_irq_restore();

and _in the worst case_ nothing will be printed to console until IRQ are
enabled on this CPU. (there are some 'if's, but the worst case is just
like this. http://marc.info/?l=linux-kernel&m=145734549308803).


I'd probably prefer to add wake_up_process() to vprintk_emit() and do it
under the logbuf lock. first, we don't suffer from disabled IRQs on current
CPU, second we have somewhat better chances to break printk() recursion
*in some cases*.

> > -- if we are going to have wake_up_process() in console_unlock(), then
> > 
> > console_unlock()->{up(), wake_up_process()}->printk()->{console_lock(), console_unlock()}->{up(), wake_up_process()}->printk()...
> > 
> 
> This cannot happen. console_lock() cannot continue because the prior
> console_unlock() does not release console_sem.lock yet when
> wake_up_process() is called. Only a deadlock exists. And my patch solves
> the problem so that the deadlock cannot happen.

ah, we lost in patches. I was talking about yet another patch
(you probably not aware of. you were not Cc'd. Sorry!) that
makes console_unlock() asynchronous:

http://marc.info/?l=linux-kernel&m=145750373530161

s/wake_up/wake_up_process/ is at the end of console_unlock().

while the patch belongs to another series, I still wanted to outline it
here, since we were talking about printk() recursion.

	-ss