Re: [RFC][PATCHv5 06/13] printk: register PM notifier

From: Rafael J. Wysocki
Date: Tue Aug 15 2017 - 07:59:50 EST


On Tuesday, August 15, 2017 4:56:18 AM CEST Sergey Senozhatsky wrote:
> It's not always possible/safe to wake_up() printk kernel
> thread. For example, late suspend/early resume may printk()
> while timekeeping is not initialized yet, so calling into the
> scheduler may result in recursive warnings.
>
> Another thing to notice is the fact that PM at some point
> freezes user space and kernel threads: freeze_processes()
> and freeze_kernel_threads(), correspondingly. Thus we need
> printk() to operate in emergency mode there and attempt to
> immediately flush pending kernel message to the console.
>
> This patch registers PM notifier, so PM can switch printk
> to emergency mode from PM_FOO_PREPARE notifiers and return

Isn't that too early? That's before user space is frozen even.

> back to printk threaded mode from PM_POST_FOO notifiers.

And isn't that too late?

> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>
> Suggested-by: Andreas Mohr <andi@xxxxxxxx>
> ---
> kernel/printk/printk.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 05165f008bc8..d3f149fad85c 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -49,6 +49,7 @@
> #include <linux/sched/debug.h>
> #include <linux/sched/task_stack.h>
> #include <linux/kthread.h>
> +#include <linux/suspend.h>
>
> #include <linux/uaccess.h>
> #include <asm/sections.h>
> @@ -2913,6 +2914,33 @@ static DEFINE_PER_CPU(struct irq_work, wake_up_klogd_work) = {
> .flags = IRQ_WORK_LAZY,
> };
>
> +static int printk_pm_notify(struct notifier_block *notify_block,
> + unsigned long mode, void *unused)
> +{
> + switch (mode) {
> + case PM_HIBERNATION_PREPARE:
> + case PM_SUSPEND_PREPARE:
> + case PM_RESTORE_PREPARE:
> + printk_emergency_begin_sync();

I'm not sure what would be wrong with calling this directly
from dpm_suspend_noirq().

> + break;
> +
> + case PM_POST_SUSPEND:
> + case PM_POST_HIBERNATION:
> + case PM_POST_RESTORE:
> + printk_emergency_end_sync();

And this could be called from dpm_resume_noirq().

In which case you wouldn't really need the stuff below.

Thanks,
Rafael