Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

[Petr Mladek]

On Tue 2018-01-16 15:10:13, Sergey Senozhatsky wrote:
> Hi,
> 
> On (01/15/18 12:50), Petr Mladek wrote:
> > On Mon 2018-01-15 11:17:43, Petr Mladek wrote:
> > > PS: Sergey, you have many good points. The printk-stuff is very
> > > complex and we could spend years discussing the perfect solution.
> > 
> > BTW: One solution that comes to my mind is based on ideas
> > already mentioned in this thread:
> > 
> > void console_unlock(void)
> > {
> > 	disable_preemtion();
> > 
> > 	while(pending_message) {
> > 
> > 	    call_console_drivers();
> > 
> > 	    if (too_long_here() && current != printk_kthread) {
> > 	       wake_up_process(printk_kthread())
> > 
> > 	}
> > 
> > 	enable_preemtion();
> > }
> 
> unfortunately disabling preemtion in console_unlock() is a bit
> dangerous :( we have paths that call console_unlock() exactly
> to flush everything (not only new pending messages, but everything)
> that is in logbuf and we cannot return from console_unlock()
> preliminary in that case.

You are right. Just to be sure. Are you talking about replaying
the entire log when a new console is registered? Or do you know
about more paths?

If I get it correctly, we allow to hand off the lock even when
replying the entire log. But you are right that we should
enable preemption in this case because there are many messages
even without printk() activity.

IMHO, the best solution would be to reply the log in a
separate process asynchronously and do not block existing
consoles in the meantime. But I am not sure if it is worth
the complexity. Anyway, it is a future work.


> > bool too_long_here(void)
> > {
> > 	return should_resched();
> > or
> > 	return spent_here() > 1 / HZ / 2;
> > or
> > 	what ever we agree on
> > }
> > 
> > 
> > int printk_kthread_func(void *data)
> > {
> > 	while(1) {
> > 		 if (!pending_messaged)
> > 			schedule();
> > 
> > 		if (console_trylock_spinning())
> > 			console_unlock();
> > 
> > 		cond_resched();
> > 	}
> > }
> 
> overall that's very close to what I have in one of my private branches.
> console_trylock_spinning() for some reason does not perform really
> well on my made-up internal printk torture tests. it seems that I
> have a much better stability (no lockups and so on) when I also let
> printk_kthread to sleep on console_sem(). but I will look further.

I believe that it is not trivial. console_trylock_spinning() is
tricky and the timing is important. For example, it might be tricky
if a torture test affects the normal workflow by many interrupts.
We might need to call even more console_unlock() code with
spinning enabled to improve the success ratio. Another problem
is that the kthread must be scheduled on another CPU. And so
on. I believe that there are many more problems and areas
for improvement.

Best Regards,
Petr