mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
@ 2026-09-15 10:01 Ryan Roberts
  2026-09-15 10:06 ` Ryan Roberts
  2026-09-16 13:08 ` Petr Mladek
  0 siblings, 2 replies; 9+ messages in thread
From: Ryan Roberts @ 2026-09-15 10:01 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Steven Rostedt, John Ogness,
	Sergey Senozhatsky, Rio
  Cc: Ryan Roberts, linux-kernel

Netconsole registers with CON_NBCON_ATOMIC_UNSAFE, so its write_atomic()
callback is only usable during nbcon_atomic_flush_unsafe(). Previously
this was only called by vpanic() if panic_timeout=0 - i.e. if the system
was configured not to reboot on panic. So if the system was configured
to reboot on panic, netconsole would never receive the panic logs.

Move vpanic()'s emergency_restart() call to after the call to
nbcon_atomic_flush_unsafe() to solve this problem. The downside is that
potentially unsafe operations are now performed prior to
emergency_restart() which could theoretically reduce the chances of the
restart succeeding. But we are already in a panic situation so it could
be argued that everything is best effort already.

In older kernels (v6.19 and earlier) netconsole is able to print these
panic logs while the system is configured to reboot on panic. So from a
user perspective this is a regression caused by commit 7eab73b18630
("netconsole: convert to NBCON console infrastructure").

We have automated test systems without BMC access, which rely on these
two features working together.

Fixes: 7eab73b18630 ("netconsole: convert to NBCON console infrastructure")
Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---
 kernel/panic.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 213725b612aa..473dc7d13b23 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -732,32 +732,22 @@ void vpanic(const char *fmt, va_list args)
 			mdelay(PANIC_TIMER_STEP);
 		}
 	}
-	if (panic_timeout != 0) {
-		/*
-		 * This will not be a clean reboot, with everything
-		 * shutting down.  But if there is a chance of
-		 * rebooting the system it will be rebooted.
-		 */
-		if (panic_reboot_mode != REBOOT_UNDEFINED)
-			reboot_mode = panic_reboot_mode;
-		emergency_restart();
-	}
+	if (panic_timeout == 0) {
 #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");
-	}
+			"twice on console to return to the boot prom\n");
 #endif
 #if defined(CONFIG_S390)
-	disabled_wait();
+		disabled_wait();
 #endif
-	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
+		pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);

-	/* Do not scroll important messages printed above */
-	suppress_printk = 1;
+		/* Do not scroll important messages printed above */
+		suppress_printk = 1;
+	}

 	/*
 	 * The final messages may not have been printed if in a context that
@@ -767,6 +757,17 @@ void vpanic(const char *fmt, va_list args)
 	console_flush_on_panic(CONSOLE_FLUSH_PENDING);
 	nbcon_atomic_flush_unsafe();

+	if (panic_timeout != 0) {
+		/*
+		 * This will not be a clean reboot, with everything
+		 * shutting down.  But if there is a chance of
+		 * rebooting the system it will be rebooted.
+		 */
+		if (panic_reboot_mode != REBOOT_UNDEFINED)
+			reboot_mode = panic_reboot_mode;
+		emergency_restart();
+	}
+
 	local_irq_enable();
 	for (i = 0; ; i += PANIC_TIMER_STEP) {
 		touch_softlockup_watchdog();
--
2.43.0


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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-15 10:01 [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot Ryan Roberts
@ 2026-09-15 10:06 ` Ryan Roberts
  2026-09-15 18:49   ` Bradley Morgan
  2026-09-16 13:08 ` Petr Mladek
  1 sibling, 1 reply; 9+ messages in thread
From: Ryan Roberts @ 2026-09-15 10:06 UTC (permalink / raw)
  To: Andrew Morton, Petr Mladek, Steven Rostedt, John Ogness,
	Sergey Senozhatsky, Rio
  Cc: linux-kernel

On 15/09/2026 11:01, Ryan Roberts wrote:
> Netconsole registers with CON_NBCON_ATOMIC_UNSAFE, so its write_atomic()
> callback is only usable during nbcon_atomic_flush_unsafe(). Previously
> this was only called by vpanic() if panic_timeout=0 - i.e. if the system
> was configured not to reboot on panic. So if the system was configured
> to reboot on panic, netconsole would never receive the panic logs.
> 
> Move vpanic()'s emergency_restart() call to after the call to
> nbcon_atomic_flush_unsafe() to solve this problem. The downside is that
> potentially unsafe operations are now performed prior to
> emergency_restart() which could theoretically reduce the chances of the
> restart succeeding. But we are already in a panic situation so it could
> be argued that everything is best effort already.
> 
> In older kernels (v6.19 and earlier) netconsole is able to print these
> panic logs while the system is configured to reboot on panic. So from a
> user perspective this is a regression caused by commit 7eab73b18630
> ("netconsole: convert to NBCON console infrastructure").
> 
> We have automated test systems without BMC access, which rely on these
> two features working together.
> 
> Fixes: 7eab73b18630 ("netconsole: convert to NBCON console infrastructure")
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---

Oops forgot to add commentary to the bottom:

I've set this as RFC because I'm not sure if the current behaviour is a
deliberate design decision? I guess if we flush the unsafe console, there is a
possibility it deadlocks, preventing the subsequent reboot. If people are
concerned about this, perhaps this needs a cmdline flag?

Thanks,
Ryan



>  kernel/panic.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 213725b612aa..473dc7d13b23 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -732,32 +732,22 @@ void vpanic(const char *fmt, va_list args)
>  			mdelay(PANIC_TIMER_STEP);
>  		}
>  	}
> -	if (panic_timeout != 0) {
> -		/*
> -		 * This will not be a clean reboot, with everything
> -		 * shutting down.  But if there is a chance of
> -		 * rebooting the system it will be rebooted.
> -		 */
> -		if (panic_reboot_mode != REBOOT_UNDEFINED)
> -			reboot_mode = panic_reboot_mode;
> -		emergency_restart();
> -	}
> +	if (panic_timeout == 0) {
>  #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");
> -	}
> +			"twice on console to return to the boot prom\n");
>  #endif
>  #if defined(CONFIG_S390)
> -	disabled_wait();
> +		disabled_wait();
>  #endif
> -	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> +		pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> 
> -	/* Do not scroll important messages printed above */
> -	suppress_printk = 1;
> +		/* Do not scroll important messages printed above */
> +		suppress_printk = 1;
> +	}
> 
>  	/*
>  	 * The final messages may not have been printed if in a context that
> @@ -767,6 +757,17 @@ void vpanic(const char *fmt, va_list args)
>  	console_flush_on_panic(CONSOLE_FLUSH_PENDING);
>  	nbcon_atomic_flush_unsafe();
> 
> +	if (panic_timeout != 0) {
> +		/*
> +		 * This will not be a clean reboot, with everything
> +		 * shutting down.  But if there is a chance of
> +		 * rebooting the system it will be rebooted.
> +		 */
> +		if (panic_reboot_mode != REBOOT_UNDEFINED)
> +			reboot_mode = panic_reboot_mode;
> +		emergency_restart();
> +	}
> +
>  	local_irq_enable();
>  	for (i = 0; ; i += PANIC_TIMER_STEP) {
>  		touch_softlockup_watchdog();
> --
> 2.43.0
> 


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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-15 10:06 ` Ryan Roberts
@ 2026-09-15 18:49   ` Bradley Morgan
  2026-09-16  7:44     ` Ryan Roberts
  0 siblings, 1 reply; 9+ messages in thread
From: Bradley Morgan @ 2026-09-15 18:49 UTC (permalink / raw)
  To: ryan.roberts
  Cc: akpm, john.ogness, linux-kernel, pmladek, rioo.tsukatsukii,
	rostedt, senozhatsky

Hey, I think thisll conflict with the arch_do_panic() patch (b45d6e4b3580)

Once you rebase. I'd reckon the
change shrinks to just moving the panic_timeout != 0 block below
nbcon_atomic_flush_unsafe().

On your RFC question, whether the current behavior is deliberate: I
don't think it is. nbcon_atomic_flush_unsafe() was placed at the end of
vpanic() by 5dde3b735413 as a final attempt to flush pending messages, and
at
that point no console was actually CON_NBCON_ATOMIC_UNSAFE, netconsole
was still legacy. There was nothing to lose by putting it last. The
ordering only became a bug once the first unsafe console showed up.
John and Petr can confirm, but I found no sign anyone chose this.

 
--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-15 18:49   ` Bradley Morgan
@ 2026-09-16  7:44     ` Ryan Roberts
  2026-09-16 14:58       ` Bradley Morgan
  0 siblings, 1 reply; 9+ messages in thread
From: Ryan Roberts @ 2026-09-16  7:44 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: akpm, john.ogness, linux-kernel, pmladek, rioo.tsukatsukii,
	rostedt, senozhatsky

On 15/09/2026 19:49, Bradley Morgan wrote:
> Hey, I think thisll conflict with the arch_do_panic() patch (b45d6e4b3580)

Ahh I see it here:
https://lore.kernel.org/all/20260831-arch_do_panic-v6-0-a1e170a9e7fd@linux.ibm.com/

And looks like it's in Andrew's mm-nonmm-unstable branch. I guess I should
repost based on that branch?

> 
> Once you rebase. I'd reckon the
> change shrinks to just moving the panic_timeout != 0 block below
> nbcon_atomic_flush_unsafe().

I think I'll also want to make the call to arch_do_panic() conditional on
(panic_timeout == 0) so it's equivalent to the patch I sent?

> 
> On your RFC question, whether the current behavior is deliberate: I
> don't think it is. nbcon_atomic_flush_unsafe() was placed at the end of
> vpanic() by 5dde3b735413 as a final attempt to flush pending messages, and
> at
> that point no console was actually CON_NBCON_ATOMIC_UNSAFE, netconsole
> was still legacy. There was nothing to lose by putting it last. The
> ordering only became a bug once the first unsafe console showed up.
> John and Petr can confirm, but I found no sign anyone chose this.

OK great; I'll wait a few days for others to chime in, otherwise I'll plan to
repost based on mm-nonmm-unstable, dropping the RFC tag.

Thanks,
Ryan


> 
>  
> --- Thanks!
> https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/


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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-15 10:01 [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot Ryan Roberts
  2026-09-15 10:06 ` Ryan Roberts
@ 2026-09-16 13:08 ` Petr Mladek
  2026-10-22 20:35   ` Gustavo Luiz Duarte
  2026-10-23  7:57   ` John Ogness
  1 sibling, 2 replies; 9+ messages in thread
From: Petr Mladek @ 2026-09-16 13:08 UTC (permalink / raw)
  To: Ryan Roberts
  Cc: Andrew Morton, Steven Rostedt, John Ogness, Sergey Senozhatsky,
	Rio, linux-kernel

On Tue 2026-09-15 11:01:38, Ryan Roberts wrote:
> Netconsole registers with CON_NBCON_ATOMIC_UNSAFE, so its write_atomic()
> callback is only usable during nbcon_atomic_flush_unsafe(). Previously
> this was only called by vpanic() if panic_timeout=0 - i.e. if the system
> was configured not to reboot on panic. So if the system was configured
> to reboot on panic, netconsole would never receive the panic logs.
> 
> Move vpanic()'s emergency_restart() call to after the call to
> nbcon_atomic_flush_unsafe() to solve this problem. The downside is that
> potentially unsafe operations are now performed prior to
> emergency_restart() which could theoretically reduce the chances of the
> restart succeeding. But we are already in a panic situation so it could
> be argued that everything is best effort already.

The panic() code is like walking on bumpy roads. It tries to be as
safe as possible. IMHO, it has few important tasks:

  + show debug messages
  + create crash dump (optional)
  + safely reboot (optional)

The debug messages are important but some users might prioritize
reboot over the debug messages. It helps when the panic() is rare
(low likely race) and the system is quickly able to handle the load
after the reboot.

This is why we put nbcon_atomic_flush_unsafe() only into the path
which ends with an infinite loop.

> In older kernels (v6.19 and earlier) netconsole is able to print these
> panic logs while the system is configured to reboot on panic. So from a
> user perspective this is a regression caused by commit 7eab73b18630
> ("netconsole: convert to NBCON console infrastructure").
> 
> We have automated test systems without BMC access, which rely on these
> two features working together.

I see. To be precise, I would say that the primary culprit is the
commit 187de7c212e5fa87 ("printk: nbcon: Allow unsafe write_atomic()
for panic"). It adds support for consoles which will be flushed
in panic() only by nbcon_atomic_flush_unsafe().

It is a big difference because printk() tries to flush both
legacy and "safe" NBCON consoles directly in panic(). In addition,
they are explicitely flushed by console_flush_on_panic().

console_flush_on_panic() is called more times in panic().
And it tries to flush legacy consoles the unsafe way even
before emergency_restart().

OK, we should close this gap. But I suggest another solution.

We already have the following functions for flushing NBCON
consoles using write_atomic()


  + nbcon_atomic_flush_pending() - flush CON_NBCON consoles
	using con->write_atomic() but only when safe

  + nbcon_atomic_flush_unsafe() - flush CON_NBCON consoles
	using con->write_atomic() and allow unsafe takeover.

I suggest to add 3rd variant, for example:

  + nbcon_atomic_unsafe_flush_unsafe() - flush
	CON_NBCON_ATOMIC_UNSAFE consoles with con->write_atomic()
	and allow unsafe takeover.

And I would call this 3rd variant in console_flush_on_panic(),
something like:

void console_flush_on_panic(enum con_flush_mode mode)
{
[...]
	printk_get_console_flush_type(&ft);
	if (ft.nbcon_atomic) {
		nbcon_atomic_flush_pending();
		nbcon_atomic_unsafe_flush_unsafe();
	}

	/* Flush legacy consoles once allowed, even when dangerous. */
	if (legacy_allow_panic_sync)
		console_flush_all(false, &next_seq, &handover);
}

But maybe, this is over engineered and we should just call
nbcon_atomic_flush_unsafe() in console_flush_on_panic(), like:

void console_flush_on_panic(enum con_flush_mode mode)
{
[...]
	printk_get_console_flush_type(&ft);
	if (ft.nbcon_atomic) {
		nbcon_atomic_flush_pending();
		nbcon_atomic_flush_unsafe();
	}

	/* Flush legacy consoles once allowed, even when dangerous. */
	if (legacy_allow_panic_sync)
		console_flush_all(false, &next_seq, &handover);
}

If there there are people really concerned about the reboot
reliability then we might need to make it configurable, e.g.
by adding reliable_panic command line option which would skip
the unsafe flush before reboot.

See below.

> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -732,32 +732,22 @@ void vpanic(const char *fmt, va_list args)
>  			mdelay(PANIC_TIMER_STEP);
>  		}
>  	}
> -	if (panic_timeout != 0) {
> -		/*
> -		 * This will not be a clean reboot, with everything
> -		 * shutting down.  But if there is a chance of
> -		 * rebooting the system it will be rebooted.
> -		 */
> -		if (panic_reboot_mode != REBOOT_UNDEFINED)
> -			reboot_mode = panic_reboot_mode;
> -		emergency_restart();
> -	}
> +	if (panic_timeout == 0) {
>  #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");
> -	}
> +			"twice on console to return to the boot prom\n");
>  #endif
>  #if defined(CONFIG_S390)
> -	disabled_wait();
> +		disabled_wait();
>  #endif
> -	pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> +		pr_emerg("---[ end Kernel panic - not syncing: %s ]---\n", buf);
> 
> -	/* Do not scroll important messages printed above */
> -	suppress_printk = 1;
> +		/* Do not scroll important messages printed above */
> +		suppress_printk = 1;

We should not supress printk when emergency_restart() is going to
be called. It might print some error messages which might explain
why it failed, ...

> +	}
> 
>  	/*
>  	 * The final messages may not have been printed if in a context that
> @@ -767,6 +757,17 @@ void vpanic(const char *fmt, va_list args)
>  	console_flush_on_panic(CONSOLE_FLUSH_PENDING);
>  	nbcon_atomic_flush_unsafe();
> 
> +	if (panic_timeout != 0) {
> +		/*
> +		 * This will not be a clean reboot, with everything
> +		 * shutting down.  But if there is a chance of
> +		 * rebooting the system it will be rebooted.
> +		 */
> +		if (panic_reboot_mode != REBOOT_UNDEFINED)
> +			reboot_mode = panic_reboot_mode;
> +		emergency_restart();
> +	}
> +
>  	local_irq_enable();
>  	for (i = 0; ; i += PANIC_TIMER_STEP) {
>  		touch_softlockup_watchdog();

Best Regards,
Petr

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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-16  7:44     ` Ryan Roberts
@ 2026-09-16 14:58       ` Bradley Morgan
  0 siblings, 0 replies; 9+ messages in thread
From: Bradley Morgan @ 2026-09-16 14:58 UTC (permalink / raw)
  To: Ryan Roberts
  Cc: akpm, john.ogness, linux-kernel, pmladek, rioo.tsukatsukii,
	rostedt, senozhatsky

On 16 September 2026 08:44:53 BST, Ryan Roberts <ryan.roberts@arm.com>
wrote:
>On 15/09/2026 19:49, Bradley Morgan wrote:
>> Hey, I think thisll conflict with the arch_do_panic() patch
>(b45d6e4b3580)
>
>Ahh I see it here:
>https://lore.kernel.org/all/20260831-arch_do_panic-v6-0-a1e170a9e7fd@linux.ibm.com/
>
>And looks like it's in Andrew's mm-nonmm-unstable branch. I guess I should
>repost based on that branch?

Yes.

(That branch also goes to linux-next)

>
>> 
>> Once you rebase. I'd reckon the
>> change shrinks to just moving the panic_timeout != 0 block below
>> nbcon_atomic_flush_unsafe().
>
>I think I'll also want to make the call to arch_do_panic() conditional on
>(panic_timeout == 0) so it's equivalent to the patch I sent?
>
>> 
>> On your RFC question, whether the current behavior is deliberate: I
>> don't think it is. nbcon_atomic_flush_unsafe() was placed at the end of
>> vpanic() by 5dde3b735413 as a final attempt to flush pending messages,
>and
>> at
>> that point no console was actually CON_NBCON_ATOMIC_UNSAFE, netconsole
>> was still legacy. There was nothing to lose by putting it last. The
>> ordering only became a bug once the first unsafe console showed up.
>> John and Petr can confirm, but I found no sign anyone chose this.
>
>OK great; I'll wait a few days for others to chime in, otherwise I'll plan
>to
>repost based on mm-nonmm-unstable, dropping the RFC tag.
>
>Thanks,
>Ryan
>
>
>> 
>>  
>> --- Thanks!
>>
>https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/
>

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/

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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-16 13:08 ` Petr Mladek
@ 2026-10-22 20:35   ` Gustavo Luiz Duarte
  2026-10-23  7:57   ` John Ogness
  1 sibling, 0 replies; 9+ messages in thread
From: Gustavo Luiz Duarte @ 2026-10-22 20:35 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Ryan Roberts, Andrew Morton, Steven Rostedt, John Ogness,
	Sergey Senozhatsky, Rio, linux-kernel

On 2026-09-16 15:08:44+02:00, Petr Mladek wrote:
> On Tue 2026-09-15 11:01:38, Ryan Roberts wrote:
> 
> > Netconsole registers with CON_NBCON_ATOMIC_UNSAFE, so its write_atomic()
> > callback is only usable during nbcon_atomic_flush_unsafe(). Previously
> > this was only called by vpanic() if panic_timeout=0 - i.e. if the system
> > was configured not to reboot on panic. So if the system was configured
> > to reboot on panic, netconsole would never receive the panic logs.
> > 
> > Move vpanic()'s emergency_restart() call to after the call to
> > nbcon_atomic_flush_unsafe() to solve this problem. The downside is that
> > potentially unsafe operations are now performed prior to
> > emergency_restart() which could theoretically reduce the chances of the
> > restart succeeding. But we are already in a panic situation so it could
> > be argued that everything is best effort already.
> 
> The panic() code is like walking on bumpy roads. It tries to be as
> safe as possible. IMHO, it has few important tasks:
> 
>   + show debug messages
>   + create crash dump (optional)
>   + safely reboot (optional)
> 
> The debug messages are important but some users might prioritize
> reboot over the debug messages. It helps when the panic() is rare
> (low likely race) and the system is quickly able to handle the load
> after the reboot.
> 
> This is why we put nbcon_atomic_flush_unsafe() only into the path
> which ends with an infinite loop.
> 
> > In older kernels (v6.19 and earlier) netconsole is able to print these
> > panic logs while the system is configured to reboot on panic. So from a
> > user perspective this is a regression caused by commit 7eab73b18630
> > ("netconsole: convert to NBCON console infrastructure").
> > 
> > We have automated test systems without BMC access, which rely on these
> > two features working together.
> 
> I see. To be precise, I would say that the primary culprit is the
> commit 187de7c212e5fa87 ("printk: nbcon: Allow unsafe write_atomic()
> for panic"). It adds support for consoles which will be flushed
> in panic() only by nbcon_atomic_flush_unsafe().
> 
> It is a big difference because printk() tries to flush both
> legacy and "safe" NBCON consoles directly in panic(). In addition,
> they are explicitely flushed by console_flush_on_panic().
> 
> console_flush_on_panic() is called more times in panic().
> And it tries to flush legacy consoles the unsafe way even
> before emergency_restart().
> 
> OK, we should close this gap. But I suggest another solution.
> 
> We already have the following functions for flushing NBCON
> consoles using write_atomic()
> 
> 
>   + nbcon_atomic_flush_pending() - flush CON_NBCON consoles
> 	using con->write_atomic() but only when safe
> 
>   + nbcon_atomic_flush_unsafe() - flush CON_NBCON consoles
> 	using con->write_atomic() and allow unsafe takeover.
> 
> I suggest to add 3rd variant, for example:
> 
>   + nbcon_atomic_unsafe_flush_unsafe() - flush
> 	CON_NBCON_ATOMIC_UNSAFE consoles with con->write_atomic()
> 	and allow unsafe takeover.
> 
> And I would call this 3rd variant in console_flush_on_panic(),
> something like:
> 
> void console_flush_on_panic(enum con_flush_mode mode)
> {
> [...]
> 	printk_get_console_flush_type(&ft);
> 	if (ft.nbcon_atomic) {
> 		nbcon_atomic_flush_pending();
> 		nbcon_atomic_unsafe_flush_unsafe();
> 	}
> 
> 	/* Flush legacy consoles once allowed, even when dangerous. */
> 	if (legacy_allow_panic_sync)
> 		console_flush_all(false, &next_seq, &handover);
> }
> 
> But maybe, this is over engineered and we should just call
> nbcon_atomic_flush_unsafe() in console_flush_on_panic(), like:
> 
> void console_flush_on_panic(enum con_flush_mode mode)
> {
> [...]
> 	printk_get_console_flush_type(&ft);
> 	if (ft.nbcon_atomic) {
> 		nbcon_atomic_flush_pending();
> 		nbcon_atomic_flush_unsafe();
> 	}
> 
> 	/* Flush legacy consoles once allowed, even when dangerous. */
> 	if (legacy_allow_panic_sync)
> 		console_flush_all(false, &next_seq, &handover);
> }

Hi Ryan, are you still planning to send a non-RFC patch? If not, I'm
happy to take over and send a patch with Petr's suggestion.
We are seeing cases in production of panic logs not reaching netconsole
because of this.



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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-09-16 13:08 ` Petr Mladek
  2026-10-22 20:35   ` Gustavo Luiz Duarte
@ 2026-10-23  7:57   ` John Ogness
  2026-10-23 10:36     ` Petr Mladek
  1 sibling, 1 reply; 9+ messages in thread
From: John Ogness @ 2026-10-23  7:57 UTC (permalink / raw)
  To: Petr Mladek, Ryan Roberts
  Cc: Andrew Morton, Steven Rostedt, Sergey Senozhatsky, Rio, linux-kernel

Sorry for the late response... this fell through the cracks. Response
below...

On 2026-09-16, Petr Mladek <pmladek@suse.com> wrote:
>> In older kernels (v6.19 and earlier) netconsole is able to print these
>> panic logs while the system is configured to reboot on panic. So from a
>> user perspective this is a regression caused by commit 7eab73b18630
>> ("netconsole: convert to NBCON console infrastructure").
>> 
>> We have automated test systems without BMC access, which rely on these
>> two features working together.
>
> I see. To be precise, I would say that the primary culprit is the
> commit 187de7c212e5fa87 ("printk: nbcon: Allow unsafe write_atomic()
> for panic"). It adds support for consoles which will be flushed
> in panic() only by nbcon_atomic_flush_unsafe().
>
> It is a big difference because printk() tries to flush both
> legacy and "safe" NBCON consoles directly in panic(). In addition,
> they are explicitely flushed by console_flush_on_panic().
>
> console_flush_on_panic() is called more times in panic().
> And it tries to flush legacy consoles the unsafe way even
> before emergency_restart().
>
> OK, we should close this gap. But I suggest another solution.
>
> We already have the following functions for flushing NBCON
> consoles using write_atomic()
>
>
>   + nbcon_atomic_flush_pending() - flush CON_NBCON consoles
> 	using con->write_atomic() but only when safe
>
>   + nbcon_atomic_flush_unsafe() - flush CON_NBCON consoles
> 	using con->write_atomic() and allow unsafe takeover.
>
> I suggest to add 3rd variant, for example:
>
>   + nbcon_atomic_unsafe_flush_unsafe() - flush
> 	CON_NBCON_ATOMIC_UNSAFE consoles with con->write_atomic()
> 	and allow unsafe takeover.

nbcon_atomic_flush_unsafe() already includes CON_NBCON_ATOMIC_UNSAFE.

> And I would call this 3rd variant in console_flush_on_panic(),
> something like:
>
> void console_flush_on_panic(enum con_flush_mode mode)
> {
> [...]
> 	printk_get_console_flush_type(&ft);
> 	if (ft.nbcon_atomic) {
> 		nbcon_atomic_flush_pending();
> 		nbcon_atomic_unsafe_flush_unsafe();

s/nbcon_atomic_unsafe_flush_unsafe/nbcon_atomic_flush_unsafe/

console_flush_on_panic() is used in multiple places and depending on the
user, it may be too risky. Yes, if there are legacy consoles the user is
already taking this risk. But with nbcon we would like to excercise some
risk management.

> If there there are people really concerned about the reboot
> reliability then we might need to make it configurable, e.g.
> by adding reliable_panic command line option which would skip
> the unsafe flush before reboot.

Yes. When originally developing the nbcon, tglx and I talked about many
more details related to this topic. For example, the ability to order
the unsafe flushing. That is one of the reasons that nbcon flushes the
full backlog for each console, rather than each console only flushing a
single record.

If we are going to expose an interface to configure if unsafe flushing
should occur, then we should also consider ordering. Even if we do not
want to build in the ordering support right now, the interface should be
such that it can be easily extended to support this.

*** WARNING: Now we get into my brainstorming, which could be rough. ***

Rather than a big hammer option, I see this more as a per-console
configuration (similar to how we want to go with the console loglevels).

Netconsole is a bit tricky, because it has its own format separate from
console. This would need to be addressed for the per-console loglevels
as well. Maybe it should be extended to something like:

netconsole=[+][r][src-port]@[src-ip]/[<dev>],[tgt-port]@<tgt-ip>/[tgt-macaddr][,options]

Which would allow things like:

netconsole=@/,@10.10.0.2/,loglevel:3,unsafe_panic:1

This would also allow to specify that consoles should _not_
perform unsafe flushing:

console=ttyS0,115200,unsafe_panic:0

And it could be used for ordering, flusing from highest value to lowest:

netconsole=@/,@10.10.0.2/,unsafe_panic:2 console=ttyS0,115200,unsafe_panic:12

(First ttyS0 is unsafe-flushed, then the netconsole is unsafe-flushed.)

For consoles with CON_NBCON_ATOMIC_UNSAFE, the default would be
unsafe_panic:0. Otherwise the default would be unsafe_panic:1.

Unfortunately the above idea does not consider the reboot vs. busy-loop
paths. I could imagine there are those that only want a different
ordering/disabling for paths where the kernel will continue to be useful
(for example, a reboot). I think that makes sense. But I am not really
sure how that distinction should be made. We don't want to configure
ourselves to death. Maybe that distinction is not relevant.

John

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

* Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
  2026-10-23  7:57   ` John Ogness
@ 2026-10-23 10:36     ` Petr Mladek
  0 siblings, 0 replies; 9+ messages in thread
From: Petr Mladek @ 2026-10-23 10:36 UTC (permalink / raw)
  To: John Ogness
  Cc: Ryan Roberts, Andrew Morton, Steven Rostedt, Sergey Senozhatsky,
	Rio, linux-kernel

On Fri 2026-10-23 10:03:47, John Ogness wrote:
> Sorry for the late response... this fell through the cracks. Response
> below...
> 
> On 2026-09-16, Petr Mladek <pmladek@suse.com> wrote:
> >> In older kernels (v6.19 and earlier) netconsole is able to print these
> >> panic logs while the system is configured to reboot on panic. So from a
> >> user perspective this is a regression caused by commit 7eab73b18630
> >> ("netconsole: convert to NBCON console infrastructure").
> >> 
> >> We have automated test systems without BMC access, which rely on these
> >> two features working together.
> >
> > I see. To be precise, I would say that the primary culprit is the
> > commit 187de7c212e5fa87 ("printk: nbcon: Allow unsafe write_atomic()
> > for panic"). It adds support for consoles which will be flushed
> > in panic() only by nbcon_atomic_flush_unsafe().
> >
> > It is a big difference because printk() tries to flush both
> > legacy and "safe" NBCON consoles directly in panic(). In addition,
> > they are explicitely flushed by console_flush_on_panic().
> >
> > console_flush_on_panic() is called more times in panic().
> > And it tries to flush legacy consoles the unsafe way even
> > before emergency_restart().
> >
> > OK, we should close this gap. But I suggest another solution.
> >
> > We already have the following functions for flushing NBCON
> > consoles using write_atomic()
> >
> >
> >   + nbcon_atomic_flush_pending() - flush CON_NBCON consoles
> > 	using con->write_atomic() but only when safe
> >
> >   + nbcon_atomic_flush_unsafe() - flush CON_NBCON consoles
> > 	using con->write_atomic() and allow unsafe takeover.
> >
> > I suggest to add 3rd variant, for example:
> >
> >   + nbcon_atomic_unsafe_flush_unsafe() - flush
> > 	CON_NBCON_ATOMIC_UNSAFE consoles with con->write_atomic()
> > 	and allow unsafe takeover.
> 
> nbcon_atomic_flush_unsafe() already includes CON_NBCON_ATOMIC_UNSAFE.

I was not clear enoungh. The purpose of the 3rd variant was
to allow the unsafe takeover only for consoles with
CON_NBCON_ATOMIC_UNSAFE. And rely only on the safe atomic flush
for consoles which support it.

But it is likely too complicated and probably not worth it.


> > And I would call this 3rd variant in console_flush_on_panic(),
> > something like:
> >
> > void console_flush_on_panic(enum con_flush_mode mode)
> > {
> > [...]
> > 	printk_get_console_flush_type(&ft);
> > 	if (ft.nbcon_atomic) {
> > 		nbcon_atomic_flush_pending();
> > 		nbcon_atomic_unsafe_flush_unsafe();
> 
> s/nbcon_atomic_unsafe_flush_unsafe/nbcon_atomic_flush_unsafe/

Yes, I am fine with this.

> console_flush_on_panic() is used in multiple places and depending on the
> user, it may be too risky. Yes, if there are legacy consoles the user is
> already taking this risk. But with nbcon we would like to excercise some
> risk management.
> 
> > If there there are people really concerned about the reboot
> > reliability then we might need to make it configurable, e.g.
> > by adding reliable_panic command line option which would skip
> > the unsafe flush before reboot.
> 
> Yes. When originally developing the nbcon, tglx and I talked about many
> more details related to this topic. For example, the ability to order
> the unsafe flushing. That is one of the reasons that nbcon flushes the
> full backlog for each console, rather than each console only flushing a
> single record.
> 
> If we are going to expose an interface to configure if unsafe flushing
> should occur, then we should also consider ordering. Even if we do not
> want to build in the ordering support right now, the interface should be
> such that it can be easily extended to support this.
> 
> *** WARNING: Now we get into my brainstorming, which could be rough. ***
> 
> Rather than a big hammer option, I see this more as a per-console
> configuration (similar to how we want to go with the console loglevels).
> 
> Netconsole is a bit tricky, because it has its own format separate from
> console. This would need to be addressed for the per-console loglevels
> as well. Maybe it should be extended to something like:
> 
> netconsole=[+][r][src-port]@[src-ip]/[<dev>],[tgt-port]@<tgt-ip>/[tgt-macaddr][,options]
> 
> Which would allow things like:
> 
> netconsole=@/,@10.10.0.2/,loglevel:3,unsafe_panic:1
> 
> This would also allow to specify that consoles should _not_
> perform unsafe flushing:
> 
> console=ttyS0,115200,unsafe_panic:0
> 
> And it could be used for ordering, flusing from highest value to lowest:
> 
> netconsole=@/,@10.10.0.2/,unsafe_panic:2 console=ttyS0,115200,unsafe_panic:12
> 
> (First ttyS0 is unsafe-flushed, then the netconsole is unsafe-flushed.)
> 
> For consoles with CON_NBCON_ATOMIC_UNSAFE, the default would be
> unsafe_panic:0. Otherwise the default would be unsafe_panic:1.
> 
> Unfortunately the above idea does not consider the reboot vs. busy-loop
> paths. I could imagine there are those that only want a different
> ordering/disabling for paths where the kernel will continue to be useful
> (for example, a reboot). I think that makes sense. But I am not really
> sure how that distinction should be made. We don't want to configure
> ourselves to death. Maybe that distinction is not relevant.

This sounds reasonable. And it is good to have a plan.  Well, I would
add this complexity only when there is a real demand for it :-)

Best Regards,
Petr

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

end of thread, other threads:[~2026-10-23 10:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15 10:01 [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot Ryan Roberts
2026-09-15 10:06 ` Ryan Roberts
2026-09-15 18:49   ` Bradley Morgan
2026-09-16  7:44     ` Ryan Roberts
2026-09-16 14:58       ` Bradley Morgan
2026-09-16 13:08 ` Petr Mladek
2026-10-22 20:35   ` Gustavo Luiz Duarte
2026-10-23  7:57   ` John Ogness
2026-10-23 10:36     ` Petr Mladek

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

all inboxes | Powered by JetHome®