From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759108AbdJQHuW (ORCPT ); Tue, 17 Oct 2017 03:50:22 -0400 Received: from mail-pf0-f177.google.com ([209.85.192.177]:56139 "EHLO mail-pf0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753104AbdJQHuU (ORCPT ); Tue, 17 Oct 2017 03:50:20 -0400 X-Google-Smtp-Source: AOwi7QAdkSH+Zrb7xJuWLXJ8qmo0cyhirFL15xZneFQv1tMLx1SPP6e4sbHoekfwOi3P9Dwsz4hjtA== Date: Tue, 17 Oct 2017 16:50:15 +0900 From: Sergey Senozhatsky To: Steven Rostedt Cc: Sergey Senozhatsky , Petr Mladek , Linus Torvalds , LKML , Peter Zijlstra , Andrew Morton , Thomas Gleixner , Ingo Molnar Subject: Re: NMI watchdog dump does not print on hard lockup Message-ID: <20171017075015.GA6915@jagdpanzerIV> References: <20171012121658.187c5af6@gandalf.local.home> <20171013111444.GB2795@pathway.suse.cz> <20171013091857.4afe8a7a@gandalf.local.home> <20171016111239.GK2795@pathway.suse.cz> <20171016131305.GE6316@tigerII.localdomain> <20171016101547.27b4aa11@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171016101547.27b4aa11@gandalf.local.home> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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