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

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

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