This patch introduces four new helper functions to abstract
the management of the panic_cpu variable. These functions
will be used in subsequent patches to refactor existing code.
The direct use of panic_cpu can be error-prone and
ambiguous, as it requires manual checks to determine which
CPU is handling the panic. The new helpers clarify intent:
panic_try_start():
Atomically sets the current CPU as the panicking CPU.
panic_reset():
Reset panic_cpu to PANIC_CPU_INVALID.
panic_in_progress():
Checks if a panic has been triggered.
panic_on_this_cpu():
Returns true if the current CPU is the panic originator.
panic_on_other_cpu():
Returns true if a panic is on another CPU.
This change lays the groundwork for improved code readability
and robustness in the panic handling subsystem.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
include/linux/panic.h | 6 +++++
kernel/panic.c | 53 ++++++++++++++++++++++++++++++++++++++++++
kernel/printk/printk.c | 5 ----
3 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/include/linux/panic.h b/include/linux/panic.h
index 7be742628c25..6f972a66c13e 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -43,6 +43,12 @@ void abort(void);
extern atomic_t panic_cpu;
#define PANIC_CPU_INVALID -1
+bool panic_try_start(void);
+void panic_reset(void);
+bool panic_in_progress(void);
+bool panic_on_this_cpu(void);
+bool panic_on_other_cpu(void);
+
/*
* Only to be used by arch init code. If the user over-wrote the default
* CONFIG_PANIC_TIMEOUT, honor it.
diff --git a/kernel/panic.c b/kernel/panic.c
index 72fcbb5a071b..eacb0c972110 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -294,6 +294,59 @@ void __weak crash_smp_send_stop(void)
atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
+bool panic_try_start(void)
+{
+ int old_cpu, this_cpu;
+
+ /*
+ * Only one CPU is allowed to execute the crash_kexec() code as with
+ * panic(). Otherwise parallel calls of panic() and crash_kexec()
+ * may stop each other. To exclude them, we use panic_cpu here too.
+ */
+ old_cpu = PANIC_CPU_INVALID;
+ this_cpu = raw_smp_processor_id();
+
+ return atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu);
+}
+EXPORT_SYMBOL(panic_try_start);
+
+void panic_reset(void)
+{
+ atomic_set(&panic_cpu, PANIC_CPU_INVALID);
+}
+EXPORT_SYMBOL(panic_reset);
+
+bool panic_in_progress(void)
+{
+ return unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID);
+}
+EXPORT_SYMBOL(panic_in_progress);
+
+/* Return true if a panic is in progress on the current CPU. */
+bool panic_on_this_cpu(void)
+{
+ /*
+ * We can use raw_smp_processor_id() here because it is impossible for
+ * the task to be migrated to the panic_cpu, or away from it. If
+ * panic_cpu has already been set, and we're not currently executing on
+ * that CPU, then we never will be.
+ */
+ return unlikely(atomic_read(&panic_cpu) == raw_smp_processor_id());
+}
+EXPORT_SYMBOL(panic_on_this_cpu);
+
+/*
+ * Return true if a panic is in progress on a remote CPU.
+ *
+ * On true, the local CPU should immediately release any printing resources
+ * that may be needed by the panic CPU.
+ */
+bool panic_on_other_cpu(void)
+{
+ return (panic_in_progress() && !this_cpu_in_panic());
+}
+EXPORT_SYMBOL(panic_on_other_cpu);
+
/*
* A variant of panic() called from NMI context. We return if we've already
* panicked on this CPU. If another CPU already panicked, loop in
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0efbcdda9aab..5fe35f377b79 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -345,11 +345,6 @@ static void __up_console_sem(unsigned long ip)
}
#define up_console_sem() __up_console_sem(_RET_IP_)
-static bool panic_in_progress(void)
-{
- return unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID);
-}
-
/* Return true if a panic is in progress on the current CPU. */
bool this_cpu_in_panic(void)
{
--
2.43.0This patch updates the fbcon_skip_panic() function to use
the panic_in_progress() helper.
The previous direct access to panic_cpu is less
readable and is being replaced by a dedicated function
that more clearly expresses the intent.
This change is part of a series to refactor the kernel's
panic handling logic for better clarity and robustness.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
drivers/video/fbdev/core/fbcon.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 55f5731e94c3..b062b05f4128 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -279,14 +279,7 @@ static int fbcon_get_rotate(struct fb_info *info)
static bool fbcon_skip_panic(struct fb_info *info)
{
-/* panic_cpu is not exported, and can't be used if built as module. Use
- * oops_in_progress instead, but non-fatal oops won't be printed.
- */
-#if defined(MODULE)
- return (info->skip_panic && unlikely(oops_in_progress));
-#else
- return (info->skip_panic && unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID));
-#endif
+ return (info->skip_panic && unlikely(panic_in_progress()));
}
static inline bool fbcon_is_active(struct vc_data *vc, struct fb_info *info)
--
2.43.0On Wed, Aug 20, 2025 at 05:14:47PM +0800, Jinchao Wang wrote:
> This patch updates the fbcon_skip_panic() function to use
> the panic_in_progress() helper.
>
> The previous direct access to panic_cpu is less
> readable and is being replaced by a dedicated function
> that more clearly expresses the intent.
>
> This change is part of a series to refactor the kernel's
> panic handling logic for better clarity and robustness.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> drivers/video/fbdev/core/fbcon.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 55f5731e94c3..b062b05f4128 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -279,14 +279,7 @@ static int fbcon_get_rotate(struct fb_info *info)
>
> static bool fbcon_skip_panic(struct fb_info *info)
> {
> -/* panic_cpu is not exported, and can't be used if built as module. Use
> - * oops_in_progress instead, but non-fatal oops won't be printed.
> - */
> -#if defined(MODULE)
> - return (info->skip_panic && unlikely(oops_in_progress));
> -#else
> - return (info->skip_panic && unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID));
> -#endif
> + return (info->skip_panic && unlikely(panic_in_progress()));
> }
>
> static inline bool fbcon_is_active(struct vc_data *vc, struct fb_info *info)
> --
> 2.43.0
Acked-by Qianqiang Liu <qianqiang.liu@163.com>
--
Best,
Qianqiang Liu
crash_kexec() had its own code to exclude
parallel execution by setting panic_cpu.
This is already handled by panic_try_start().
Switch to panic_try_start() to remove the
duplication and keep the logic consistent.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/crash_core.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index a4ef79591eb2..bb38bbaf3a26 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -4,6 +4,7 @@
* Copyright (C) 2002-2004 Eric Biederman <ebiederm@xmission.com>
*/
+#include "linux/panic.h"
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/buildid.h>
@@ -143,17 +144,7 @@ STACK_FRAME_NON_STANDARD(__crash_kexec);
__bpf_kfunc void crash_kexec(struct pt_regs *regs)
{
- int old_cpu, this_cpu;
-
- /*
- * Only one CPU is allowed to execute the crash_kexec() code as with
- * panic(). Otherwise parallel calls of panic() and crash_kexec()
- * may stop each other. To exclude them, we use panic_cpu here too.
- */
- old_cpu = PANIC_CPU_INVALID;
- this_cpu = raw_smp_processor_id();
-
- if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu)) {
+ if (panic_try_start()) {
/* This is the 1st CPU which comes here, so go ahead. */
__crash_kexec(regs);
@@ -161,7 +152,7 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
* Reset panic_cpu to allow another panic()/crash_kexec()
* call.
*/
- atomic_set(&panic_cpu, PANIC_CPU_INVALID);
+ panic_reset();
}
}
--
2.43.0On 08/20/25 at 05:14pm, Jinchao Wang wrote:
> crash_kexec() had its own code to exclude
> parallel execution by setting panic_cpu.
> This is already handled by panic_try_start().
>
> Switch to panic_try_start() to remove the
> duplication and keep the logic consistent.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
I had to use b4 to grab back the whole patchset, but I can't comment on
other patches, especially the patch 1.
Firstly, this series looks interesting. It does enhance code
readibility. But I am a vim user, I like open code on this one line of
code wrapping. So leave this to other reviewers to decide if this should
be accepted.
Secondly, the lines of your patch log are too short, it's not convenient
for reading. Can you set your mail writer to change this.
Thirdly, please add people to CC in all patches. I don't know why you
only CC me in patch 3 if the whole patchset is related to crash and
panic.
Thanks
Baoquan
> ---
> kernel/crash_core.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index a4ef79591eb2..bb38bbaf3a26 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2002-2004 Eric Biederman <ebiederm@xmission.com>
> */
>
> +#include "linux/panic.h"
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/buildid.h>
> @@ -143,17 +144,7 @@ STACK_FRAME_NON_STANDARD(__crash_kexec);
>
> __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> {
> - int old_cpu, this_cpu;
> -
> - /*
> - * Only one CPU is allowed to execute the crash_kexec() code as with
> - * panic(). Otherwise parallel calls of panic() and crash_kexec()
> - * may stop each other. To exclude them, we use panic_cpu here too.
> - */
> - old_cpu = PANIC_CPU_INVALID;
> - this_cpu = raw_smp_processor_id();
> -
> - if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu)) {
> + if (panic_try_start()) {
> /* This is the 1st CPU which comes here, so go ahead. */
> __crash_kexec(regs);
>
> @@ -161,7 +152,7 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> * Reset panic_cpu to allow another panic()/crash_kexec()
> * call.
> */
> - atomic_set(&panic_cpu, PANIC_CPU_INVALID);
> + panic_reset();
> }
> }
>
> --
> 2.43.0
>
On 8/21/25 10:43, Baoquan He wrote: > On 08/20/25 at 05:14pm, Jinchao Wang wrote: >> crash_kexec() had its own code to exclude >> parallel execution by setting panic_cpu. >> This is already handled by panic_try_start(). >> >> Switch to panic_try_start() to remove the >> duplication and keep the logic consistent. >> >> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com> > > I had to use b4 to grab back the whole patchset, but I can't comment on > other patches, especially the patch 1. > > Firstly, this series looks interesting. It does enhance code > readibility. But I am a vim user, I like open code on this one line of > code wrapping. So leave this to other reviewers to decide if this should > be accepted. > > Secondly, the lines of your patch log are too short, it's not convenient > for reading. Can you set your mail writer to change this. > Thanks for the feedback. I will make sure to set my email client to use a wider line length for future patches. > Thirdly, please add people to CC in all patches. I don't know why you > only CC me in patch 3 if the whole patchset is related to crash and > panic. I use git send-email, which automatically adds the To and Cc headers based on these two configuration lines: tocmd = "scripts/get_maintainer.pl --norolestats --nol" ccCmd = "scripts/get_maintainer.pl --norolestats --nom" It seems this isn't working very well. I'll find another configuration for the toCmd/ccCmd. I would be very grateful if you could share your configuration or commands. -- Best regards, Jinchao
On 08/20/25 at 05:14pm, Jinchao Wang wrote:
> crash_kexec() had its own code to exclude
> parallel execution by setting panic_cpu.
> This is already handled by panic_try_start().
>
> Switch to panic_try_start() to remove the
> duplication and keep the logic consistent.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> kernel/crash_core.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index a4ef79591eb2..bb38bbaf3a26 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2002-2004 Eric Biederman <ebiederm@xmission.com>
> */
>
> +#include "linux/panic.h"
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/buildid.h>
> @@ -143,17 +144,7 @@ STACK_FRAME_NON_STANDARD(__crash_kexec);
>
> __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> {
> - int old_cpu, this_cpu;
> -
> - /*
> - * Only one CPU is allowed to execute the crash_kexec() code as with
> - * panic(). Otherwise parallel calls of panic() and crash_kexec()
> - * may stop each other. To exclude them, we use panic_cpu here too.
> - */
> - old_cpu = PANIC_CPU_INVALID;
> - this_cpu = raw_smp_processor_id();
> -
> - if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu)) {
> + if (panic_try_start()) {
Seriously, where can I find this panic_try_start() and the
panic_reset()?
> /* This is the 1st CPU which comes here, so go ahead. */
> __crash_kexec(regs);
>
> @@ -161,7 +152,7 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> * Reset panic_cpu to allow another panic()/crash_kexec()
> * call.
> */
> - atomic_set(&panic_cpu, PANIC_CPU_INVALID);
> + panic_reset();
> }
> }
>
> --
> 2.43.0
>
On 08/20/25 at 09:35pm, Baoquan He wrote:
> On 08/20/25 at 05:14pm, Jinchao Wang wrote:
> > crash_kexec() had its own code to exclude
> > parallel execution by setting panic_cpu.
> > This is already handled by panic_try_start().
> >
> > Switch to panic_try_start() to remove the
> > duplication and keep the logic consistent.
> >
> > Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> > ---
> > kernel/crash_core.c | 15 +++------------
> > 1 file changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> > index a4ef79591eb2..bb38bbaf3a26 100644
> > --- a/kernel/crash_core.c
> > +++ b/kernel/crash_core.c
> > @@ -4,6 +4,7 @@
> > * Copyright (C) 2002-2004 Eric Biederman <ebiederm@xmission.com>
> > */
> >
> > +#include "linux/panic.h"
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > #include <linux/buildid.h>
> > @@ -143,17 +144,7 @@ STACK_FRAME_NON_STANDARD(__crash_kexec);
> >
> > __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> > {
> > - int old_cpu, this_cpu;
> > -
> > - /*
> > - * Only one CPU is allowed to execute the crash_kexec() code as with
> > - * panic(). Otherwise parallel calls of panic() and crash_kexec()
> > - * may stop each other. To exclude them, we use panic_cpu here too.
> > - */
> > - old_cpu = PANIC_CPU_INVALID;
> > - this_cpu = raw_smp_processor_id();
> > -
> > - if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu)) {
> > + if (panic_try_start()) {
>
> Seriously, where can I find this panic_try_start() and the
> panic_reset()?
Ok, just found it's in a patch series. Grabbed the patchset and will
have a look.
>
> > /* This is the 1st CPU which comes here, so go ahead. */
> > __crash_kexec(regs);
> >
> > @@ -161,7 +152,7 @@ __bpf_kfunc void crash_kexec(struct pt_regs *regs)
> > * Reset panic_cpu to allow another panic()/crash_kexec()
> > * call.
> > */
> > - atomic_set(&panic_cpu, PANIC_CPU_INVALID);
> > + panic_reset();
> > }
> > }
> >
> > --
> > 2.43.0
> >
>
nmi_panic() duplicated the logic to claim
panic_cpu with atomic_try_cmpxchg. This is
already wrapped in panic_try_start().
Replace the open-coded logic with
panic_try_start(), and use
panic_on_other_cpu() for the fallback path.
This removes duplication and keeps panic
handling code consistent.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/panic.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index eacb0c972110..cd86d37d124c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -355,15 +355,9 @@ EXPORT_SYMBOL(panic_on_other_cpu);
*/
void nmi_panic(struct pt_regs *regs, const char *msg)
{
- int old_cpu, this_cpu;
-
- old_cpu = PANIC_CPU_INVALID;
- this_cpu = raw_smp_processor_id();
-
- /* atomic_try_cmpxchg updates old_cpu on failure */
- if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu))
+ if (panic_try_start())
panic("%s", msg);
- else if (old_cpu != this_cpu)
+ else if (panic_on_other_cpu())
nmi_panic_self_stop(regs);
}
EXPORT_SYMBOL(nmi_panic);
--
2.43.0vpanic() had open-coded logic to claim
panic_cpu with atomic_try_cmpxchg. This is
already handled by panic_try_start().
Switch to panic_try_start() and use
panic_on_other_cpu() for the fallback path.
This removes duplicate code and makes panic
handling consistent across functions.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/panic.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index cd86d37d124c..5266e195f5ac 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -415,7 +415,6 @@ void vpanic(const char *fmt, va_list args)
static char buf[1024];
long i, i_next = 0, len;
int state = 0;
- int old_cpu, this_cpu;
bool _crash_kexec_post_notifiers = crash_kexec_post_notifiers;
if (panic_on_warn) {
@@ -452,13 +451,10 @@ void vpanic(const char *fmt, va_list args)
* `old_cpu == this_cpu' means we came from nmi_panic() which sets
* panic_cpu to this CPU. In this case, this is also the 1st CPU.
*/
- old_cpu = PANIC_CPU_INVALID;
- this_cpu = raw_smp_processor_id();
-
/* atomic_try_cmpxchg updates old_cpu on failure */
- if (atomic_try_cmpxchg(&panic_cpu, &old_cpu, this_cpu)) {
+ if (panic_try_start()) {
/* go ahead */
- } else if (old_cpu != this_cpu)
+ } else if (panic_on_other_cpu())
panic_smp_self_stop();
console_verbose();
--
2.43.0nbcon_context_try_acquire() compared
panic_cpu directly with smp_processor_id().
This open-coded check is now provided by
panic_on_this_cpu().
Switch to panic_on_this_cpu() to simplify
the code and improve readability.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/printk/nbcon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 646801813415..7490865e2f44 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -2,6 +2,7 @@
// Copyright (C) 2022 Linutronix GmbH, John Ogness
// Copyright (C) 2022 Intel, Thomas Gleixner
+#include "linux/panic.h"
#include <linux/atomic.h>
#include <linux/bug.h>
#include <linux/console.h>
@@ -589,7 +590,6 @@ static struct printk_buffers panic_nbcon_pbufs;
*/
static bool nbcon_context_try_acquire(struct nbcon_context *ctxt, bool is_reacquire)
{
- unsigned int cpu = smp_processor_id();
struct console *con = ctxt->console;
struct nbcon_state cur;
int err;
@@ -614,7 +614,7 @@ static bool nbcon_context_try_acquire(struct nbcon_context *ctxt, bool is_reacqu
/* Acquire succeeded. */
/* Assign the appropriate buffer for this context. */
- if (atomic_read(&panic_cpu) == cpu)
+ if (panic_on_this_cpu())
ctxt->pbufs = &panic_nbcon_pbufs;
else
ctxt->pbufs = con->pbufs;
--
2.43.0On 2025-08-20, Jinchao Wang <wangjinchao600@gmail.com> wrote:
> nbcon_context_try_acquire() compared
> panic_cpu directly with smp_processor_id().
> This open-coded check is now provided by
> panic_on_this_cpu().
>
> Switch to panic_on_this_cpu() to simplify
> the code and improve readability.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> kernel/printk/nbcon.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 646801813415..7490865e2f44 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -2,6 +2,7 @@
> // Copyright (C) 2022 Linutronix GmbH, John Ogness
> // Copyright (C) 2022 Intel, Thomas Gleixner
>
> +#include "linux/panic.h"
> #include <linux/atomic.h>
> #include <linux/bug.h>
> #include <linux/console.h>
Please use angle brackets. Also, the includes are sorted
alphabetically. So it should look like this:
#include <linux/irqflags.h>
#include <linux/kthread.h>
#include <linux/minmax.h>
+#include <linux/panic.h>
#include <linux/percpu.h>
#include <linux/preempt.h>
#include <linux/slab.h>
> @@ -589,7 +590,6 @@ static struct printk_buffers panic_nbcon_pbufs;
> */
> static bool nbcon_context_try_acquire(struct nbcon_context *ctxt, bool is_reacquire)
> {
> - unsigned int cpu = smp_processor_id();
> struct console *con = ctxt->console;
> struct nbcon_state cur;
> int err;
> @@ -614,7 +614,7 @@ static bool nbcon_context_try_acquire(struct nbcon_context *ctxt, bool is_reacqu
> /* Acquire succeeded. */
>
> /* Assign the appropriate buffer for this context. */
> - if (atomic_read(&panic_cpu) == cpu)
> + if (panic_on_this_cpu())
> ctxt->pbufs = &panic_nbcon_pbufs;
> else
> ctxt->pbufs = con->pbufs;
With the above changes:
Reviewed-by: John Ogness <john.ogness@linutronix.de>
On 8/25/25 17:53, John Ogness wrote: > On 2025-08-20, Jinchao Wang <wangjinchao600@gmail.com> wrote: >> nbcon_context_try_acquire() compared >> panic_cpu directly with smp_processor_id(). >> This open-coded check is now provided by >> panic_on_this_cpu(). >> >> Switch to panic_on_this_cpu() to simplify >> the code and improve readability. >> >> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com> >> --- >> kernel/printk/nbcon.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c >> index 646801813415..7490865e2f44 100644 >> --- a/kernel/printk/nbcon.c >> +++ b/kernel/printk/nbcon.c >> @@ -2,6 +2,7 @@ >> // Copyright (C) 2022 Linutronix GmbH, John Ogness >> // Copyright (C) 2022 Intel, Thomas Gleixner >> >> +#include "linux/panic.h" >> #include <linux/atomic.h> >> #include <linux/bug.h> >> #include <linux/console.h> > > Please use angle brackets. Also, the includes are sorted > alphabetically. So it should look like this: > > #include <linux/irqflags.h> > #include <linux/kthread.h> > #include <linux/minmax.h> > +#include <linux/panic.h> > #include <linux/percpu.h> > #include <linux/preempt.h> > #include <linux/slab.h> > Thanks for the feedback. This patch, v2, has been merged to the -mm branch already, but I will fix the include style in a separate cleanup patch. -- Best regards, Jinchao
The helper this_cpu_in_panic() duplicated
logic already provided by panic_on_this_cpu().
Remove this_cpu_in_panic() and switch all
users to panic_on_this_cpu().
This simplifies the code and avoids having
two helpers for the same check.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
include/linux/printk.h | 2 --
kernel/panic.c | 2 +-
kernel/printk/nbcon.c | 2 +-
kernel/printk/printk.c | 15 ++-------------
kernel/printk/printk_ringbuffer.c | 2 +-
lib/dump_stack.c | 2 +-
6 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 5d22b803f51e..45c663124c9b 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -330,8 +330,6 @@ static inline bool pr_flush(int timeout_ms, bool reset_on_progress)
#endif
-bool this_cpu_in_panic(void);
-
#ifdef CONFIG_SMP
extern int __printk_cpu_sync_try_get(void);
extern void __printk_cpu_sync_wait(void);
diff --git a/kernel/panic.c b/kernel/panic.c
index 5266e195f5ac..8415e4073da1 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -343,7 +343,7 @@ EXPORT_SYMBOL(panic_on_this_cpu);
*/
bool panic_on_other_cpu(void)
{
- return (panic_in_progress() && !this_cpu_in_panic());
+ return (panic_in_progress() && !panic_on_this_cpu());
}
EXPORT_SYMBOL(panic_on_other_cpu);
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 7490865e2f44..c6d1a4a747e9 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1394,7 +1394,7 @@ enum nbcon_prio nbcon_get_default_prio(void)
{
unsigned int *cpu_emergency_nesting;
- if (this_cpu_in_panic())
+ if (panic_on_this_cpu())
return NBCON_PRIO_PANIC;
cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5fe35f377b79..faa8b1f0585b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -17,6 +17,7 @@
* 01Mar01 Andrew Morton
*/
+#include "linux/panic.h"
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/kernel.h>
@@ -345,18 +346,6 @@ static void __up_console_sem(unsigned long ip)
}
#define up_console_sem() __up_console_sem(_RET_IP_)
-/* Return true if a panic is in progress on the current CPU. */
-bool this_cpu_in_panic(void)
-{
- /*
- * We can use raw_smp_processor_id() here because it is impossible for
- * the task to be migrated to the panic_cpu, or away from it. If
- * panic_cpu has already been set, and we're not currently executing on
- * that CPU, then we never will be.
- */
- return unlikely(atomic_read(&panic_cpu) == raw_smp_processor_id());
-}
-
/*
* Return true if a panic is in progress on a remote CPU.
*
@@ -365,7 +354,7 @@ bool this_cpu_in_panic(void)
*/
bool other_cpu_in_panic(void)
{
- return (panic_in_progress() && !this_cpu_in_panic());
+ return (panic_in_progress() && !panic_on_this_cpu());
}
/*
diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
index d9fb053cff67..e2a1b2d34d2b 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -2143,7 +2143,7 @@ static bool _prb_read_valid(struct printk_ringbuffer *rb, u64 *seq,
* But it would have the sequence number returned
* by "prb_next_reserve_seq() - 1".
*/
- if (this_cpu_in_panic() &&
+ if (panic_on_this_cpu() &&
(!debug_non_panic_cpus || legacy_allow_panic_sync) &&
((*seq + 1) < prb_next_reserve_seq(rb))) {
(*seq)++;
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index b3a85fe8b673..f0c78b5b5324 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -102,7 +102,7 @@ static void __dump_stack(const char *log_lvl)
*/
asmlinkage __visible void dump_stack_lvl(const char *log_lvl)
{
- bool in_panic = this_cpu_in_panic();
+ bool in_panic = panic_on_this_cpu();
unsigned long flags;
/*
--
2.43.0The helper other_cpu_in_panic() duplicated
logic already provided by panic_on_other_cpu().
Remove other_cpu_in_panic() and update all
users to call panic_on_other_cpu() instead.
This removes redundant code and makes panic
handling consistent.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/printk/internal.h | 1 -
kernel/printk/nbcon.c | 8 ++++----
kernel/printk/printk.c | 19 ++++---------------
3 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index ef282001f200..f72bbfa266d6 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -332,7 +332,6 @@ struct printk_message {
unsigned long dropped;
};
-bool other_cpu_in_panic(void);
bool printk_get_next_message(struct printk_message *pmsg, u64 seq,
bool is_extended, bool may_supress);
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index c6d1a4a747e9..171480135830 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -255,7 +255,7 @@ static int nbcon_context_try_acquire_direct(struct nbcon_context *ctxt,
* opportunity to perform any necessary cleanup if they were
* interrupted by the panic CPU while printing.
*/
- if (other_cpu_in_panic() &&
+ if (panic_on_other_cpu() &&
(!is_reacquire || cur->unsafe_takeover)) {
return -EPERM;
}
@@ -310,7 +310,7 @@ static bool nbcon_waiter_matches(struct nbcon_state *cur, int expected_prio)
* Event #2 implies the new context is PANIC.
* Event #3 occurs when panic() has flushed the console.
* Event #4 occurs when a non-panic CPU reacquires.
- * Event #5 is not possible due to the other_cpu_in_panic() check
+ * Event #5 is not possible due to the panic_on_other_cpu() check
* in nbcon_context_try_acquire_handover().
*/
@@ -349,7 +349,7 @@ static int nbcon_context_try_acquire_requested(struct nbcon_context *ctxt,
struct nbcon_state new;
/* Note that the caller must still remove the request! */
- if (other_cpu_in_panic())
+ if (panic_on_other_cpu())
return -EPERM;
/*
@@ -447,7 +447,7 @@ static int nbcon_context_try_acquire_handover(struct nbcon_context *ctxt,
* nbcon_waiter_matches(). In particular, the assumption that
* lower priorities are ignored during panic.
*/
- if (other_cpu_in_panic())
+ if (panic_on_other_cpu())
return -EPERM;
/* Handover is not possible on the same CPU. */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index faa8b1f0585b..236f03937107 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -346,17 +346,6 @@ static void __up_console_sem(unsigned long ip)
}
#define up_console_sem() __up_console_sem(_RET_IP_)
-/*
- * Return true if a panic is in progress on a remote CPU.
- *
- * On true, the local CPU should immediately release any printing resources
- * that may be needed by the panic CPU.
- */
-bool other_cpu_in_panic(void)
-{
- return (panic_in_progress() && !panic_on_this_cpu());
-}
-
/*
* This is used for debugging the mess that is the VT code by
* keeping track if we have the console semaphore held. It's
@@ -2391,7 +2380,7 @@ asmlinkage int vprintk_emit(int facility, int level,
* non-panic CPUs are generating any messages, they will be
* silently dropped.
*/
- if (other_cpu_in_panic() &&
+ if (panic_on_other_cpu() &&
!debug_non_panic_cpus &&
!panic_triggering_all_cpu_backtrace)
return 0;
@@ -2827,7 +2816,7 @@ void console_lock(void)
might_sleep();
/* On panic, the console_lock must be left to the panic cpu. */
- while (other_cpu_in_panic())
+ while (panic_on_other_cpu())
msleep(1000);
down_console_sem();
@@ -2847,7 +2836,7 @@ EXPORT_SYMBOL(console_lock);
int console_trylock(void)
{
/* On panic, the console_lock must be left to the panic cpu. */
- if (other_cpu_in_panic())
+ if (panic_on_other_cpu())
return 0;
if (down_trylock_console_sem())
return 0;
@@ -3227,7 +3216,7 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove
any_progress = true;
/* Allow panic_cpu to take over the consoles safely. */
- if (other_cpu_in_panic())
+ if (panic_on_other_cpu())
goto abandon;
if (do_cond_resched)
--
2.43.0On 2025-08-20, Jinchao Wang <wangjinchao600@gmail.com> wrote: > The helper other_cpu_in_panic() duplicated > logic already provided by panic_on_other_cpu(). > > Remove other_cpu_in_panic() and update all > users to call panic_on_other_cpu() instead. > > This removes redundant code and makes panic > handling consistent. > > Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com> Reviewed-by: John Ogness <john.ogness@linutronix.de>
Both watchdog_buddy_check_hardlockup() and
watchdog_overflow_callback() may trigger
during a panic. This can lead to recursive
panic handling.
Add panic_in_progress() checks so watchdog
activity is skipped once a panic has begun.
This prevents recursive panic and keeps the
panic path more reliable.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/watchdog_buddy.c | 5 +++++
kernel/watchdog_perf.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/kernel/watchdog_buddy.c b/kernel/watchdog_buddy.c
index ee754d767c21..79a85623028c 100644
--- a/kernel/watchdog_buddy.c
+++ b/kernel/watchdog_buddy.c
@@ -93,6 +93,11 @@ void watchdog_buddy_check_hardlockup(int hrtimer_interrupts)
*/
if (hrtimer_interrupts % 3 != 0)
return;
+ /*
+ * pass the buddy check if a panic is in process
+ */
+ if (panic_in_progress())
+ return;
/* check for a hardlockup on the next CPU */
next_cpu = watchdog_next_cpu(smp_processor_id());
diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c
index 9c58f5b4381d..7641de750ca5 100644
--- a/kernel/watchdog_perf.c
+++ b/kernel/watchdog_perf.c
@@ -12,6 +12,7 @@
#define pr_fmt(fmt) "NMI watchdog: " fmt
+#include <linux/panic.h>
#include <linux/nmi.h>
#include <linux/atomic.h>
#include <linux/module.h>
@@ -110,6 +111,8 @@ static void watchdog_overflow_callback(struct perf_event *event,
if (!watchdog_check_timestamp())
return;
+ if (panic_in_progress())
+ return;
watchdog_hardlockup_check(smp_processor_id(), regs);
}
--
2.43.0On Wed, Aug 20, 2025 at 05:14:54PM +0800, Jinchao Wang wrote: > Both watchdog_buddy_check_hardlockup() and > watchdog_overflow_callback() may trigger > during a panic. This can lead to recursive > panic handling. > > Add panic_in_progress() checks so watchdog > activity is skipped once a panic has begun. > > This prevents recursive panic and keeps the > panic path more reliable. > > Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com> > --- > kernel/watchdog_buddy.c | 5 +++++ > kernel/watchdog_perf.c | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/kernel/watchdog_buddy.c b/kernel/watchdog_buddy.c > index ee754d767c21..79a85623028c 100644 > --- a/kernel/watchdog_buddy.c > +++ b/kernel/watchdog_buddy.c > @@ -93,6 +93,11 @@ void watchdog_buddy_check_hardlockup(int hrtimer_interrupts) > */ > if (hrtimer_interrupts % 3 != 0) > return; > + /* > + * pass the buddy check if a panic is in process > + */ > + if (panic_in_progress()) > + return; > > /* check for a hardlockup on the next CPU */ > next_cpu = watchdog_next_cpu(smp_processor_id()); > diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c > index 9c58f5b4381d..7641de750ca5 100644 > --- a/kernel/watchdog_perf.c > +++ b/kernel/watchdog_perf.c > @@ -12,6 +12,7 @@ > > #define pr_fmt(fmt) "NMI watchdog: " fmt > > +#include <linux/panic.h> > #include <linux/nmi.h> > #include <linux/atomic.h> > #include <linux/module.h> > @@ -110,6 +111,8 @@ static void watchdog_overflow_callback(struct perf_event *event, > > if (!watchdog_check_timestamp()) > return; > + if (panic_in_progress()) > + return; It looks like watchdog_check_timestamp() does some real work, like updates last_timestamp and so on. Under the panic condition all this may be unreliable, right? Maybe it's worth to make panic_in_progress() the first check in the chain? With that, Reviewed-by: Yury Norov (NVIDIA) <yury.norov@gmail.com> > > watchdog_hardlockup_check(smp_processor_id(), regs); > } > -- > 2.43.0
On 8/20/25 23:18, Yury Norov wrote: > On Wed, Aug 20, 2025 at 05:14:54PM +0800, Jinchao Wang wrote: >> Both watchdog_buddy_check_hardlockup() and >> watchdog_overflow_callback() may trigger >> during a panic. This can lead to recursive >> panic handling. >> >> Add panic_in_progress() checks so watchdog >> activity is skipped once a panic has begun. >> >> This prevents recursive panic and keeps the >> panic path more reliable. >> >> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com> >> --- >> kernel/watchdog_buddy.c | 5 +++++ >> kernel/watchdog_perf.c | 3 +++ >> 2 files changed, 8 insertions(+) >> >> diff --git a/kernel/watchdog_buddy.c b/kernel/watchdog_buddy.c >> index ee754d767c21..79a85623028c 100644 >> --- a/kernel/watchdog_buddy.c >> +++ b/kernel/watchdog_buddy.c >> @@ -93,6 +93,11 @@ void watchdog_buddy_check_hardlockup(int hrtimer_interrupts) >> */ >> if (hrtimer_interrupts % 3 != 0) >> return; >> + /* >> + * pass the buddy check if a panic is in process >> + */ >> + if (panic_in_progress()) >> + return; >> >> /* check for a hardlockup on the next CPU */ >> next_cpu = watchdog_next_cpu(smp_processor_id()); >> diff --git a/kernel/watchdog_perf.c b/kernel/watchdog_perf.c >> index 9c58f5b4381d..7641de750ca5 100644 >> --- a/kernel/watchdog_perf.c >> +++ b/kernel/watchdog_perf.c >> @@ -12,6 +12,7 @@ >> >> #define pr_fmt(fmt) "NMI watchdog: " fmt >> >> +#include <linux/panic.h> >> #include <linux/nmi.h> >> #include <linux/atomic.h> >> #include <linux/module.h> >> @@ -110,6 +111,8 @@ static void watchdog_overflow_callback(struct perf_event *event, >> >> if (!watchdog_check_timestamp()) >> return; >> + if (panic_in_progress()) >> + return; > > It looks like watchdog_check_timestamp() does some real work, like > updates last_timestamp and so on. Under the panic condition all this > may be unreliable, right? > > Maybe it's worth to make panic_in_progress() the first check in the > chain? > That's a good point. Thank you. > With that, > > Reviewed-by: Yury Norov (NVIDIA) <yury.norov@gmail.com> > >> >> watchdog_hardlockup_check(smp_processor_id(), regs); >> } >> -- >> 2.43.0 -- Best regards, Jinchao
© 2016 - 2025 Red Hat, Inc.