linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org
Subject: [PATCH printk v3 03/15] printk: add missing memory barrier to wake_up_klogd()
Date: Wed, 20 Apr 2022 01:52:25 +0206	[thread overview]
Message-ID: <20220419234637.357112-4-john.ogness@linutronix.de> (raw)
In-Reply-To: <20220419234637.357112-1-john.ogness@linutronix.de>

It is important that any new records are visible to preparing
waiters before the waker checks if the wait queue is empty.
Otherwise it is possible that:

- there are new records available
- the waker sees an empty wait queue and does not wake
- the preparing waiter sees no new records and begins to wait

This is exactly the problem that the function description of
waitqueue_active() warns about.

Use wq_has_sleeper() instead of waitqueue_active() because it
includes the necessary full memory barrier.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk.c | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 13a1eebe72af..f817dfb4852d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -746,8 +746,19 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
 			goto out;
 		}
 
+		/*
+		 * Guarantee this task is visible on the waitqueue before
+		 * checking the wake condition.
+		 *
+		 * The full memory barrier within set_current_state() of
+		 * prepare_to_wait_event() pairs with the full memory barrier
+		 * within wq_has_sleeper().
+		 *
+		 * This pairs with wake_up_klogd:A.
+		 */
 		ret = wait_event_interruptible(log_wait,
-				prb_read_valid(prb, atomic64_read(&user->seq), r));
+				prb_read_valid(prb,
+					atomic64_read(&user->seq), r)); /* LMM(devkmsg_read:A) */
 		if (ret)
 			goto out;
 	}
@@ -1513,7 +1524,18 @@ static int syslog_print(char __user *buf, int size)
 		seq = syslog_seq;
 
 		mutex_unlock(&syslog_lock);
-		len = wait_event_interruptible(log_wait, prb_read_valid(prb, seq, NULL));
+		/*
+		 * Guarantee this task is visible on the waitqueue before
+		 * checking the wake condition.
+		 *
+		 * The full memory barrier within set_current_state() of
+		 * prepare_to_wait_event() pairs with the full memory barrier
+		 * within wq_has_sleeper().
+		 *
+		 * This pairs with wake_up_klogd:A.
+		 */
+		len = wait_event_interruptible(log_wait,
+				prb_read_valid(prb, seq, NULL)); /* LMM(syslog_print:A) */
 		mutex_lock(&syslog_lock);
 
 		if (len)
@@ -3316,7 +3338,18 @@ void wake_up_klogd(void)
 		return;
 
 	preempt_disable();
-	if (waitqueue_active(&log_wait)) {
+	/*
+	 * Guarantee any new records can be seen by tasks preparing to wait
+	 * before this context checks if the wait queue is empty.
+	 *
+	 * The full memory barrier within wq_has_sleeper() pairs with the full
+	 * memory barrier within set_current_state() of
+	 * prepare_to_wait_event(), which is called after ___wait_event() adds
+	 * the waiter but before it has checked the wait condition.
+	 *
+	 * This pairs with devkmsg_read:A and syslog_print:A.
+	 */
+	if (wq_has_sleeper(&log_wait)) { /* LMM(wake_up_klogd:A) */
 		this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
 		irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
 	}
-- 
2.30.2


  parent reply	other threads:[~2022-04-19 23:46 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-19 23:46 [PATCH printk v3 00/15] printk/for-next John Ogness
2022-04-19 23:46 ` [PATCH printk v3 01/15] printk: rename cpulock functions John Ogness
2022-04-19 23:46 ` [PATCH printk v3 02/15] printk: cpu sync always disable interrupts John Ogness
2022-04-19 23:46 ` John Ogness [this message]
2022-04-20 12:34   ` [PATCH printk v3 03/15] printk: add missing memory barrier to wake_up_klogd() Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 04/15] printk: wake up all waiters John Ogness
2022-04-20 12:36   ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 05/15] printk: wake waiters for safe and NMI contexts John Ogness
2022-04-20 13:55   ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 06/15] printk: get caller_id/timestamp after migration disable John Ogness
2022-04-19 23:46 ` [PATCH printk v3 07/15] printk: call boot_delay_msec() in printk_delay() John Ogness
2022-04-19 23:46 ` [PATCH printk v3 08/15] printk: add con_printk() macro for console details John Ogness
2022-04-20 14:01   ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 09/15] printk: refactor and rework printing logic John Ogness
2022-04-20 14:55   ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 10/15] printk: move buffer definitions into console_emit_next_record() caller John Ogness
2022-04-19 23:46 ` [PATCH printk v3 11/15] printk: add pr_flush() John Ogness
2022-04-20 15:10   ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 12/15] printk: add functions to prefer direct printing John Ogness
2022-04-19 23:46 ` [PATCH printk v3 13/15] printk: add kthread console printers John Ogness
2022-04-20 17:53   ` Petr Mladek
2022-04-20 20:02     ` John Ogness
2022-04-21 14:25       ` Petr Mladek
2022-04-19 23:46 ` [PATCH printk v3 14/15] printk: extend console_lock for proper kthread support John Ogness
2022-04-20  2:13   ` kernel test robot
2022-04-20 13:32     ` John Ogness
2022-04-20  4:04   ` kernel test robot
2022-04-21 12:41   ` Petr Mladek
2022-04-21 14:30     ` John Ogness
2022-04-22 13:03       ` Petr Mladek
2022-04-22 14:14         ` John Ogness
2022-04-22 15:15           ` Petr Mladek
2022-04-22 21:25             ` John Ogness
2022-04-25 15:18               ` Petr Mladek
2022-04-25 19:10                 ` John Ogness
2022-04-19 23:46 ` [PATCH printk v3 15/15] printk: remove @console_locked John Ogness
2022-04-21 12:46   ` Petr Mladek
2022-04-21 14:40 ` [PATCH printk v3 00/15] printk/for-next Petr Mladek
2022-04-21 15:02   ` John Ogness

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=20220419234637.357112-4-john.ogness@linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=tglx@linutronix.de \
    /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).