All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: lkp@01.org, LKML <linux-kernel@vger.kernel.org>,
	Kyle McMartin <kyle@kernel.org>,
	Dave Jones <davej@codemonkey.org.uk>, Jan Kara <jack@suse.com>,
	Calvin Owens <calvinowens@fb.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	kernel test robot <ying.huang@linux.intel.com>,
	kernel-team@fb.com
Subject: [PATCH] printk: clear console_may_schedule on panic flushing
Date: Mon, 11 Jan 2016 13:43:48 -0500	[thread overview]
Message-ID: <20160111184348.GF1898@mtj.duckdns.org> (raw)
In-Reply-To: <878u3w95ke.fsf@yhuang-dev.intel.com>

Commit "printk: do cond_resched() between lines while outputting to
consoles" made console flushing perform cond_resched() after each line
if the context allows as determined by whether the console lock was
acquired with console_lock().  The condition is carried in
console_may_schedule.

During panic, console lock status is ignored and the messages are
forcifully flushed which is implemented by performing
console_trylock(); console_unlock(); sequence ignoring whether trylock
succeeds or fails.  This means that the emergency flushing, after
trylock failure, may enter flushing path with console_may_schedule set
from the actual holder.

As a system may panic from any context, this can lead to
cond_resched() being invoked from a non-sleepable context triggering
an extra warning dump while panicking which is noisy and can be
confusing.  Besides, even when panicking from a sleepable context, we
don't want to be yielding during emergency message dumping.

Currently, the emergency dumping is opencoded in panic().  This patch
replaces the open coded implementation with a new function,
console_flush_on_panic() and makes it explicitly clear
console_may_schedule before starting the emergency flushing.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: kernel test robot <ying.huang@linux.intel.com>
Link: http://lkml.kernel.org/g/878u3w95ke.fsf@yhuang-dev.intel.com
---
 include/linux/console.h |    1 +
 kernel/panic.c          |    3 +--
 kernel/printk/printk.c  |   19 +++++++++++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/linux/console.h b/include/linux/console.h
index bd19434..ea731af 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -150,6 +150,7 @@ extern int console_trylock(void);
 extern void console_unlock(void);
 extern void console_conditional_schedule(void);
 extern void console_unblank(void);
+extern void console_flush_on_panic(void);
 extern struct tty_driver *console_device(int *);
 extern void console_stop(struct console *);
 extern void console_start(struct console *);
diff --git a/kernel/panic.c b/kernel/panic.c
index b333380..d96469d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -180,8 +180,7 @@ void panic(const char *fmt, ...)
 	 * panic() is not being callled from OOPS.
 	 */
 	debug_locks_off();
-	console_trylock();
-	console_unlock();
+	console_flush_on_panic();
 
 	if (!panic_blink)
 		panic_blink = no_blink;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index d0b8697..7ebcfea 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2386,6 +2386,25 @@ void console_unblank(void)
 	console_unlock();
 }
 
+/**
+ * console_flush_on_panic - flush console content on panic
+ *
+ * Immediately output all pending messages no matter what.
+ */
+void console_flush_on_panic(void)
+{
+	/*
+	 * If someone else is holding the console lock, trylock will fail
+	 * and may_schedule may be set.  Ignore and proceed to unlock so
+	 * that messages are flushed out.  As this can be called from any
+	 * context and we don't want to get preempted while flushing,
+	 * ensure may_schedule is cleared.
+	 */
+	console_trylock();
+	console_may_schedule = 0;
+	console_unlock();
+}
+
 /*
  * Return the console tty driver structure and its associated index
  */

WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: lkp@lists.01.org
Subject: [PATCH] printk: clear console_may_schedule on panic flushing
Date: Mon, 11 Jan 2016 13:43:48 -0500	[thread overview]
Message-ID: <20160111184348.GF1898@mtj.duckdns.org> (raw)
In-Reply-To: <878u3w95ke.fsf@yhuang-dev.intel.com>

[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]

Commit "printk: do cond_resched() between lines while outputting to
consoles" made console flushing perform cond_resched() after each line
if the context allows as determined by whether the console lock was
acquired with console_lock().  The condition is carried in
console_may_schedule.

During panic, console lock status is ignored and the messages are
forcifully flushed which is implemented by performing
console_trylock(); console_unlock(); sequence ignoring whether trylock
succeeds or fails.  This means that the emergency flushing, after
trylock failure, may enter flushing path with console_may_schedule set
from the actual holder.

As a system may panic from any context, this can lead to
cond_resched() being invoked from a non-sleepable context triggering
an extra warning dump while panicking which is noisy and can be
confusing.  Besides, even when panicking from a sleepable context, we
don't want to be yielding during emergency message dumping.

Currently, the emergency dumping is opencoded in panic().  This patch
replaces the open coded implementation with a new function,
console_flush_on_panic() and makes it explicitly clear
console_may_schedule before starting the emergency flushing.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: kernel test robot <ying.huang@linux.intel.com>
Link: http://lkml.kernel.org/g/878u3w95ke.fsf(a)yhuang-dev.intel.com
---
 include/linux/console.h |    1 +
 kernel/panic.c          |    3 +--
 kernel/printk/printk.c  |   19 +++++++++++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/linux/console.h b/include/linux/console.h
index bd19434..ea731af 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -150,6 +150,7 @@ extern int console_trylock(void);
 extern void console_unlock(void);
 extern void console_conditional_schedule(void);
 extern void console_unblank(void);
+extern void console_flush_on_panic(void);
 extern struct tty_driver *console_device(int *);
 extern void console_stop(struct console *);
 extern void console_start(struct console *);
diff --git a/kernel/panic.c b/kernel/panic.c
index b333380..d96469d 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -180,8 +180,7 @@ void panic(const char *fmt, ...)
 	 * panic() is not being callled from OOPS.
 	 */
 	debug_locks_off();
-	console_trylock();
-	console_unlock();
+	console_flush_on_panic();
 
 	if (!panic_blink)
 		panic_blink = no_blink;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index d0b8697..7ebcfea 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2386,6 +2386,25 @@ void console_unblank(void)
 	console_unlock();
 }
 
+/**
+ * console_flush_on_panic - flush console content on panic
+ *
+ * Immediately output all pending messages no matter what.
+ */
+void console_flush_on_panic(void)
+{
+	/*
+	 * If someone else is holding the console lock, trylock will fail
+	 * and may_schedule may be set.  Ignore and proceed to unlock so
+	 * that messages are flushed out.  As this can be called from any
+	 * context and we don't want to get preempted while flushing,
+	 * ensure may_schedule is cleared.
+	 */
+	console_trylock();
+	console_may_schedule = 0;
+	console_unlock();
+}
+
 /*
  * Return the console tty driver structure and its associated index
  */

  reply	other threads:[~2016-01-11 18:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-11  3:08 [lkp] [printk] db43e77a44: BUG: sleeping function called from invalid context at kernel/printk/printk.c:2328 kernel test robot
2016-01-11  3:08 ` kernel test robot
2016-01-11 18:43 ` Tejun Heo [this message]
2016-01-11 18:43   ` [PATCH] printk: clear console_may_schedule on panic flushing Tejun Heo
2016-01-11 21:59   ` Andrew Morton
2016-01-11 21:59     ` Andrew Morton
2016-01-11 22:09     ` Tejun Heo
2016-01-11 22:09       ` Tejun Heo
2016-01-11 22:25       ` Tejun Heo
2016-01-11 22:25         ` Tejun Heo
2016-01-12 13:50     ` Sergey Senozhatsky
2016-01-12 13:50       ` Sergey Senozhatsky

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=20160111184348.GF1898@mtj.duckdns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=calvinowens@fb.com \
    --cc=davej@codemonkey.org.uk \
    --cc=jack@suse.com \
    --cc=kernel-team@fb.com \
    --cc=kyle@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@01.org \
    --cc=sfr@canb.auug.org.au \
    --cc=ying.huang@linux.intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.