Re: [RFC][PATCHv3 5/5] printk: register PM notifier

[Petr Mladek]

On Tue 2017-05-09 17:28:59, 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 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
> back to printk threaded mode from PM_POST_FOO notifiers.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>
> Suggested-by: Andreas Mohr <andi@xxxxxxxx>
> ---
>  kernel/printk/printk.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 81ea575728b9..6aae36a29aca 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2928,6 +2929,30 @@ 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();
> +		break;
> +
> +	case PM_POST_SUSPEND:
> +	case PM_POST_HIBERNATION:
> +	case PM_POST_RESTORE:
> +		printk_emergency_end();
> +		break;
> +	}
> +
> +	return 0;

Heh, it seems that the meaning of return values
is a bit unclear and messy. For example see an older problem
with android memory handlers at https://lkml.org/lkml/2012/4/9/177

My understanding is that notifiers should return NOTIFY_OK
when they handled it and NOTIFY_DONE when they did nothing.
For example, see cpu_hotplug_pm_callback().

In reality, there does not seem to be a big difference,
see notifier_to_errno() in __pm_notifier_call_chain().

Anyway, I would try to follow the cpu_hotplug_pm_callback()
example.


> +}
> +
> +static struct notifier_block printk_pm_nb = {
> +	.notifier_call = printk_pm_notify,
> +};
> +
>  static int printk_kthread_func(void *data)
>  {
>  	while (1) {
> @@ -2961,6 +2986,8 @@ static int __init init_printk_kthread(void)
>  
>  	sched_setscheduler(thread, SCHED_FIFO, &param);
>  	printk_kthread = thread;
> +
> +	WARN_ON(register_pm_notifier(&printk_pm_nb) != 0);

I think that a simple error message might be enough.

Also we might want to force the emergency mode in case of error.
Otherwise, the messages will not appear during suspend and such
operations.


BTW: Do you know about some locations that would need to be
patched explicitly, for example, kexec?

Some old versions of this patch touched console_suspend().
This brought me to snapshot_ioctl(). It looks like an
API that allows to create snapshots from user space.
I wonder if we should switch to the emergency mode
there are well, probably in the SNAPSHOT_FREEZE
stage.

Best Regards,
Petr