All of lore.kernel.org
 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,
	Andrew Morton <akpm@linux-foundation.org>,
	Dmitry Safonov <0x7f454c46@gmail.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Peter Zijlstra <peterz@infradead.org>,
	Marco Elver <elver@google.com>,
	Alexander Potapenko <glider@google.com>,
	Stephen Boyd <swboyd@chromium.org>
Subject: [PATCH next v2 1/2] dump_stack: move cpu lock to printk.c
Date: Mon,  7 Jun 2021 22:02:31 +0200	[thread overview]
Message-ID: <20210607200232.22211-2-john.ogness@linutronix.de> (raw)
In-Reply-To: <20210607200232.22211-1-john.ogness@linutronix.de>

dump_stack() implements its own cpu-reentrant spinning lock to
best-effort serialize stack traces in the printk log. However,
there are other functions (such as show_regs()) that can also
benefit from this serialization.

Move the cpu-reentrant spinning lock (cpu lock) into new helper
functions printk_cpu_lock_irqsave()/printk_cpu_unlock_irqrestore()
so that it is available for others as well. For !CONFIG_SMP the
cpu lock is a NOP.

Note that having multiple cpu locks in the system can easily
lead to deadlock. Code needing a cpu lock should use the
printk cpu lock, since the printk cpu lock could be acquired
from any code and any context.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 include/linux/printk.h | 13 ++++++++
 kernel/printk/printk.c | 75 ++++++++++++++++++++++++++++++++++++++++++
 lib/dump_stack.c       | 41 +++--------------------
 3 files changed, 92 insertions(+), 37 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index f589b8b60806..b84e0c59220f 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -287,6 +287,19 @@ static inline void printk_safe_flush_on_panic(void)
 }
 #endif
 
+#if defined(CONFIG_SMP)
+extern void printk_cpu_lock_irqsave(bool *lock_flag, unsigned long *irq_flags);
+extern void printk_cpu_unlock_irqrestore(bool lock_flag, unsigned long irq_flags);
+#else
+static inline void printk_cpu_lock_irqsave(bool *lock_flag, unsigned long *irq_flags)
+{
+}
+
+static inline void printk_cpu_unlock_irqrestore(bool lock_flag, unsigned long irq_flags)
+{
+}
+#endif
+
 extern int kptr_restrict;
 
 /**
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 114e9963f903..f94babb38493 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3532,3 +3532,78 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
 EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
 
 #endif
+
+#ifdef CONFIG_SMP
+static atomic_t printk_cpulock_owner = ATOMIC_INIT(-1);
+
+/*
+ * printk_cpu_lock_irqsave: Acquire the printk cpu-reentrant spinning lock
+ *                          and disable interrupts.
+ * @lock_flag: A buffer to store lock state.
+ * @irq_flags: A buffer to store irq state.
+ *
+ * If no processor has the lock, the calling processor takes the lock and
+ * becomes the owner. If the calling processor is already the owner of the
+ * lock, this function succeeds immediately. If the lock is held by another
+ * processor, this function spins until the calling processor becomes the
+ * owner. This function returns with interrupts disabled.
+ *
+ * It is safe to call this function from any context and state.
+ */
+void printk_cpu_lock_irqsave(bool *lock_flag, unsigned long *irq_flags)
+{
+	int old;
+	int cpu;
+
+retry:
+	local_irq_save(*irq_flags);
+
+	cpu = smp_processor_id();
+
+	old = atomic_cmpxchg(&printk_cpulock_owner, -1, cpu);
+	if (old == -1) {
+		/* This CPU is now the owner. */
+
+		*lock_flag = true;
+
+	} else if (old == cpu) {
+		/* This CPU is already the owner. */
+
+		*lock_flag = false;
+
+	} else {
+		local_irq_restore(*irq_flags);
+
+		/*
+		 * Wait for the lock to release before jumping to cmpxchg()
+		 * in order to mitigate the thundering herd problem.
+		 */
+		do {
+			cpu_relax();
+		} while (atomic_read(&printk_cpulock_owner) != -1);
+
+		goto retry;
+	}
+}
+EXPORT_SYMBOL(printk_cpu_lock_irqsave);
+
+/*
+ * printk_cpu_unlock_irqrestore: Release the printk cpu-reentrant spinning
+ *                               lock and restore interrupts.
+ * @lock_flag: The current lock state.
+ * @irq_flags: The current irq state.
+ *
+ * Release the lock. The calling processor must be the owner of the lock.
+ *
+ * It is safe to call this function from any context and state.
+ */
+void printk_cpu_unlock_irqrestore(bool lock_flag, unsigned long irq_flags)
+{
+	if (lock_flag) {
+		atomic_set(&printk_cpulock_owner, -1);
+
+		local_irq_restore(irq_flags);
+	}
+}
+EXPORT_SYMBOL(printk_cpu_unlock_irqrestore);
+#endif /* CONFIG_SMP */
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 6e7ca3d67710..84c68bad94c7 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -93,52 +93,19 @@ static void __dump_stack(const char *log_lvl)
  *
  * Architectures can override this implementation by implementing its own.
  */
-#ifdef CONFIG_SMP
-static atomic_t dump_lock = ATOMIC_INIT(-1);
-
 asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
 {
-	unsigned long flags;
-	int was_locked;
-	int old;
-	int cpu;
+	unsigned long irq_flags;
+	bool lock_flag;
 
 	/*
 	 * Permit this cpu to perform nested stack dumps while serialising
 	 * against other CPUs
 	 */
-retry:
-	local_irq_save(flags);
-	cpu = smp_processor_id();
-	old = atomic_cmpxchg(&dump_lock, -1, cpu);
-	if (old == -1) {
-		was_locked = 0;
-	} else if (old == cpu) {
-		was_locked = 1;
-	} else {
-		local_irq_restore(flags);
-		/*
-		 * Wait for the lock to release before jumping to
-		 * atomic_cmpxchg() in order to mitigate the thundering herd
-		 * problem.
-		 */
-		do { cpu_relax(); } while (atomic_read(&dump_lock) != -1);
-		goto retry;
-	}
-
-	__dump_stack(log_lvl);
-
-	if (!was_locked)
-		atomic_set(&dump_lock, -1);
-
-	local_irq_restore(flags);
-}
-#else
-asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
-{
+	printk_cpu_lock_irqsave(&lock_flag, &irq_flags);
 	__dump_stack(log_lvl);
+	printk_cpu_unlock_irqrestore(lock_flag, irq_flags);
 }
-#endif
 EXPORT_SYMBOL(dump_stack_lvl);
 
 asmlinkage __visible void dump_stack(void)
-- 
2.20.1


  reply	other threads:[~2021-06-07 20:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 20:02 [PATCH next v2 0/2] introduce printk cpu lock John Ogness
2021-06-07 20:02 ` John Ogness [this message]
2021-06-08  2:43   ` [PATCH next v2 1/2] dump_stack: move cpu lock to printk.c kernel test robot
2021-06-08  2:43     ` kernel test robot
2021-06-08 13:48     ` Petr Mladek
2021-06-08 13:48       ` Petr Mladek
2021-06-10 13:26       ` John Ogness
2021-06-10 13:26         ` John Ogness
2021-06-11  7:00         ` Petr Mladek
2021-06-11  7:00           ` Petr Mladek
2021-06-08 11:40   ` Petr Mladek
2021-06-08 13:55     ` John Ogness
2021-06-08 14:54       ` Petr Mladek
2021-06-07 20:02 ` [PATCH next v2 2/2] printk: fix cpu lock ordering John Ogness
2021-06-08 12:55   ` Petr Mladek
2021-06-08 14:18     ` John Ogness
2021-06-08 14:49       ` Petr Mladek
2021-06-10 14:44         ` 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=20210607200232.22211-2-john.ogness@linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=sfr@canb.auug.org.au \
    --cc=swboyd@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 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.