Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Introduce arch_do_panic
@ 2026-07-30  9:23 Mete Durlu
  2026-07-30  9:23 ` [PATCH v3 1/3] panic: " Mete Durlu
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Mete Durlu @ 2026-07-30  9:23 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Bradley Morgan
  Cc: linux-kernel, linux-s390, sparclinux, Mete Durlu

Changes in v3:
- Use __weak functions instead of symbol override via ifdef
- Patch 2: Better explain what is fixed (Bradley Morgan)
- Add statements about print order changes (Bradley Morgan)
- Patch 2: Add a pr_warn() to notify user about on_panic and
  panic_timeout relationship if they try to assign a trigger to
  on_panic action while panic_timeout is set to a value other than 0
  (Bradley Morgan)

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. Reordering also allows more information to be printed during
panic a for s390. sys_info(), kmsg_dump_desc(), etc... are now
reachable.

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.
Stop-A enablement markers are now printed after "end Kernel panic"
line.

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/kernel/ipl.c    | 19 +++++--------------
 arch/sparc/kernel/setup.c |  8 ++++++++
 include/linux/panic.h     |  2 ++
 kernel/panic.c            | 15 +++------------
 4 files changed, 18 insertions(+), 26 deletions(-)
---
base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
change-id: 20260724-arch_do_panic-a97f699aa332

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


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

* [PATCH v3 1/3] panic: Introduce arch_do_panic
  2026-07-30  9:23 [PATCH v3 0/3] Introduce arch_do_panic Mete Durlu
@ 2026-07-30  9:23 ` Mete Durlu
  2026-07-30  9:45   ` sashiko-bot
  2026-07-30 13:03   ` Bradley Morgan
  2026-07-30  9:23 ` [PATCH v3 2/3] s390: Implement arch_do_panic Mete Durlu
  2026-07-30  9:23 ` [PATCH v3 3/3] sparc: " Mete Durlu
  2 siblings, 2 replies; 10+ messages in thread
From: Mete Durlu @ 2026-07-30  9:23 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Bradley Morgan
  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>
---
 include/linux/panic.h | 2 ++
 kernel/panic.c        | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/linux/panic.h b/include/linux/panic.h
index f1dd417e54b2..98dd7dfd27de 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -110,4 +110,6 @@ extern void add_taint(unsigned flag, enum lockdep_ok);
 extern int test_taint(unsigned flag);
 extern unsigned long get_taint(void);
 
+void arch_do_panic(void);
+
 #endif	/* _LINUX_PANIC_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index 213725b612aa..726a97842232 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -567,6 +567,8 @@ static void panic_other_cpus_shutdown(bool crash_kexec)
 		crash_smp_send_stop();
 }
 
+void __weak arch_do_panic(void) {}
+
 /**
  * vpanic - halt the system
  * @fmt: The text string to print
@@ -756,6 +758,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.53.0


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

* [PATCH v3 2/3] s390: Implement arch_do_panic
  2026-07-30  9:23 [PATCH v3 0/3] Introduce arch_do_panic Mete Durlu
  2026-07-30  9:23 ` [PATCH v3 1/3] panic: " Mete Durlu
@ 2026-07-30  9:23 ` Mete Durlu
  2026-07-30 10:05   ` sashiko-bot
  2026-07-30 11:54   ` Heiko Carstens
  2026-07-30  9:23 ` [PATCH v3 3/3] sparc: " Mete Durlu
  2 siblings, 2 replies; 10+ messages in thread
From: Mete Durlu @ 2026-07-30  9:23 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Bradley Morgan
  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. By reordering s390's panic handler
allow more information to be printed during a panic.
The execution order of panic handlers now allows for user specified
panic_timeout value to be taken into account. This fixes the broken
"panic" kernel parameter for s390, earlier it was just ignored
inexplicibly.

This now means that the panic_timeout value takes precedence over user
defined on_panic behavior defined via "chshut" or writing to
/sys/firmware/shutdown_actions/on_panic.

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/kernel/ipl.c | 19 +++++--------------
 kernel/panic.c         |  3 ---
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
index 3c346b02ceb9..6a5fa9213450 100644
--- a/arch/s390/kernel/ipl.c
+++ b/arch/s390/kernel/ipl.c
@@ -2111,11 +2111,15 @@ static ssize_t on_panic_store(struct kobject *kobj,
 			      struct kobj_attribute *attr,
 			      const char *buf, size_t len)
 {
+	if (panic_timeout) {
+		pr_warn("on_panic action will be ignored in favor of panic timeout (panic=%d)",
+			panic_timeout);
+	}
 	return set_trigger(buf, &on_panic_trigger, len);
 }
 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 +2335,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 +2367,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 726a97842232..ee6e3f9e3900 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -752,9 +752,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.53.0


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

* [PATCH v3 3/3] sparc: Implement arch_do_panic
  2026-07-30  9:23 [PATCH v3 0/3] Introduce arch_do_panic Mete Durlu
  2026-07-30  9:23 ` [PATCH v3 1/3] panic: " Mete Durlu
  2026-07-30  9:23 ` [PATCH v3 2/3] s390: Implement arch_do_panic Mete Durlu
@ 2026-07-30  9:23 ` Mete Durlu
  2026-07-30 10:14   ` sashiko-bot
  2 siblings, 1 reply; 10+ messages in thread
From: Mete Durlu @ 2026-07-30  9:23 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Bradley Morgan
  Cc: linux-kernel, linux-s390, sparclinux, Mete Durlu

Implement sparc specific arch_do_panic() instead of using sparc specific
ifdef sections in vpanic() code.
Reorder arch specific panic handling, sparc's Stop-A messages are now
printed after "end Kernel panic" marker.

Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
 arch/sparc/kernel/setup.c | 8 ++++++++
 kernel/panic.c            | 9 ---------
 2 files changed, 8 insertions(+), 9 deletions(-)

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 ee6e3f9e3900..7dda841c16f9 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -744,15 +744,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.53.0


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

* Re: [PATCH v3 1/3] panic: Introduce arch_do_panic
  2026-07-30  9:23 ` [PATCH v3 1/3] panic: " Mete Durlu
@ 2026-07-30  9:45   ` sashiko-bot
  2026-07-30 13:03   ` Bradley Morgan
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30  9:45 UTC (permalink / raw)
  To: Mete Durlu; +Cc: linux-s390, Alexander Gordeev, Heiko Carstens, Vasily Gorbik

> 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/20260730-arch_do_panic-v3-0-d5401e683cdb@linux.ibm.com?part=1


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

* Re: [PATCH v3 2/3] s390: Implement arch_do_panic
  2026-07-30  9:23 ` [PATCH v3 2/3] s390: Implement arch_do_panic Mete Durlu
@ 2026-07-30 10:05   ` sashiko-bot
  2026-07-30 11:54   ` Heiko Carstens
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30 10:05 UTC (permalink / raw)
  To: Mete Durlu; +Cc: Vasily Gorbik, Alexander Gordeev, linux-s390, Heiko Carstens

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `pr_warn()` call in `on_panic_store()` is missing a terminating newline (`\n`), which will cause garbled console output.
- [Low] The file `arch/s390/kernel/ipl.c` does not include `<linux/panic.h>`, leading to a missing prototype compiler warning for `arch_do_panic()`.
--

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

s390: Implement arch_do_panic

This commit replaces the s390 custom panic notifier handler with the
arch_do_panic() hook. This reordering allows the user-specified
panic_timeout value to be evaluated correctly in common code. The timeout
now takes precedence over user-defined on_panic behavior.

> diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
> index 3c346b02ceb95..6a5fa9213450f 100644
> --- a/arch/s390/kernel/ipl.c
> +++ b/arch/s390/kernel/ipl.c
> @@ -2111,11 +2111,15 @@ static ssize_t on_panic_store(struct kobject *kobj,
>  			      struct kobj_attribute *attr,
>  			      const char *buf, size_t len)
>  {
> +	if (panic_timeout) {
> +		pr_warn("on_panic action will be ignored in favor of panic timeout (panic=%d)",
> +			panic_timeout);

[Severity: Medium]
Will this result in garbled console output since there is no terminating
newline in the format string? Because printk does not automatically append
a newline, the next message printed to the kernel log might be appended to
this same line.

> +	}
>  	return set_trigger(buf, &on_panic_trigger, len);
>  }
>  static struct kobj_attribute on_panic_attr = __ATTR_RW(on_panic);
>  
> -static void do_panic(void)
> +void arch_do_panic(void)

[Severity: Low]
Does this trigger a missing prototype compiler warning? The global function
arch_do_panic() is defined here to override the weak definition, but its
declaration is in linux/panic.h which does not seem to be included in this
file.

>  {
>  	lgr_info_log();
>  	on_panic_trigger.action->fn(&on_panic_trigger);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-arch_do_panic-v3-0-d5401e683cdb@linux.ibm.com?part=2

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

* Re: [PATCH v3 3/3] sparc: Implement arch_do_panic
  2026-07-30  9:23 ` [PATCH v3 3/3] sparc: " Mete Durlu
@ 2026-07-30 10:14   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30 10:14 UTC (permalink / raw)
  To: Mete Durlu; +Cc: Alexander Gordeev, Heiko Carstens, Vasily Gorbik, linux-s390

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing explicit `#include <linux/panic.h>` and `#include <linux/printk.h>` in `arch/sparc/kernel/setup.c`.
- [Low] Redundant local block-scoped `extern` declaration for `stop_a_enabled`.
--

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

sparc: Implement arch_do_panic

This commit implements a sparc specific arch_do_panic() instead of using
ifdef sections in vpanic() code. It also reorders arch specific panic handling.

> 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]
Should this file explicitly include <linux/panic.h> and <linux/printk.h>?

Since arch_do_panic() is a globally exported function, missing the panic.h
header can cause a missing prototypes warning during compilation. It might
also be better to explicitly include printk.h for pr_emerg() rather than
relying on implicit header inclusions.

> +	extern int stop_a_enabled;

[Severity: Low]
Is this block-scoped extern declaration still needed?

While this declaration was copied from its old location in kernel/panic.c,
it is redundant here because arch/sparc/kernel/setup.c already includes
<asm/setup.h>, which provides the exact same global extern declaration for
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");
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-arch_do_panic-v3-0-d5401e683cdb@linux.ibm.com?part=3

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

* Re: [PATCH v3 2/3] s390: Implement arch_do_panic
  2026-07-30  9:23 ` [PATCH v3 2/3] s390: Implement arch_do_panic Mete Durlu
  2026-07-30 10:05   ` sashiko-bot
@ 2026-07-30 11:54   ` Heiko Carstens
  2026-07-30 13:15     ` Mete Durlu
  1 sibling, 1 reply; 10+ messages in thread
From: Heiko Carstens @ 2026-07-30 11:54 UTC (permalink / raw)
  To: Mete Durlu
  Cc: Andrew Morton, Petr Mladek, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, Bradley Morgan, linux-kernel, linux-s390,
	sparclinux

On Thu, Jul 30, 2026 at 11:23:28AM +0200, Mete Durlu wrote:
> 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. By reordering s390's panic handler
> allow more information to be printed during a panic.
> The execution order of panic handlers now allows for user specified
> panic_timeout value to be taken into account. This fixes the broken
> "panic" kernel parameter for s390, earlier it was just ignored
> inexplicibly.
> 
> This now means that the panic_timeout value takes precedence over user
> defined on_panic behavior defined via "chshut" or writing to
> /sys/firmware/shutdown_actions/on_panic.
> 
> 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/kernel/ipl.c | 19 +++++--------------
>  kernel/panic.c         |  3 ---
>  2 files changed, 5 insertions(+), 17 deletions(-)

So, finally I took a closer look :)

Question: why is it desirable that panic_timeout takes precedence? The result
of this change is quite surprising: if anybody (e.g. a distribution) sets
CONFIG_PANIC_TIMEOUT to a non-zero value this completely breaks "on_panic"
behaviour on s390.

I could understand if this change would result in a larger timeout and
additional information being printed, but not that it breaks existing and
actually designed and desired behaviour.

This change also makes it more likely that the system deadlocks on console
messages, before the actual arch_do_panic() is called, if I'm not mistaken.
Which would also be a regression.

What I like about this patch set is that it removes architecture dependent
ifdefs from common code. But the side effects are very questionable.

The "obvious" cleanup would be to move only the existing ifdef'ed code
into arch_do_panic(), and only then provide semantical changes, which
wouldn't need to be part of such a cleanup series.

> diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
> index 3c346b02ceb9..6a5fa9213450 100644
> --- a/arch/s390/kernel/ipl.c
> +++ b/arch/s390/kernel/ipl.c
> @@ -2111,11 +2111,15 @@ static ssize_t on_panic_store(struct kobject *kobj,
>  			      struct kobj_attribute *attr,
>  			      const char *buf, size_t len)
>  {
> +	if (panic_timeout) {
> +		pr_warn("on_panic action will be ignored in favor of panic timeout (panic=%d)",
> +			panic_timeout);
> +	}
>  	return set_trigger(buf, &on_panic_trigger, len);
>  }

I'm wondering why AI doesn't complain about this user trigger-able warning
message. This is not good. *If* we go this way, writing to this attribute
should simply fail, instead of giving the user the impression that something
has been configured, which would actually do something.

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

* Re: [PATCH v3 1/3] panic: Introduce arch_do_panic
  2026-07-30  9:23 ` [PATCH v3 1/3] panic: " Mete Durlu
  2026-07-30  9:45   ` sashiko-bot
@ 2026-07-30 13:03   ` Bradley Morgan
  1 sibling, 0 replies; 10+ messages in thread
From: Bradley Morgan @ 2026-07-30 13:03 UTC (permalink / raw)
  To: Mete Durlu, 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

On 30 July 2026 10:23:27 BST, Mete Durlu <meted@linux.ibm.com> wrote:
>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>

Nice.

Reviewed-by: Bradley Morgan <include@grrlz.net>

>Signed-off-by: Mete Durlu <meted@linux.ibm.com>
>---
> include/linux/panic.h | 2 ++
> kernel/panic.c        | 3 +++
> 2 files changed, 5 insertions(+)
>
>diff --git a/include/linux/panic.h b/include/linux/panic.h
>index f1dd417e54b2..98dd7dfd27de 100644
>--- a/include/linux/panic.h
>+++ b/include/linux/panic.h
>@@ -110,4 +110,6 @@ extern void add_taint(unsigned flag, enum lockdep_ok);
> extern int test_taint(unsigned flag);
> extern unsigned long get_taint(void);
> 
>+void arch_do_panic(void);
>+
> #endif	/* _LINUX_PANIC_H */
>diff --git a/kernel/panic.c b/kernel/panic.c
>index 213725b612aa..726a97842232 100644
>--- a/kernel/panic.c
>+++ b/kernel/panic.c
>@@ -567,6 +567,8 @@ static void panic_other_cpus_shutdown(bool crash_kexec)
> 		crash_smp_send_stop();
> }
> 
>+void __weak arch_do_panic(void) {}
>+
> /**
>  * vpanic - halt the system
>  * @fmt: The text string to print
>@@ -756,6 +758,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;
> 
>
>


Thanks!

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

* Re: [PATCH v3 2/3] s390: Implement arch_do_panic
  2026-07-30 11:54   ` Heiko Carstens
@ 2026-07-30 13:15     ` Mete Durlu
  0 siblings, 0 replies; 10+ messages in thread
From: Mete Durlu @ 2026-07-30 13:15 UTC (permalink / raw)
  To: Heiko Carstens
  Cc: Andrew Morton, Petr Mladek, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, Bradley Morgan, linux-kernel, linux-s390,
	sparclinux

On 30/07/2026 13:54, Heiko Carstens wrote:
> On Thu, Jul 30, 2026 at 11:23:28AM +0200, Mete Durlu wrote:
>> 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. By reordering s390's panic handler
>> allow more information to be printed during a panic.
>> The execution order of panic handlers now allows for user specified
>> panic_timeout value to be taken into account. This fixes the broken
>> "panic" kernel parameter for s390, earlier it was just ignored
>> inexplicibly.
>>
>> This now means that the panic_timeout value takes precedence over user
>> defined on_panic behavior defined via "chshut" or writing to
>> /sys/firmware/shutdown_actions/on_panic.
>>
>> 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/kernel/ipl.c | 19 +++++--------------
>>   kernel/panic.c         |  3 ---
>>   2 files changed, 5 insertions(+), 17 deletions(-)
> 
> So, finally I took a closer look :)
> 
> Question: why is it desirable that panic_timeout takes precedence? The result
> of this change is quite surprising: if anybody (e.g. a distribution) sets
> CONFIG_PANIC_TIMEOUT to a non-zero value this completely breaks "on_panic"
> behaviour on s390.

panic timeout can be set during boot or compile time as you said, so it
can be used to determine what will happen to a system if it panics
during boot along with after boot. Since panic timeout covers a larger
area I thought it should get precedence.
Being able to choose what will happen on panic before boot is a super
power IMO and would help immensely if one would like to boot an untested
kernel via kexec for example.

> I could understand if this change would result in a larger timeout and
> additional information being printed, but not that it breaks existing and
> actually designed and desired behaviour.

For that to happen users have to "misconfigure" the system and try to
use both panic_timeout and a custom "on_panic" action. The same goes for
kdump, when kdump is configured "on_panic" actions are ignored silently
and system always dumps on panic.

> This change also makes it more likely that the system deadlocks on console
> messages, before the actual arch_do_panic() is called, if I'm not mistaken.
> Which would also be a regression.

This I wasn't aware of, I don't understand how it can deadlock on
console messages. I will look this up.

> What I like about this patch set is that it removes architecture dependent
> ifdefs from common code. But the side effects are very questionable.
> 
> The "obvious" cleanup would be to move only the existing ifdef'ed code
> into arch_do_panic(), and only then provide semantical changes, which
> wouldn't need to be part of such a cleanup series.

ifdef'ed code for s390 is unreachable as the code called by panic
notifiers aka "on_panic_trigger" do the same regardless of the
action it is configured to and stops with disabled_wait().

If that should be the way, I can remove the ifdef s390 chunk
from vpanic and propose the panic_timeout changes in a separate
patch.

>> diff --git a/arch/s390/kernel/ipl.c b/arch/s390/kernel/ipl.c
>> index 3c346b02ceb9..6a5fa9213450 100644
>> --- a/arch/s390/kernel/ipl.c
>> +++ b/arch/s390/kernel/ipl.c
>> @@ -2111,11 +2111,15 @@ static ssize_t on_panic_store(struct kobject *kobj,
>>   			      struct kobj_attribute *attr,
>>   			      const char *buf, size_t len)
>>   {
>> +	if (panic_timeout) {
>> +		pr_warn("on_panic action will be ignored in favor of panic timeout (panic=%d)",
>> +			panic_timeout);
>> +	}
>>   	return set_trigger(buf, &on_panic_trigger, len);
>>   }
> 
> I'm wondering why AI doesn't complain about this user trigger-able warning
> message. This is not good. *If* we go this way, writing to this attribute
> should simply fail, instead of giving the user the impression that something
> has been configured, which would actually do something.

My thought process was to put some marker to dmesg for users
to figure out what happened and why their system didn't respect
to their "on_panic" action. I know this is subobtimal but
simply failing the write to "on_panic" doesn't provide a good
solution IMO as users can first write to "on_panic" and then
set a value to panic_timeout from sysctl.

Regardless, I think I can remove semantic changes for s390
from this patchset and submit them from a different patch series
like you suggested.

Thank you for your input Heiko!

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

end of thread, other threads:[~2026-07-30 13:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30  9:23 [PATCH v3 0/3] Introduce arch_do_panic Mete Durlu
2026-07-30  9:23 ` [PATCH v3 1/3] panic: " Mete Durlu
2026-07-30  9:45   ` sashiko-bot
2026-07-30 13:03   ` Bradley Morgan
2026-07-30  9:23 ` [PATCH v3 2/3] s390: Implement arch_do_panic Mete Durlu
2026-07-30 10:05   ` sashiko-bot
2026-07-30 11:54   ` Heiko Carstens
2026-07-30 13:15     ` Mete Durlu
2026-07-30  9:23 ` [PATCH v3 3/3] sparc: " Mete Durlu
2026-07-30 10:14   ` sashiko-bot

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