From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Peter Zijlstra <peterz@infradead.org>,
x86@kernel.org, John Ogness <john.ogness@linutronix.de>,
Thomas Gleixner <tglx@linutronix.de>,
Andrew Morton <akpm@linux-foundation.org>,
Petr Tesarik <ptesarik@suse.cz>,
linux-kernel@vger.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Subject: Re: [PATCH 1/2] printk/panic: Access the main printk log in panic() only when safe
Date: Wed, 31 Jul 2019 15:59:49 +0900 [thread overview]
Message-ID: <20190731065949.GB1756@jagdpanzerIV> (raw)
In-Reply-To: <20190731060839.GA1756@jagdpanzerIV>
On (07/31/19 15:08), Sergey Senozhatsky wrote:
> When you have a chance, mind to take a look at the patch below?
> Doesn't look very difficult (half of it are white-spaces and
> comments, I believe).
I'm very sorry for annoyance.
Updated version:
-- passes !PRINTK build
-- moved WRITE_ONCE(console_waiter, false) in printk_enter_panic_mode()
-- added panic_in_progress_on_other_cpu() to console_trylock_spinning()
No more updates this week. Will wait for feedback.
Once again, sorry!
---
include/linux/printk.h | 5 ++++
kernel/panic.c | 9 +++++-
kernel/printk/printk.c | 64 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 57c9473f4a81..8293156d8243 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -245,6 +245,7 @@ extern asmlinkage void dump_stack(void) __cold;
extern void printk_safe_init(void);
extern void printk_safe_flush(void);
extern void printk_safe_flush_on_panic(void);
+extern void printk_enter_panic_mode(int cpu);
#else
static inline __printf(1, 0)
int vprintk(const char *s, va_list args)
@@ -320,6 +321,10 @@ static inline void printk_safe_flush(void)
static inline void printk_safe_flush_on_panic(void)
{
}
+
+static inline void printk_enter_panic_mode(int cpu)
+{
+}
#endif
extern int kptr_restrict;
diff --git a/kernel/panic.c b/kernel/panic.c
index d1ece4c363b9..85fac975a90f 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -254,13 +254,20 @@ void panic(const char *fmt, ...)
crash_smp_send_stop();
}
+ /* Misbehaving secondary CPUs cannot printk() to the main logbuf now */
+ printk_enter_panic_mode(this_cpu);
+
/*
* Run any panic handlers, including those that might need to
* add information to the kmsg dump output.
*/
atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
- /* Call flush even twice. It tries harder with a single online CPU */
+ /*
+ * Call flush even twice. It tries harder with a single online CPU.
+ * Even if we failed to stop some of secondary CPUs we have printk
+ * locks re-initialized and keep secondary CPUs off printk().
+ */
printk_safe_flush_on_panic();
kmsg_dump(KMSG_DUMP_PANIC);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index f0bc37a511a7..cd51aa7d08a9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1625,6 +1625,57 @@ static DEFINE_RAW_SPINLOCK(console_owner_lock);
static struct task_struct *console_owner;
static bool console_waiter;
+/*
+ * When system is in panic() this is used to permit printk() calls only from
+ * panic_cpu.
+ */
+static atomic_t __read_mostly printk_panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
+
+static int panic_in_progress_on_other_cpu(void)
+{
+ int cpu = atomic_read(&printk_panic_cpu);
+
+ return cpu != PANIC_CPU_INVALID && cpu != smp_processor_id();
+}
+
+void printk_enter_panic_mode(int cpu)
+{
+ unsigned long timeout;
+
+ cpu = atomic_cmpxchg(&printk_panic_cpu, PANIC_CPU_INVALID, cpu);
+ /* printk can enter panic mode only once */
+ if (cpu != PANIC_CPU_INVALID)
+ return;
+
+ WRITE_ONCE(console_waiter, false);
+
+ /*
+ * Wait for active secondary CPUs (if there are any) to leave
+ * console_unlock() printing loop (for up to one second).
+ */
+ if (num_online_cpus() > 1) {
+ timeout = USEC_PER_SEC;
+ while (num_online_cpus() > 1 && timeout--)
+ udelay(1);
+ }
+
+ debug_locks_off();
+ /*
+ * On some platforms crash_smp_send_stop() can kill CPUs via NMI
+ * vector. Re-init printk() locks just in case if any of those killed
+ * CPUs held any of printk() locks. On platforms which don't support
+ * NMI stop, misbehaving secondary CPUs will be handled by
+ * panic_in_progress_on_other_cpu() test.
+ *
+ * We re-init only printk() locks here. oops_in_progress is expected
+ * to be set by now, so good console drivers are in lockless mode,
+ * bad console drivers, however, can deadlock.
+ */
+ raw_spin_lock_init(&logbuf_lock);
+ sema_init(&console_sem, 1);
+ raw_spin_lock_init(&console_owner_lock);
+}
+
/**
* console_lock_spinning_enable - mark beginning of code where another
* thread might safely busy wait
@@ -1739,6 +1790,10 @@ static int console_trylock_spinning(void)
spin_release(&console_owner_dep_map, 1, _THIS_IP_);
printk_safe_exit_irqrestore(flags);
+
+ if (panic_in_progress_on_other_cpu())
+ return 0;
+
/*
* The owner passed the console lock to us.
* Since we did not spin on console lock, annotate
@@ -1900,6 +1955,9 @@ int vprintk_store(int facility, int level,
size_t text_len;
enum log_flags lflags = 0;
+ if (panic_in_progress_on_other_cpu())
+ return 0;
+
/*
* The printf needs to come first; we need the syslog
* prefix which might be passed-in as a parameter.
@@ -2076,6 +2134,7 @@ static ssize_t msg_print_ext_body(char *buf, size_t size,
char *text, size_t text_len) { return 0; }
static void console_lock_spinning_enable(void) { }
static int console_lock_spinning_disable_and_check(void) { return 0; }
+static int panic_in_progress_on_other_cpu(void) { return 0; }
static void call_console_drivers(const char *ext_text, size_t ext_len,
const char *text, size_t len) {}
static size_t msg_print_text(const struct printk_log *msg, bool syslog,
@@ -2468,6 +2527,11 @@ void console_unlock(void)
return;
}
+ if (panic_in_progress_on_other_cpu()) {
+ printk_safe_exit_irqrestore(flags);
+ return;
+ }
+
printk_safe_exit_irqrestore(flags);
if (do_cond_resched)
--
2.22.0
next prev parent reply other threads:[~2019-07-31 6:59 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 7:28 [PATCH 0/2] panic/printk/x86: Prevent some more printk-related deadlocks in panic() Petr Mladek
2019-07-16 7:28 ` [PATCH 1/2] printk/panic: Access the main printk log in panic() only when safe Petr Mladek
2019-07-17 9:56 ` Sergey Senozhatsky
2019-07-18 8:36 ` Petr Mladek
2019-07-18 9:49 ` Sergey Senozhatsky
2019-07-19 12:57 ` Petr Mladek
2019-07-23 3:13 ` Sergey Senozhatsky
2019-07-24 12:27 ` Petr Mladek
2019-07-31 6:08 ` Sergey Senozhatsky
2019-07-31 6:59 ` Sergey Senozhatsky [this message]
2019-07-18 10:11 ` Sergey Senozhatsky
2019-07-16 7:28 ` [PATCH 2/2] printk/panic/x86: Allow to access printk log buffer after crash_smp_send_stop() Petr Mladek
2019-07-18 10:47 ` Sergey Senozhatsky
2019-07-18 11:07 ` Konstantin Khlebnikov
2019-07-18 11:29 ` Sergey Senozhatsky
2019-07-19 12:19 ` Petr Mladek
2019-07-18 9:59 ` [PATCH 0/2] panic/printk/x86: Prevent some more printk-related deadlocks in panic() Konstantin Khlebnikov
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=20190731065949.GB1756@jagdpanzerIV \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=john.ogness@linutronix.de \
--cc=koct9i@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=ptesarik@suse.cz \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky@gmail.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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).