Re: NMI watchdog dump does not print on hard lockup

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

On (10/16/17 10:15), Steven Rostedt wrote:
[..]
> > just "brainstorming" it... with some silly ideas.
> > 
> > pushing the data from NMI panic might look like we are replacing one
> > deadlock scenario with another deadlock scenario. some of the console
> > drivers are soooo complex internally. so I have been thinking about...
> > may be we can extend struct console and add ->write_on_panic() and that
> > handler must be as lockless as possible; so lockless that calling it
> > from anything that is not panic() is a severe bug.
> 
> This may not be a bad idea. And make it so it can't be called unless we
> are in panic mode (or at least "oops in progress").

right.

we used to have that zap_locks() function, which used to re-init printk()
internal locks on panic (printk recursion while in panic, to be exact):
logbuf spin_lock and console_sem. I wasn't to fond of this function, it
was missing the fact that on panic every printk() is a direct printk (at
least we have such expectation), IOW, it involves
	console_unlock()->call_console_drivers()

so punching printk()'s locks and leaving console drivers' locks intact
was not fair. at all. so, to improve the situation, I removed zap_locks().
/* kidding */


we have sort of re-entrant printk() now. but not completely re-entrant,
because console drivers are not re-entrant. so we can do

a) add ->zap_locks() callback to console drivers

   each console (which wants to be useful) can re-init its locks there, we
   will call it from panic() only. but, given how complex some of the
   consoles, I'd much rather prefer

b) add ->write_on_panic() callback to console drivers

   and do a barely legal print out there


I don't expect/want/push for/etc every console driver to implement
->write_on_panic() callback, just several most commonly used ones.
basically, the ones that you and PeterZ are using.


we also can split our flush_on_panic() and factor out the most
important part of console_unlock(). the first flush_on_panic(), let's
call it flush_on_panic_immediately() or whatever we name it, can push
messages only to those console drivers that have ->write_on_panic()
enabled. and it must call factored out part of console_unlock(). we
don't want flush_on_panic_immediately() to attempt up() the console
semaphore, because this can deadlock. so that factored out __console_unlock()
won't care about console_sem at all.


the second flush_on_panic() can push the data to all registered and
enabled consoles. this has chances to deadlock, but we can be less
nervous about it [given that there was at least one console with
->write_on_panic()].


> If oops_in_progress is set, and the console has a "write_on_panic"
> handler, then just call that.

yes. I don't like oops_in_progress variable, but some flag is
definitely needed.

> Heck, if it doesn't have one, and early_printk is defined, then perhaps
> that should be the default "write_on_panic" output?

yes, early_printk is a good addition. my systems have
"# CONFIG_EARLY_PRINTK is not set".

	-ss