Messages in this thread | | | Date | Wed, 19 Aug 2015 11:32:15 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH v4] fs/pstore: provide panic data even in suspend |
| |
On Mon 2015-08-17 16:51:39, check.kernel@gmail.com wrote: > From: Dongdong Yang <yangdongdong@xiaomi.com> > > 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 <yangdongdong@xiaomi.com> > Signed-off-by: Linghua Gu <gulinghua@xiaomi.com> > Signed-off-by: Hui Du <duhui@xiaomi.com> > --- > 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
| |