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

From: Sergey Senozhatsky
Date: Wed Mar 16 2016 - 02:59:49 EST


On (03/16/16 14:39), Byungchul Park wrote:
> On Tue, Mar 15, 2016 at 11:07:38PM +0900, Sergey Senozhatsky wrote:
> >
> > something like this (*minimally tested so far*).
> >
> > -- move wake_up() and friends under the logbuf section; so we can detect
> ^^^^^^^^^^^^^^
> section protected by logbuf_lock?
>
> > printk() recursion from wake_up()
>
> Excuse me, but I fear that it can cause an unnecessary deadlock.

where? shouldn't it be

vprintk_emit()
local_irq_save(flags);
this_cpu = smp_processor_id();

raw_spin_lock(&logbuf_lock);
static logbuf_cpu = this_cpu;

wake_up_process()
spin_lock()
spin_dump()
vprintk_emit() << recursion
local_irq_save(flags);
this_cpu = smp_processor_id();

if (unlikely(logbuf_cpu == this_cpu)) {
recursion_bug = true;
return; << break recursion
}

?

> Furthermore it cannot be handled if it is caused by logbuf_lock.

sure, I'm not even trying to address a logbuf spin_dump recursion problem here.
I'm only trying to minimize the impact of newly introduced spin locks.

I don't have a very clear vision (at the moment) of how to fix printk recursion
caused by logbuf lock of console_sem corruptions, etc. Should spin_dump
be aware of the locks that can be taken by printk()? Hm, I can't even count all
the locks that possibly can be taken by printk->console_drivers and most likely
don't even see all of the cases where printk can recurse. Disable lock debug
in vprintk_emit() the same way lockdep is desabled? Hm...

Ingo's POV is that printk must be reworked and become smarter in this aspect.

> I mean that it would be better to keep the wake_up and friend out of the
> critical section by logbuf_lock.

in this case wake_up_process() can recurse us forever.

-ss