On (09/01/16 10:58), Petr Mladek wrote:
> On Wed 2016-08-31 21:52:24, Sergey Senozhatsky wrote:
> > On (08/31/16 11:38), Petr Mladek wrote:
> > > 2. Potential deadlocks when calling wake_up_process() by
> > > async printk and console_unlock().
> >
> > * there are many reasons to those recursive printk() calls -- some
> > can be addressed, some cannot. for instance, it doesn't matter how many
> > per-CPU buffers we use for alternative printk() once the logbuf_lock is
> > corrupted.
>
> Yup and BTW: Peter Zijlstra wants to avoid zapping locks whenever
> possible because it corrupts the state. It might solve the actual
> state but it might cause deadlock by the double unlock.
yes, don't really want to zap_locks() either.
[..]
> Great catch! From the already mentioned solutions, I would prefer
> using deferred variants of WARN()/BUG()/printk() on these locations.
> Together with using lockdep to find these locations.
hmm... need to think more. one of the problems is that we would have to
periodically "scan" for new WARNs/BUGs/etc doing all the types of random
.configs
> Also there is the Peter Zijlstra's idea of using a lockless
> "early" console to debug the situations where it happens.
> It might make sense to make such a console easy to use.
aha, not really familiar with early console.
> I am unable to find any other generic solution that would prevent this
> from the printk() side at the moment.
>
> > 5. not 100% guaranteed printing on panic
[..]
> That might be very hard to solve in general as well. Again the PeterZ's
> idea with the lockless console might help here.
"need to google it".
> > > I wonder how to separate the problems and make them more manageable.
> >
> > so I was thinking for a moment about doing the recursion detection rework
> > before the async_printk. just because better recursion detection is a nice
> > thing to have in the first place and it probably may help us catching some
> > of the surprises that async_printk might have. but it probably will be more
> > problematic than I thought.
> >
> > then async_printk. I have a refreshed series on my hands, addressing
> > Viresh's reports. it certainly makes things better, but it doesn't
> > eliminate all of the lockups/etc sources.
>
> We must separate historical possible lockups and new regressions.
> Only regressions should block the async printk series. Old
> bugs should be fixed separately to keep the series manageable.
agree.
> Anyway, I think that the async printk will make sense even
> when we solve all the other issues. If async printk does not
> cause regressions, why not make it in.
sure.
> > a console_unlock() doing
> > wake_up_process(printk_kthread) would make it better.
>
> I am not sure what you mean by this.
I meant that this thing
local_irq_save() // or preempt_disable()
...
if (console_trylock())
console_unlock();
...
local_irq_restore() // or preempt_enable()
can easily lockup the system if console_trylock() was successful and there
are enough messages to print. printk_kthread can't help, because here we
basically enforce the `old' behavior. we have async printk, but not async
console output. tweaking console_unlock() to offload the actual printing loop
to printk_kthread would make the entire console output async:
static void console_sync_flush_and_unlock(void)
{
for (;;) {
...
call_console_drivers();
...
}
}
void console_unlock(void)
{
if (!MOTORMOUTH && can_printk_async()) {
up();
wake_up_process(printk_kthread);
return;
}
console_sync_flush_and_unlock();
}
-ss
