mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>, Ryan Roberts <ryan.roberts@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Rio <rioo.tsukatsukii@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
Date: Fri, 23 Oct 2026 10:03:47 +0206	[thread overview]
Message-ID: <87bj8kkfwk.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <aqqU3I7d9lMAjSvy@pathway.suse.cz>

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

  parent reply	other threads:[~2026-10-23  7:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15 10:01 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 [this message]
2026-10-23 10:36     ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bj8kkfwk.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rioo.tsukatsukii@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=ryan.roberts@arm.com \
    --cc=senozhatsky@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®