lkml.org 
[lkml]   [2017]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCHv3 5/5] printk: register PM notifier
    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@gmail.com>
    > Suggested-by: Andreas Mohr <andi@lisas.de>
    > ---
    > 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

    \
     
     \ /
      Last update: 2017-05-30 11:56    [W:5.790 / U:3.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site