[PATCH v2 0/3] Introduce arch_do_panic

[PATCH v2 0/3] Introduce arch_do_panic

[Mete Durlu]

Changes in v2 - Address Sashiko findings;
- Patch 2: Remove unused leftover code
- Patch 2: Mention panic_timeout and shutdown_actions relationship for
  s390 in commit message
- Patch 3: Use bug.h instead of setup.h to pass around arch_do_panic
  implementation of sparc

Replace architecture-specific ifdef sections in vpanic() with a clean
arch_do_panic() hook. Currently s390 and sparc embed their panic
handlers directly in vpanic() using preprocessor conditionals, making
the common code path harder to maintain.

Introduce arch_do_panic() as an architecture extension point called at
the end of vpanic(). Architectures can use this hook to implement their
specific panic handling without polluting the generic panic code.

Move s390 panic handling from the panic_notifier chain to
arch_do_panic(). This corrects the execution order so that the
panic_timeout is properly evaluated before architecture-specific
actions. The previous notifier-based approach executed too early in the
panic sequence. 

Move sparc panic handling from ifdef blocks to arch_do_panic(). Remove
the preprocessor conditionals from vpanic() and place the Stop-A
enablement code in architecture-specific files where it belongs.

The cleanup reduces vpanic() complexity and establishes a pattern for other
architectures needing custom panic behavior.

Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
Mete Durlu (3):
      panic: Introduce arch_do_panic
      s390: Implement arch_do_panic
      sparc: Implement arch_do_panic

 arch/s390/include/asm/setup.h  |  3 +++
 arch/s390/kernel/ipl.c         | 15 +--------------
 arch/sparc/include/asm/bug.h   |  3 +++
 arch/sparc/include/asm/setup.h |  1 -
 arch/sparc/kernel/setup.c      |  8 ++++++++
 kernel/panic.c                 | 18 ++++++------------
 6 files changed, 21 insertions(+), 27 deletions(-)
---
base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
change-id: 20260724-arch_do_panic-a97f699aa332

Best regards,
-- 
Mete Durlu <meted@linux.ibm.com>

[PATCH v2 1/3] panic: Introduce arch_do_panic

[Mete Durlu]

Introduce a hook for architectures to put their specific panic handlers.
s390 and sparc already has ifdef preprocessor checks to execute
architecture specific code. Pave the way for vpanic() cleanup.

Suggested-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
 kernel/panic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/panic.c b/kernel/panic.c
index 213725b612aa..1eb0cdc159d9 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -567,6 +567,11 @@ static void panic_other_cpus_shutdown(bool crash_kexec)
 		crash_smp_send_stop();
 }
 
+#ifndef arch_do_panic
+#define arch_do_panic arch_do_panic
+static inline void arch_do_panic(void) {}
+#endif
+
 /**
  * vpanic - halt the system
  * @fmt: The text string to print
@@ -756,6 +761,7 @@ void vpanic(const char *fmt, va_list args)
 #endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
 
+	arch_do_panic();
 	/* Do not scroll important messages printed above */
 	suppress_printk = 1;
 

-- 
2.55.0

Re: [PATCH v2 1/3] panic: Introduce arch_do_panic

[sashiko-bot]

> Introduce a hook for architectures to put their specific panic handlers.
> s390 and sparc already has ifdef preprocessor checks to execute
> architecture specific code. Pave the way for vpanic() cleanup.
> 
> Suggested-by: Sven Schnelle <svens@linux.ibm.com>
> Signed-off-by: Mete Durlu <meted@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-arch_do_panic-v2-0-4e25ceb05075@linux.ibm.com?part=1

Re: [PATCH v2 1/3] panic: Introduce arch_do_panic

[Bradley Morgan]

Hi Mete,

> +#ifndef arch_do_panic
> +#define arch_do_panic arch_do_panic
> +static inline void arch_do_panic(void) {}
> +#endif

this is fragile. the arch override only kicks in if whatever header
defines the macro happens to be in panic.c's include chain. if it ever
falls out, the empty stub wins silently, the arch version still
compiles as a global nobody calls, and the hook is just dead. no build
break, no warning, nothing.

use a weak function instead. arch defines the strong symbol and thats
it, no macro dance, no header placement contract, and the silent stub
failure cant happen. nobody cares about an indirect call in the panic
path. something like:

diff --git a/include/linux/panic.h b/include/linux/panic.h
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -13,6 +13,7 @@ __printf(1, 2)
 void panic(const char *fmt, ...) __noreturn __cold;
 __printf(1, 0)
 void vpanic(const char *fmt, va_list args) __noreturn __cold;
+void arch_do_panic(void);
 void nmi_panic(struct pt_regs *regs, const char *msg);
 void check_panic_on_warn(const char *origin);
 extern void oops_enter(void);
diff --git a/kernel/panic.c b/kernel/panic.c
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -567,6 +567,11 @@ static void panic_other_cpus_shutdown(bool crash_kexec)
 		crash_smp_send_stop();
 }
 
+/* Overridden by architectures with a panic handler of last resort. */
+void __weak arch_do_panic(void)
+{
+}
+
 /**
  * vpanic - halt the system
  * @fmt: The text string to print
@@ -756,6 +761,7 @@ void vpanic(const char *fmt, va_list args)
 #endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
 
+	arch_do_panic();
 	/* Do not scroll important messages printed above */
 	suppress_printk = 1;
 
untested, and no SoB from me, its a suggestion. if you fold it into v3
its your patch under your own SoB as usual.


(Heh, hope my client don't mutilate this)

the prototype in panic.h also means the arch definitions dont
trip -Wmissing-prototypes, which your version dodges with the macro but
only as long as the header keeps getting pulled in.

> 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> 
> +	arch_do_panic();

one more thing on the call site: arch code now runs (and prints, on
sparc) after the "end Kernel panic" marker, which until now was the
last line anyone would ever see. tools grep for that as a terminal
marker. probably fine, but say so in the changelog instead of leaving
it implicit.

s/already has/already have/ in the changelog while your at it.

Thanks!

[PATCH v2 2/3] s390: Implement arch_do_panic

[Mete Durlu]

s390 has a custom panic handler which carries out user specified actions
during a panic scenario. This handler is invoked via the panic_notifier
call chain and executed before panic_timeout value is evaluated in
common code.

Use arch_do_panic() hook to invoke arch specific panic handling instead
of using panic_notifier call chain. The execution order of panic
handlers now allow for user specified panic_timeout value to be taken
into account.

This now means that the panic_timeout value takes precedence over user
defined on_panic behavior.

Fixes: ff6b8ea68f4b ("[S390] ipl/dump on panic.")
Suggested-by: Sven Schnelle <svens@linux.ibm.com>
Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
 arch/s390/include/asm/setup.h |  3 +++
 arch/s390/kernel/ipl.c        | 15 +--------------
 kernel/panic.c                |  3 ---
 3 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/arch/s390/include/asm/setup.h b/arch/s390/include/asm/setup.h
index cbf60ade741d..6b38017f7cf3 100644
--- a/arch/s390/include/asm/setup.h
+++ b/arch/s390/include/asm/setup.h
@@ -90,6 +90,9 @@ extern void (*_machine_restart)(char *command);
 extern void (*_machine_halt)(void);
 extern void (*_machine_power_off)(void);
 
+void arch_do_panic(void);
+#define arch_do_panic arch_do_panic
+
 struct oldmem_data {
 	unsigned long start;
 	unsigned long size;
diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
index 3c346b02ceb9..8ab02e821256 100644
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -2115,7 +2115,7 @@ static ssize_t on_panic_store(struct kobject *kobj,
 }
 static struct kobj_attribute on_panic_attr = __ATTR_RW(on_panic);
 
-static void do_panic(void)
+void arch_do_panic(void)
 {
 	lgr_info_log();
 	on_panic_trigger.action->fn(&on_panic_trigger);
@@ -2331,18 +2331,6 @@ static int __init vmcmd_on_poff_setup(char *str)
 }
 __setup("vmpoff=", vmcmd_on_poff_setup);
 
-static int on_panic_notify(struct notifier_block *self,
-			   unsigned long event, void *data)
-{
-	do_panic();
-	return NOTIFY_OK;
-}
-
-static struct notifier_block on_panic_nb = {
-	.notifier_call = on_panic_notify,
-	.priority = INT_MIN,
-};
-
 void __init setup_ipl(void)
 {
 	BUILD_BUG_ON(sizeof(struct ipl_parameter_block) != PAGE_SIZE);
@@ -2375,7 +2363,6 @@ void __init setup_ipl(void)
 		/* We have no info to copy */
 		break;
 	}
-	atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
 }
 
 void __no_stack_protector s390_reset_system(void)
diff --git a/kernel/panic.c b/kernel/panic.c
index 1eb0cdc159d9..de0bda946cab 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -755,9 +755,6 @@ void vpanic(const char *fmt, va_list args)
 		pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
 			 "twice on console to return to the boot prom\n");
 	}
-#endif
-#if defined(CONFIG_S390)
-	disabled_wait();
 #endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
 

-- 
2.55.0

Re: [PATCH v2 2/3] s390: Implement arch_do_panic

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving the s390 on_panic trigger execution to arch_do_panic() causes it to be completely skipped if a non-zero panic_timeout is configured, silently breaking hardware crash dumps.
--

--- Patch [2]: [PATCH v2 2/3] s390: Implement arch_do_panic ---
commit c54479b24863a9224f8701eeac5365ad207e8313
Author: Mete Durlu <meted@linux.ibm.com>

s390: Implement arch_do_panic

This commit moves the s390-specific panic handling out of the panic_notifier
call chain and into the arch_do_panic hook. The stated intention is to
allow the generic panic_timeout to take precedence over the architecture's
user-defined panic behavior.

> diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
> --- a/arch/s390/kernel/ipl.c
> +++ b/arch/s390/kernel/ipl.c

[ ... ]

> @@ -2115,7 +2115,7 @@ static ssize_t on_panic_store(struct kobject *kobj,
>  }
>  static struct kobj_attribute on_panic_attr = __ATTR_RW(on_panic);
>  
> -static void do_panic(void)
> +void arch_do_panic(void)
>  {
>  	lgr_info_log();
>  	on_panic_trigger.action->fn(&on_panic_trigger);
>  	stop_run(&on_panic_trigger);
>  }

[Severity: High]
If a non-zero panic_timeout is configured, vpanic calls emergency_restart
which never returns.

Since arch_do_panic is invoked at the very end of vpanic after the timeout
logic:

kernel/panic.c:vpanic() {
        ...
        if (panic_timeout != 0) {
                ...
                emergency_restart();
        }
        ...
        arch_do_panic();
        ...
}

Does this mean the s390 hardware dump logic here will be completely skipped
instead of taking lower precedence? Could this regression silently break
crash dumps configured via on_panic=dump when a generic timeout parameter
is set?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-arch_do_panic-v2-0-4e25ceb05075@linux.ibm.com?part=2

Re: [PATCH v2 2/3] s390: Implement arch_do_panic

[Bradley Morgan]

Mete,

> -#endif
> -#if defined(CONFIG_S390)
> -	disabled_wait();
> -#endif

removal is fine, this was unreachable anyway. the notifier ran
do_panic() which ends in stop_run(), so nothing ever came back to hit
this disabled_wait().

> +void arch_do_panic(void);
> +#define arch_do_panic arch_do_panic

with the weak function i suggested on patch 1, this whole asm/setup.h
hunk disappears and the patch shrinks to exactly the parts that mean
something:

diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -2115,7 +2115,7 @@ static ssize_t on_panic_store(struct kobject *kobj,
 }
 static struct kobj_attribute on_panic_attr = __ATTR_RW(on_panic);
 
-static void do_panic(void)
+void arch_do_panic(void)
 {
 	lgr_info_log();
 	on_panic_trigger.action->fn(&on_panic_trigger);
@@ -2331,18 +2331,6 @@ static int __init vmcmd_on_poff_setup(char *str)
 }
 __setup("vmpoff=", vmcmd_on_poff_setup);
 
-static int on_panic_notify(struct notifier_block *self,
-			   unsigned long event, void *data)
-{
-	do_panic();
-	return NOTIFY_OK;
-}
-
-static struct notifier_block on_panic_nb = {
-	.notifier_call = on_panic_notify,
-	.priority = INT_MIN,
-};
-
 void __init setup_ipl(void)
 {
 	BUILD_BUG_ON(sizeof(struct ipl_parameter_block) != PAGE_SIZE);
@@ -2375,7 +2363,6 @@ void __init setup_ipl(void)
 		/* We have no info to copy */
 		break;
 	}
-	atomic_notifier_chain_register(&panic_notifier_list, &on_panic_nb);
 }
 
 void __no_stack_protector s390_reset_system(void)
diff --git a/kernel/panic.c b/kernel/panic.c
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -755,9 +755,6 @@ void vpanic(const char *fmt, va_list args)
 		pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
 			 "twice on console to return to the boot prom\n");
 	}
-#endif
-#if defined(CONFIG_S390)
-	disabled_wait();
 #endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
 
same deal, suggestion only, no SoB from me, fold it in under yours.
check whether linux/panic_notifier.h is still needed in ipl.c after
the notifier goes away.

> This now means that the panic_timeout value takes precedence over user
> defined on_panic behavior.

this is the part i want to poke at. distros routinely ship panic=N on
the cmdline. with this change, anyone who configured on_panic=dump on
such a system silently stops getting dumps, because
emergency_restart() runs first and never returns. a reboot instead of
the dump you explicitly asked for is strictly worse. if you and Sven
want that semantic, fine, but spell it out properly and give the s390
docs for on_panic a line about the new precedence.

also worth saying in the changelog what this actually wins: today the
INT_MIN notifier never returns, so sys_info, kmsg_dump, the post
notifier crash_kexec path, the console flushes and the end banner
never execute on s390 once on_panic fires. after this patch they all
do. thats the headline and the changelog undersells it.

> Fixes: ff6b8ea68f4b ("[S390] ipl/dump on panic.")

What's broken? What's buggy? I'm sure this is just a behaviour 
change, could you either

1: Drop the fixes tag
2: Put in the changelog What's broken and why?


s/now allow/now allows/ in the changelog..
Thanks!

[PATCH v2 3/3] sparc: Implement arch_do_panic

[Mete Durlu]

Implement sparc specific arch_do_panic() instead of using sparc specific
ifdef sections in vpanic() code.

Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
 arch/sparc/include/asm/bug.h   | 3 +++
 arch/sparc/include/asm/setup.h | 1 -
 arch/sparc/kernel/setup.c      | 8 ++++++++
 kernel/panic.c                 | 9 ---------
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/sparc/include/asm/bug.h b/arch/sparc/include/asm/bug.h
index ea53e418f6c0..70dc3358430c 100644
--- a/arch/sparc/include/asm/bug.h
+++ b/arch/sparc/include/asm/bug.h
@@ -27,4 +27,7 @@ void do_BUG(const char *file, int line);
 struct pt_regs;
 void __noreturn die_if_kernel(char *str, struct pt_regs *regs);
 
+void arch_do_panic(void);
+#define arch_do_panic arch_do_panic
+
 #endif
diff --git a/arch/sparc/include/asm/setup.h b/arch/sparc/include/asm/setup.h
index 21bed5514028..fed37c85ed00 100644
--- a/arch/sparc/include/asm/setup.h
+++ b/arch/sparc/include/asm/setup.h
@@ -43,7 +43,6 @@ void __init device_scan(void);
 
 /* unaligned_32.c */
 unsigned long safe_compute_effective_address(struct pt_regs *, unsigned int);
-
 #endif
 
 #ifdef CONFIG_SPARC64
diff --git a/arch/sparc/kernel/setup.c b/arch/sparc/kernel/setup.c
index 4975867d9001..87d3569c520f 100644
--- a/arch/sparc/kernel/setup.c
+++ b/arch/sparc/kernel/setup.c
@@ -36,6 +36,14 @@ static const struct ctl_table sparc_sysctl_table[] = {
 #endif
 };
 
+void arch_do_panic(void)
+{
+	extern int stop_a_enabled;
+	/* Make sure the user can actually press Stop-A (L1-A) */
+	stop_a_enabled = 1;
+	pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
+		 "twice on console to return to the boot prom\n");
+}
 
 static int __init init_sparc_sysctls(void)
 {
diff --git a/kernel/panic.c b/kernel/panic.c
index de0bda946cab..593aa2c24d93 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -747,15 +747,6 @@ void vpanic(const char *fmt, va_list args)
 			reboot_mode = panic_reboot_mode;
 		emergency_restart();
 	}
-#ifdef __sparc__
-	{
-		extern int stop_a_enabled;
-		/* Make sure the user can actually press Stop-A (L1-A) */
-		stop_a_enabled = 1;
-		pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
-			 "twice on console to return to the boot prom\n");
-	}
-#endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
 
 	arch_do_panic();

-- 
2.55.0

Re: [PATCH v2 3/3] sparc: Implement arch_do_panic

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The function `arch_do_panic` is defined without a visible prototype, triggering a `-Wmissing-prototypes` compiler warning.
- [Low] Redundant local `extern` declaration of `stop_a_enabled`.
--

--- Patch [3]: [PATCH v2 3/3] sparc: Implement arch_do_panic ---
commit ab46dd14f9e80619721232e0fc090eea0709afa1
Author: Mete Durlu <meted@linux.ibm.com>

sparc: Implement arch_do_panic

Implement sparc specific arch_do_panic() instead of using sparc specific
ifdef sections in vpanic() code.

> diff --git a/arch/sparc/kernel/setup.c b/arch/sparc/kernel/setup.c
> index 4975867d9001b..87d3569c520f8 100644
> --- a/arch/sparc/kernel/setup.c
> +++ b/arch/sparc/kernel/setup.c

[ ... ]

> @@ -36,6 +36,14 @@ static const struct ctl_table sparc_sysctl_table[] = {
>  #endif
>  };
>  
> +void arch_do_panic(void)

[Severity: Low]
Does this trigger a -Wmissing-prototypes compiler warning? The patch
declares arch_do_panic() in arch/sparc/include/asm/bug.h, but fails to
include that header (or linux/bug.h) here where the function is actually
implemented. 

While it might appear that asm/setup.h indirectly includes linux/bug.h
via linux/interrupt.h -> linux/jump_label.h, the jump_label.h header
only includes linux/bug.h if CONFIG_JUMP_LABEL is disabled. If enabled,
the indirect inclusion path is broken. Could we add a direct include of
linux/bug.h or asm/bug.h to ensure safety across all configurations?

> +{
> +	extern int stop_a_enabled;

[Severity: Low]
Is this local extern declaration still needed? Because
arch/sparc/kernel/setup.c directly includes asm/setup.h at the top of the
file, stop_a_enabled is already globally declared for all configurations.

Could we remove this redundant block-scope declaration that was leftover
from when the code resided in kernel/panic.c?

> +	/* Make sure the user can actually press Stop-A (L1-A) */
> +	stop_a_enabled = 1;
> +	pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
> +		 "twice on console to return to the boot prom\n");
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-arch_do_panic-v2-0-4e25ceb05075@linux.ibm.com?part=3

Re: [PATCH v2 3/3] sparc: Implement arch_do_panic

[Bradley Morgan]

Hi Mete,

the move itself is right. with the weak function from patch 1 the
asm/bug.h hunk goes away, and the asm/setup.h hunk deleting a blank
line is unrelated churn that shouldnt be here either. what remains:

diff --git a/arch/sparc/kernel/setup.c b/arch/sparc/kernel/setup.c
--- a/arch/sparc/kernel/setup.c
+++ b/arch/sparc/kernel/setup.c
@@ -36,6 +36,15 @@ static const struct ctl_table sparc_sysctl_table[] = {
 #endif
 };
 
+void arch_do_panic(void)
+{
+	extern int stop_a_enabled;
+
+	/* Make sure the user can actually press Stop-A (L1-A) */
+	stop_a_enabled = 1;
+	pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
+		 "twice on console to return to the boot prom\n");
+}
 
 static int __init init_sparc_sysctls(void)
 {
diff --git a/kernel/panic.c b/kernel/panic.c
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -747,15 +747,6 @@ void vpanic(const char *fmt, va_list args)
 			reboot_mode = panic_reboot_mode;
 		emergency_restart();
 	}
-#ifdef __sparc__
-	{
-		extern int stop_a_enabled;
-		/* Make sure the user can actually press Stop-A (L1-A) */
-		stop_a_enabled = 1;
-		pr_emerg("Press Stop-A (L1-A) from sun keyboard or send break\n"
-			 "twice on console to return to the boot prom\n");
-	}
-#endif
 	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
 
 	arch_do_panic();

suggestion only, no SoB from me, its yours to fold into v3.

note the Stop A message now prints after the "end Kernel panic"
marker instead of before it. thats the call site from patch 1 doing
that, im ok with it, but it wants a sentence in the changelog.

with the stray hunks gone this looks good to me.
Thanks!