[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160714150349.GH13151@quack2.suse.cz>
Date: Thu, 14 Jul 2016 17:03:49 +0200
From: Jan Kara <jack@...e.cz>
To: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc: Jan Kara <jack@...e.cz>,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
Viresh Kumar <viresh.kumar@...aro.org>, rjw@...ysocki.net,
Tejun Heo <tj@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
vlevenetz@...sol.com, vaibhav.hiremath@...aro.org,
alex.elder@...aro.org, johan@...nel.org, akpm@...ux-foundation.org,
rostedt@...dmis.org, linux-pm@...r.kernel.org,
Petr Mladek <pmladek@...e.com>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [Query] Preemption (hogging) of the work handler
On Thu 14-07-16 23:34:50, Sergey Senozhatsky wrote:
> Hello Jan,
>
> On (07/14/16 16:12), Jan Kara wrote:
> [..]
> > > *** a printk() call from here will kill the system. either it will
> > > recurse printk(), or spin forever in 'nested' printk() on one of
> > > the already taken spin locks.
> [..]
> > And with sync printk the above deadlock doesn't trigger only by chance - if
> > there happened to be a waiter on console_sem while we suspend, the same
> > deadlock would trigger because up(&console_sem) will try to wake him up and
> > the warning in timekeeping code will cause recursive printk.
> >
> > So I think your patch doesn't really address the real issue - it only
> > works around the particular WARN_ON(timekeeping_enabled) warning but if
> > there was a different warning in timekeeping code which would trigger, it
> > has a potential for causing recursive printk deadlock (and indeed we had
> > such issues previously - see e.g. 504d58745c9c "timer: Fix lock inversion
> > between hrtimer_bases.lock and scheduler locks").
>
> we switch to sync printk in suspend_console(), that is happening
> long before we start bringing cpu downs
>
> suspend_devices_and_enter()
> suspend_console()
> ...
> suspend_enter()
> ...
> dpm_suspend_late
> ...
> disable_nonboot_cpus
>
>
>
> and cpu_down() in printk does
>
> static int console_cpu_notify(struct notifier_block *self,
> unsigned long action, void *hcpu)
> {
> switch (action) {
> case CPU_ONLINE:
> case CPU_DEAD:
> case CPU_DOWN_FAILED:
> case CPU_UP_CANCELED:
> console_lock();
> console_unlock();
> }
> return NOTIFY_OK;
> }
>
> so I think this console_lock() sort of guarantees that there should be
> no sleeping tasks in console semaphore wait list. or am I missing something?
No, probably you're right - unless there would be a CPU notifier executed
after console_cpu_notify() which would try to acquire console_sem for some
reason. But that is a wild speculation and I tend to agree that in
synchronous printk case and current code the wakeup cannot happen.
But my point really is that I don't see why changing process state (which
is what wakeup actually is) should be problematic even this late during
suspend...
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists