public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
	"Petr Mladek" <pmladek@suse.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Lech Perczak" <l.perczak@camlintechnologies.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Krzysztof Drobiński" <k.drobinski@camlintechnologies.com>,
	"Pawel Lenkow" <p.lenkow@camlintechnologies.com>,
	"John Ogness" <john.ogness@linutronix.de>,
	"Tejun Heo" <tj@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>
Subject: Re: Regression in v4.19.106 breaking waking up of readers of /proc/kmsg and /dev/kmsg
Date: Sat, 29 Feb 2020 13:08:06 +0900	[thread overview]
Message-ID: <20200229040806.GA72248@google.com> (raw)
In-Reply-To: <20200229033253.GA212847@google.com>

On (20/02/29 12:32), Sergey Senozhatsky wrote:
> > I'm wondering now if we should revert this commit before 5.6 comes out
> > (it landed in 5.6-rc1).  "Is much less likely to happen given the
> > other random initialization patches" is not the same as "guaranteed
> > not to happen".
> > 
> > What do folks think?
> 
> Well, my 5 cents, there is nothing that prevents "too-early"
> printk_deferred() calls in the future. From that POV I'd probably
> prefer to "forbid" printk_deffered() to touch per-CPU deferred
> machinery until it's not "too early" anymore. Similar to what we
> do in printk_safe::queue_flush_work().

Informal patch

=====
Subject: [PATCH] printk: defer printk_deferred() until irq_work is ready

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 kernel/printk/internal.h    |  3 +++
 kernel/printk/printk.c      |  6 ++++++
 kernel/printk/printk_safe.c | 11 ++++++++---
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index c8e6ab689d42..2f051fb83c00 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -23,6 +23,8 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args);
 void __printk_safe_enter(void);
 void __printk_safe_exit(void);
 
+bool printk_irq_work_ready(void);
+
 #define printk_safe_enter_irqsave(flags)	\
 	do {					\
 		local_irq_save(flags);		\
@@ -64,4 +66,5 @@ __printf(1, 0) int vprintk_func(const char *fmt, va_list args) { return 0; }
 #define printk_safe_enter_irq() local_irq_disable()
 #define printk_safe_exit_irq() local_irq_enable()
 
+bool printk_irq_work_ready(void) { return false; }
 #endif /* CONFIG_PRINTK */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ad4606234545..bb545c86124e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3009,6 +3009,9 @@ static DEFINE_PER_CPU(struct irq_work, wake_up_klogd_work) = {
 
 void wake_up_klogd(void)
 {
+	if (!printk_irq_work_ready())
+		return;
+
 	preempt_disable();
 	if (waitqueue_active(&log_wait)) {
 		this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
@@ -3019,6 +3022,9 @@ void wake_up_klogd(void)
 
 void defer_console_output(void)
 {
+	if (!printk_irq_work_ready())
+		return;
+
 	preempt_disable();
 	__this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT);
 	irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c
index b4045e782743..5d2c282984d0 100644
--- a/kernel/printk/printk_safe.c
+++ b/kernel/printk/printk_safe.c
@@ -27,7 +27,7 @@
  * There are situations when we want to make sure that all buffers
  * were handled or when IRQs are blocked.
  */
-static int printk_safe_irq_ready __read_mostly;
+static bool __printk_irq_work_ready __read_mostly;
 
 #define SAFE_LOG_BUF_LEN ((1 << CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT) -	\
 				sizeof(atomic_t) -			\
@@ -48,10 +48,15 @@ static DEFINE_PER_CPU(int, printk_context);
 static DEFINE_PER_CPU(struct printk_safe_seq_buf, nmi_print_seq);
 #endif
 
+bool printk_irq_work_ready(void)
+{
+	return __printk_irq_work_ready;
+}
+
 /* Get flushed in a more safe context. */
 static void queue_flush_work(struct printk_safe_seq_buf *s)
 {
-	if (printk_safe_irq_ready)
+	if (printk_irq_work_ready())
 		irq_work_queue(&s->work);
 }
 
@@ -408,7 +413,7 @@ void __init printk_safe_init(void)
 	 * variable is set.
 	 */
 	barrier();
-	printk_safe_irq_ready = 1;
+	__printk_irq_work_ready = true;
 
 	/* Flush pending messages that did not have scheduled IRQ works. */
 	printk_safe_flush();
-- 
2.25.0.265.gbab2e86ba0-goog


  reply	other threads:[~2020-02-29  4:08 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-27 11:09 Regression in v4.19.106 breaking waking up of readers of /proc/kmsg and /dev/kmsg Lech Perczak
2020-02-27 12:36 ` Greg Kroah-Hartman
2020-02-27 12:39   ` Lech Perczak
2020-02-27 14:08     ` Lech Perczak
2020-02-28  3:13       ` Sergey Senozhatsky
2020-02-28  9:11         ` John Ogness
2020-02-28 12:02           ` Sergey Senozhatsky
2020-02-28 10:04         ` Petr Mladek
2020-02-28 10:58           ` Greg Kroah-Hartman
2020-02-28 11:32             ` Petr Mladek
2020-02-28 11:39               ` Greg Kroah-Hartman
2020-02-28 13:02               ` Petr Mladek
2020-02-28 13:41                 ` Greg Kroah-Hartman
2020-02-28 20:53                 ` Theodore Y. Ts'o
2020-02-29  3:32                   ` Sergey Senozhatsky
2020-02-29  4:08                     ` Sergey Senozhatsky [this message]
2020-02-29 23:47                     ` Steven Rostedt
2020-03-01  5:22                       ` Sergey Senozhatsky
2020-03-02  9:49                         ` Petr Mladek
2020-03-02  9:59                           ` Sergey Senozhatsky
2020-02-28 11:49           ` 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=20200229040806.GA72248@google.com \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=k.drobinski@camlintechnologies.com \
    --cc=l.perczak@camlintechnologies.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.lenkow@camlintechnologies.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=tytso@mit.edu \
    /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