From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753553AbbHSJcZ (ORCPT ); Wed, 19 Aug 2015 05:32:25 -0400 Received: from mx2.suse.de ([195.135.220.15]:36758 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751516AbbHSJcW (ORCPT ); Wed, 19 Aug 2015 05:32:22 -0400 Date: Wed, 19 Aug 2015 11:32:15 +0200 From: Petr Mladek To: check.kernel@gmail.com Cc: Joe Perches , Alex Elder , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Andrew Morton , "Luis R. Rodriguez" , Peter Hurley , Tejun Heo , Hui Du , Linghua Gu , linux-kernel@vger.kernel.org, Dongdong Yang Subject: Re: [PATCH v4] fs/pstore: provide panic data even in suspend Message-ID: <20150819093215.GG3829@pathway.suse.cz> References: <1439801499-1818-1-git-send-email-check.kernel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1439801499-1818-1-git-send-email-check.kernel@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 2015-08-17 16:51:39, check.kernel@gmail.com wrote: > From: Dongdong Yang > > If system restart after panic, this patch also enables > panic and oops messages which in suspend context to be > logged into ramoops console buffer where it can be read > back at some later point. > > Signed-off-by: Dongdong Yang > Signed-off-by: Linghua Gu > Signed-off-by: Hui Du > --- > fs/pstore/ram.c | 16 ++++++++++++++++ > include/linux/printk.h | 6 ++++++ > kernel/printk/printk.c | 6 ++++++ > 3 files changed, 28 insertions(+) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index 6c26c4d..3d981a1 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -642,8 +642,23 @@ static void ramoops_register_dummy(void) > } > } > > +static int ramoops_console_notify(struct notifier_block *this, > + unsigned long event, void *ptr) > +{ > + pr_emerg("ramoops unlock console ...\n"); > + emergency_unlock_console(); > + > + return 0; > +} > + > +static struct notifier_block ramoop_nb = { > + .notifier_call = ramoops_console_notify, > + .priority = INT_MAX, > +}; > + > static int __init ramoops_init(void) > { > + atomic_notifier_chain_register(&panic_notifier_list, &ramoop_nb); > ramoops_register_dummy(); > return platform_driver_register(&ramoops_driver); > } > @@ -654,6 +669,7 @@ static void __exit ramoops_exit(void) > platform_driver_unregister(&ramoops_driver); > platform_device_unregister(dummy); > kfree(dummy_data); > + atomic_notifier_chain_unregister(&panic_notifier_list, &ramoop_nb); > } > module_exit(ramoops_exit); > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index a6298b2..bd043ed 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -230,6 +230,12 @@ static inline void show_regs_print_info(const char *log_lvl) > } > #endif > > +/* > + * Enables printk() before emergency_restart > + * if we go into suspend states and run into oops. > + */ > +void emergency_unlock_console(void); > + > extern asmlinkage void dump_stack(void) __cold; > > #ifndef pr_fmt > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index cf8c242..abb8af4 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2107,6 +2107,12 @@ void resume_console(void) > console_unlock(); > } > > +void emergency_unlock_console(void) > +{ > + if (oops_in_progress && panic_timeout != 0 && console_suspended) > + resume_console(); IMHO, you need to put "console_sem" down if you want to test "console_suspended". And you must keep it down until you resume the console to avoid a race => we could not call resume_console() in the current form. Also I am a bit nervous that this operation is not symmetric and the console will stay resumed. Please look at bust_spinlocks(), how this function is used in panic(), and see how the "oops_in_progress" is handled. To be honest, I do not understand this code in details enough and I do not have time to investigate it at the moment. I just use a common sense to produce this feedback. Others might give you a better feedback. It is possible that this should get handled a more generic way and not by a ramoops specific handler. BTW: Have you tested this with more consoles attached? I wonder if other consoles are still usable in this situation and if they will not block the output to ramoops. Best Regards, Petr > +} > + > /** > * console_cpu_notify - print deferred console messages after CPU hotplug > * @self: notifier struct > -- > 2.5.0 > > > v4: > - Move declaration emergency_unlock_console to printk.h from pstore_ram.h > - Update sign-off information. > > Thanks, > Dongdong Yang