From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
Viresh Kumar <viresh.kumar@linaro.org>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
rjw@rjwysocki.net, Tejun Heo <tj@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
vlevenetz@mm-sol.com, vaibhav.hiremath@linaro.org,
alex.elder@linaro.org, johan@kernel.org,
akpm@linux-foundation.org, rostedt@goodmis.org,
linux-pm@vger.kernel.org, Petr Mladek <pmladek@suse.com>,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [Query] Preemption (hogging) of the work handler
Date: Thu, 14 Jul 2016 23:34:50 +0900 [thread overview]
Message-ID: <20160714143450.GA619@swordfish> (raw)
In-Reply-To: <20160714141216.GC13151@quack2.suse.cz>
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?
> So there are IMHO two issues here worth looking at:
>
> 1) I didn't find how a wakeup would would lead to calling to ktime_get() in
> the current upstream kernel or even current RT kernel. Maybe this is a
> problem specific to the 3.10 kernel you are using? If yes, we don't have to
> do anything for current upstream AFAIU.
I personally suspect it's an in-hose (custom) code.
-ss
> If I just missed how wakeup can call into ktime_get() in current upstream,
> there is another question:
>
> 2) Is it OK that printk calls wakeup so late during suspend? I believe it
> is but I'm neither scheduler nor suspend expert. If it is OK, and wakeup
> can lead to ktime_get() in current upstream, then this contradicts the
> check WARN_ON(timekeeping_suspended) in ktime_get() and something is wrong.
>
> Adding Thomas to CC as timer / RT expert...
>
> Honza
>
> > so... I think we can switch to sync printk mode in suspend_console() and
> > enable async printk from resume_console(). IOW, suspend/kexec are now
> > executed under sync printk mode.
> >
> > we already call console_unlock() during suspend, which is synchronous,
> > many times (e.g. console_cpu_notify()).
> >
> >
> > something like below, perhaps. will this work for you?
> >
> > ---
> > kernel/printk/printk.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index bbb4180..786690e 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -288,6 +288,11 @@ static u32 log_buf_len = __LOG_BUF_LEN;
> >
> > /* Control whether printing to console must be synchronous. */
> > static bool __read_mostly printk_sync = true;
> > +/*
> > + * Force sync printk mode during suspend/kexec, regardless whether
> > + * console_suspend_enabled permits console suspend.
> > + */
> > +static bool __read_mostly force_printk_sync;
> > /* Printing kthread for async printk */
> > static struct task_struct *printk_kthread;
> > /* When `true' printing thread has messages to print */
> > @@ -295,7 +300,7 @@ static bool printk_kthread_need_flush_console;
> >
> > static inline bool can_printk_async(void)
> > {
> > - return !printk_sync && printk_kthread;
> > + return !printk_sync && printk_kthread && !force_printk_sync;
> > }
> >
> > /* Return log buffer address */
> > @@ -2027,6 +2032,7 @@ static bool suppress_message_printing(int level) { return false; }
> >
> > /* Still needs to be defined for users */
> > DEFINE_PER_CPU(printk_func_t, printk_func);
> > +static bool __read_mostly force_printk_sync;
> >
> > #endif /* CONFIG_PRINTK */
> >
> > @@ -2163,6 +2169,8 @@ MODULE_PARM_DESC(console_suspend, "suspend console during suspend"
> > */
> > void suspend_console(void)
> > {
> > + force_printk_sync = true;
> > +
> > if (!console_suspend_enabled)
> > return;
> > printk("Suspending console(s) (use no_console_suspend to debug)\n");
> > @@ -2173,6 +2181,8 @@ void suspend_console(void)
> >
> > void resume_console(void)
> > {
> > + force_printk_sync = false;
> > +
> > if (!console_suspend_enabled)
> > return;
> > down_console_sem();
> > --
> > 2.9.0.rc1
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
>
next prev parent reply other threads:[~2016-07-14 14:35 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-01 16:59 [Query] Preemption (hogging) of the work handler Viresh Kumar
2016-07-01 17:22 ` Tejun Heo
2016-07-01 17:28 ` Viresh Kumar
2016-07-06 18:28 ` Viresh Kumar
2016-07-06 19:23 ` Steven Rostedt
2016-07-06 19:25 ` Viresh Kumar
2016-07-11 10:26 ` Jan Kara
2016-07-11 15:44 ` Sergey Senozhatsky
2016-07-11 22:35 ` Viresh Kumar
2016-07-11 22:44 ` Rafael J. Wysocki
2016-07-11 22:46 ` Viresh Kumar
2016-07-12 12:24 ` Rafael J. Wysocki
2016-07-12 13:02 ` Viresh Kumar
2016-07-12 13:56 ` Petr Mladek
2016-07-12 14:04 ` Viresh Kumar
2016-07-12 9:38 ` Sergey Senozhatsky
2016-07-12 12:52 ` Petr Mladek
2016-07-12 13:12 ` Viresh Kumar
2016-07-12 17:11 ` Viresh Kumar
2016-07-12 19:59 ` Rafael J. Wysocki
2016-07-12 20:08 ` Viresh Kumar
2016-07-13 7:00 ` Sergey Senozhatsky
2016-07-13 12:05 ` Rafael J. Wysocki
2016-07-13 12:57 ` Sergey Senozhatsky
2016-07-13 13:22 ` Rafael J. Wysocki
2016-07-12 14:03 ` Sergey Senozhatsky
2016-07-12 14:12 ` Viresh Kumar
2016-07-14 23:52 ` Viresh Kumar
2016-07-15 13:11 ` Sergey Senozhatsky
2016-07-15 15:57 ` Viresh Kumar
2016-07-12 23:19 ` Viresh Kumar
2016-07-13 0:18 ` Viresh Kumar
2016-07-13 5:45 ` Sergey Senozhatsky
2016-07-13 15:39 ` Viresh Kumar
2016-07-13 23:08 ` Rafael J. Wysocki
2016-07-13 23:18 ` Viresh Kumar
2016-07-13 23:38 ` Greg Kroah-Hartman
2016-07-14 0:55 ` Sergey Senozhatsky
2016-07-14 1:09 ` Rafael J. Wysocki
2016-07-14 1:32 ` Sergey Senozhatsky
2016-07-14 21:57 ` Viresh Kumar
2016-07-14 21:55 ` Viresh Kumar
2016-07-14 14:12 ` Jan Kara
2016-07-14 14:33 ` Rafael J. Wysocki
2016-07-14 14:39 ` Jan Kara
2016-07-14 14:47 ` Rafael J. Wysocki
2016-07-14 14:55 ` Jan Kara
2016-07-14 22:14 ` Viresh Kumar
2016-07-14 14:34 ` Sergey Senozhatsky [this message]
2016-07-14 15:03 ` Jan Kara
2016-07-14 22:12 ` Viresh Kumar
2016-07-18 11:01 ` Jan Kara
2016-07-18 11:49 ` Rafael J. Wysocki
2016-07-29 20:42 ` Viresh Kumar
2016-07-30 2:12 ` Sergey Senozhatsky
2016-07-11 19:03 ` Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160714143450.GA619@swordfish \
--to=sergey.senozhatsky@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=alex.elder@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=jack@suse.cz \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=pmladek@suse.com \
--cc=rjw@rjwysocki.net \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=vaibhav.hiremath@linaro.org \
--cc=viresh.kumar@linaro.org \
--cc=vlevenetz@mm-sol.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).