Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot

[Petr Mladek <pmladek@suse.com>]

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