Messages in this thread | | | Date | Tue, 17 Oct 2017 16:50:15 +0900 | From | Sergey Senozhatsky <> | Subject | Re: NMI watchdog dump does not print on hard lockup |
| |
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
| |