The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Introduce arch_do_panic
@ 2026-07-27 10:36 Mete Durlu
  2026-07-27 10:36 ` [PATCH v2 1/3] panic: " Mete Durlu
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Mete Durlu @ 2026-07-27 10:36 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson
  Cc: linux-kernel, linux-s390, sparclinux, 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>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/3] panic: Introduce arch_do_panic
  2026-07-27 10:36 [PATCH v2 0/3] Introduce arch_do_panic Mete Durlu
@ 2026-07-27 10:36 ` Mete Durlu
  2026-07-27 12:59   ` Bradley Morgan
  2026-07-27 10:36 ` [PATCH v2 2/3] s390: Implement arch_do_panic Mete Durlu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Mete Durlu @ 2026-07-27 10:36 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson
  Cc: linux-kernel, linux-s390, sparclinux, 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/3] s390: Implement arch_do_panic
  2026-07-27 10:36 [PATCH v2 0/3] Introduce arch_do_panic Mete Durlu
  2026-07-27 10:36 ` [PATCH v2 1/3] panic: " Mete Durlu
@ 2026-07-27 10:36 ` Mete Durlu
  2026-07-27 13:01   ` Bradley Morgan
  2026-07-27 10:36 ` [PATCH v2 3/3] sparc: " Mete Durlu
  2026-07-29  8:56 ` [PATCH v2 0/3] Introduce arch_do_panic Heiko Carstens
  3 siblings, 1 reply; 15+ messages in thread
From: Mete Durlu @ 2026-07-27 10:36 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson
  Cc: linux-kernel, linux-s390, sparclinux, 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/3] sparc: Implement arch_do_panic
  2026-07-27 10:36 [PATCH v2 0/3] Introduce arch_do_panic Mete Durlu
  2026-07-27 10:36 ` [PATCH v2 1/3] panic: " Mete Durlu
  2026-07-27 10:36 ` [PATCH v2 2/3] s390: Implement arch_do_panic Mete Durlu
@ 2026-07-27 10:36 ` Mete Durlu
  2026-07-27 13:02   ` Bradley Morgan
  2026-07-29  8:56 ` [PATCH v2 0/3] Introduce arch_do_panic Heiko Carstens
  3 siblings, 1 reply; 15+ messages in thread
From: Mete Durlu @ 2026-07-27 10:36 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson
  Cc: linux-kernel, linux-s390, sparclinux, 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] panic: Introduce arch_do_panic
  2026-07-27 10:36 ` [PATCH v2 1/3] panic: " Mete Durlu
@ 2026-07-27 12:59   ` Bradley Morgan
  2026-07-28 10:13     ` Mete Durlu
  0 siblings, 1 reply; 15+ messages in thread
From: Bradley Morgan @ 2026-07-27 12:59 UTC (permalink / raw)
  To: meted
  Cc: agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux, svens

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!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] s390: Implement arch_do_panic
  2026-07-27 10:36 ` [PATCH v2 2/3] s390: Implement arch_do_panic Mete Durlu
@ 2026-07-27 13:01   ` Bradley Morgan
  2026-07-28 10:43     ` Mete Durlu
  0 siblings, 1 reply; 15+ messages in thread
From: Bradley Morgan @ 2026-07-27 13:01 UTC (permalink / raw)
  To: meted
  Cc: agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux, svens

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!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 3/3] sparc: Implement arch_do_panic
  2026-07-27 10:36 ` [PATCH v2 3/3] sparc: " Mete Durlu
@ 2026-07-27 13:02   ` Bradley Morgan
  0 siblings, 0 replies; 15+ messages in thread
From: Bradley Morgan @ 2026-07-27 13:02 UTC (permalink / raw)
  To: meted
  Cc: agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux, svens

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!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/3] panic: Introduce arch_do_panic
  2026-07-27 12:59   ` Bradley Morgan
@ 2026-07-28 10:13     ` Mete Durlu
  0 siblings, 0 replies; 15+ messages in thread
From: Mete Durlu @ 2026-07-28 10:13 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux, svens

On 27/07/2026 14:59, Bradley Morgan wrote:
> Hi Mete,

Hi Bradley,

>> +#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.

While I acknowledge the benefits you just listed, I got the
notion that maintainers don't prefer __weak implementations
and instead opt for arch specific override via ifndef guards.
Therefore I wanted to start with this approach.

However I am willing to switch if the maintainers would favor
that approach.
> 
> 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.

Right, I'll mention that. And by changelog I take it that you mean
the commit message, right?

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

Thanks, I'll fix the typo in the next version.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] s390: Implement arch_do_panic
  2026-07-27 13:01   ` Bradley Morgan
@ 2026-07-28 10:43     ` Mete Durlu
  2026-07-28 11:02       ` Bradley Morgan
  0 siblings, 1 reply; 15+ messages in thread
From: Mete Durlu @ 2026-07-28 10:43 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux, svens

On 27/07/2026 15:01, Bradley Morgan wrote:

Hi Bradley,

>> 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.

FWICT, there is no mention of shutdown actions in kernel documentation.
s390's user documentation is a different story. I agree it should be
documented on Linux on IBM systems documentation.

I'd argue that specifying both on_panic=dump and panic=N (N>0) is
a configuration error, but maybe a pr_warn() could be added to the
when on_panic=dump is configured with panic=N is set. That could
hint what is wrong if anyone investigating dmesg and trying to
figure out why system didn't dump.

On another note, dumping via shutdown actions serves as a backup
and kdump is the main dump mechanism nowadays. Kdump always takes
precedence over panic=N.

> 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.
> 

Right, I will add that part to the commit message too.

>> Fixes: ff6b8ea68f4b ("[S390] ipl/dump on panic.")
> 
> What's broken? What's buggy? 

I can put a couple of sentences on how and why panic=
kernel command line argument was broken for s390.
> s/now allow/now allows/ in the changelog..

Will fix in next version!

Thank you


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] s390: Implement arch_do_panic
  2026-07-28 10:43     ` Mete Durlu
@ 2026-07-28 11:02       ` Bradley Morgan
  2026-07-28 11:40         ` Sven Schnelle
  0 siblings, 1 reply; 15+ messages in thread
From: Bradley Morgan @ 2026-07-28 11:02 UTC (permalink / raw)
  To: Mete Durlu
  Cc: agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux, svens

On 28 July 2026 11:43:17 BST, Mete Durlu <meted@linux.ibm.com> wrote:
>On 27/07/2026 15:01, Bradley Morgan wrote:
>
>Hi Bradley,
>
>>> 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.
>
>FWICT, there is no mention of shutdown actions in kernel documentation.
>s390's user documentation is a different story. I agree it should be
>documented on Linux on IBM systems documentation.
>
>I'd argue that specifying both on_panic=dump and panic=N (N>0) is
>a configuration error, but maybe a pr_warn() could be added to the
>when on_panic=dump is configured with panic=N is set. That could
>hint what is wrong if anyone investigating dmesg and trying to
>figure out why system didn't dump.
>


hey, sorry I'm out, so I can't do the most massive think ever, and I may
embarrass myself, heh.

I think a BUG would be sufficient, we would want to shut down the system
with said bug, I know about panic on warn, but sometimes that's not always
enabled, something like a BUG would be good, what do you think?

>On another note, dumping via shutdown actions serves as a backup
>and kdump is the main dump mechanism nowadays. Kdump always takes
>precedence over panic=N.
>
>> 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.
>> 
>
>Right, I will add that part to the commit message too.
>
>>> Fixes: ff6b8ea68f4b ("[S390] ipl/dump on panic.")
>> 
>> What's broken? What's buggy? 
>
>I can put a couple of sentences on how and why panic=
>kernel command line argument was broken for s390.
>> s/now allow/now allows/ in the changelog..
>
>Will fix in next version!
>
>Thank you
>
>

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] s390: Implement arch_do_panic
  2026-07-28 11:02       ` Bradley Morgan
@ 2026-07-28 11:40         ` Sven Schnelle
  2026-07-28 11:43           ` Bradley Morgan
  0 siblings, 1 reply; 15+ messages in thread
From: Sven Schnelle @ 2026-07-28 11:40 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: Mete Durlu, agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux

Bradley Morgan <include@grrlz.net> writes:

> On 28 July 2026 11:43:17 BST, Mete Durlu <meted@linux.ibm.com> wrote:
>>On 27/07/2026 15:01, Bradley Morgan wrote:
>>
>>Hi Bradley,
>>
>>>> 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.
>>
>>FWICT, there is no mention of shutdown actions in kernel documentation.
>>s390's user documentation is a different story. I agree it should be
>>documented on Linux on IBM systems documentation.
>>
>>I'd argue that specifying both on_panic=dump and panic=N (N>0) is
>>a configuration error, but maybe a pr_warn() could be added to the
>>when on_panic=dump is configured with panic=N is set. That could
>>hint what is wrong if anyone investigating dmesg and trying to
>>figure out why system didn't dump.
>>
>
>
> hey, sorry I'm out, so I can't do the most massive think ever, and I may
> embarrass myself, heh.
>
> I think a BUG would be sufficient, we would want to shut down the system
> with said bug, I know about panic on warn, but sometimes that's not always
> enabled, something like a BUG would be good, what do you think?

Err... No. You don't want to make a mistake while updating the kernel
panic parameters just to figure out that on the next boot the kernel just BUGs()
because of that. There are of course lots of reasons why changing the kernel
command line could end up in a non-bootable system but this one would
be ridiculous.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/3] s390: Implement arch_do_panic
  2026-07-28 11:40         ` Sven Schnelle
@ 2026-07-28 11:43           ` Bradley Morgan
  0 siblings, 0 replies; 15+ messages in thread
From: Bradley Morgan @ 2026-07-28 11:43 UTC (permalink / raw)
  To: Sven Schnelle
  Cc: Mete Durlu, agordeev, akpm, andreas, borntraeger, davem, gor, hca,
	linux-kernel, linux-s390, pmladek, sparclinux

On 28 July 2026 12:40:18 BST, Sven Schnelle <svens@linux.ibm.com> wrote:
>Bradley Morgan <include@grrlz.net> writes:
>
>> On 28 July 2026 11:43:17 BST, Mete Durlu <meted@linux.ibm.com> wrote:
>>>On 27/07/2026 15:01, Bradley Morgan wrote:
>>>
>>>Hi Bradley,
>>>
>>>>> 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.
>>>
>>>FWICT, there is no mention of shutdown actions in kernel documentation.
>>>s390's user documentation is a different story. I agree it should be
>>>documented on Linux on IBM systems documentation.
>>>
>>>I'd argue that specifying both on_panic=dump and panic=N (N>0) is
>>>a configuration error, but maybe a pr_warn() could be added to the
>>>when on_panic=dump is configured with panic=N is set. That could
>>>hint what is wrong if anyone investigating dmesg and trying to
>>>figure out why system didn't dump.
>>>
>>
>>
>> hey, sorry I'm out, so I can't do the most massive think ever, and I may
>> embarrass myself, heh.
>>
>> I think a BUG would be sufficient, we would want to shut down the system
>> with said bug, I know about panic on warn, but sometimes that's not
>always
>> enabled, something like a BUG would be good, what do you think?
>
>Err... No. You don't want to make a mistake while updating the kernel
>panic parameters just to figure out that on the next boot the kernel just
>BUGs()
>because of that. There are of course lots of reasons why changing the
>kernel
>command line could end up in a non-bootable system but this one would
>be ridiculous.
>

hey, yes, fair enough , thanks, I never said the warn was objectively
wrong, (fair enough), it was more a question.

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] Introduce arch_do_panic
  2026-07-27 10:36 [PATCH v2 0/3] Introduce arch_do_panic Mete Durlu
                   ` (2 preceding siblings ...)
  2026-07-27 10:36 ` [PATCH v2 3/3] sparc: " Mete Durlu
@ 2026-07-29  8:56 ` Heiko Carstens
  2026-07-29 10:48   ` Mete Durlu
  3 siblings, 1 reply; 15+ messages in thread
From: Heiko Carstens @ 2026-07-29  8:56 UTC (permalink / raw)
  To: Mete Durlu
  Cc: Andrew Morton, Petr Mladek, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, linux-kernel, linux-s390, sparclinux

On Mon, Jul 27, 2026 at 12:36:19PM +0200, Mete Durlu wrote:
> 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(-)

Putting the define in a different header file per architecture doesn't
seem to be a good idea. There is no guarantee that this will work. So
either you find a common header file, where it is known that is (and
will be) included in panic.c, or you go with a weak function.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] Introduce arch_do_panic
  2026-07-29  8:56 ` [PATCH v2 0/3] Introduce arch_do_panic Heiko Carstens
@ 2026-07-29 10:48   ` Mete Durlu
  2026-07-29 12:06     ` Heiko Carstens
  0 siblings, 1 reply; 15+ messages in thread
From: Mete Durlu @ 2026-07-29 10:48 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Petr Mladek, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, linux-kernel, linux-s390, sparclinux

On 29/07/2026 10:56, Heiko Carstens wrote:
> On Mon, Jul 27, 2026 at 12:36:19PM +0200, Mete Durlu wrote:
>> 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(-)
> 
> Putting the define in a different header file per architecture doesn't
> seem to be a good idea. There is no guarantee that this will work. So
> either you find a common header file, where it is known that is (and
> will be) included in panic.c, or you go with a weak function.

I went over the headers included to panic.c, either they are really
irrelevant or not present on other archs. To me the best candidate
header seems like "asm/bug.h"

$ find ./arch/ -name "bug.h" | grep include/asm | wc -l
16

$ ls ./arch | wc -l
22 # actually 21 as we shouldn't count Kconfig file

The architectures missing bug.h in their include/asm/ directory are
hexagon, microblaze, nios2, um and xtensa

I believe 16/21 is a good ratio but if this is still not good
enough, I'd like to fallback to __weak implementation.

What do you think Heiko?



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/3] Introduce arch_do_panic
  2026-07-29 10:48   ` Mete Durlu
@ 2026-07-29 12:06     ` Heiko Carstens
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Carstens @ 2026-07-29 12:06 UTC (permalink / raw)
  To: Mete Durlu
  Cc: Andrew Morton, Petr Mladek, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, linux-kernel, linux-s390, sparclinux

On Wed, Jul 29, 2026 at 12:48:49PM +0200, Mete Durlu wrote:
> On 29/07/2026 10:56, Heiko Carstens wrote:
> > On Mon, Jul 27, 2026 at 12:36:19PM +0200, Mete Durlu wrote:
> > Putting the define in a different header file per architecture doesn't
> > seem to be a good idea. There is no guarantee that this will work. So
> > either you find a common header file, where it is known that is (and
> > will be) included in panic.c, or you go with a weak function.
> 
> I went over the headers included to panic.c, either they are really
> irrelevant or not present on other archs. To me the best candidate
> header seems like "asm/bug.h"
> 
> $ find ./arch/ -name "bug.h" | grep include/asm | wc -l
> 16
> 
> $ ls ./arch | wc -l
> 22 # actually 21 as we shouldn't count Kconfig file
> 
> The architectures missing bug.h in their include/asm/ directory are
> hexagon, microblaze, nios2, um and xtensa
> 
> I believe 16/21 is a good ratio but if this is still not good
> enough, I'd like to fallback to __weak implementation.
> 
> What do you think Heiko?

Then go for __weak and wait if people shout at you :)

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-07-29 12:06 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27 10:36 [PATCH v2 0/3] Introduce arch_do_panic Mete Durlu
2026-07-27 10:36 ` [PATCH v2 1/3] panic: " Mete Durlu
2026-07-27 12:59   ` Bradley Morgan
2026-07-28 10:13     ` Mete Durlu
2026-07-27 10:36 ` [PATCH v2 2/3] s390: Implement arch_do_panic Mete Durlu
2026-07-27 13:01   ` Bradley Morgan
2026-07-28 10:43     ` Mete Durlu
2026-07-28 11:02       ` Bradley Morgan
2026-07-28 11:40         ` Sven Schnelle
2026-07-28 11:43           ` Bradley Morgan
2026-07-27 10:36 ` [PATCH v2 3/3] sparc: " Mete Durlu
2026-07-27 13:02   ` Bradley Morgan
2026-07-29  8:56 ` [PATCH v2 0/3] Introduce arch_do_panic Heiko Carstens
2026-07-29 10:48   ` Mete Durlu
2026-07-29 12:06     ` Heiko Carstens

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox