Re: [PATCH printk v1 0/1] Allow unsafe ->write_atomic() for panic

[Petr Mladek <pmladek@suse.com>]

On Fri 2025-09-26 08:17:53, Breno Leitao wrote:
> Hello John,
> 
> On Fri, Sep 26, 2025 at 11:27:49AM +0206, John Ogness wrote:
> > On 2025-09-17, Breno Leitao <leitao@debian.org> wrote:
> > > Upon further consideration, it's worth noting that not all network
> > > drivers rely on irq-unsafe locks. In practice, only a subset of drivers
> > > use them, while most network drivers I'm familiar with maintain IRQ-safe
> > > TX paths.
> > >
> > > If we could determine the IRQ safety characteristics (IRQ-safe vs
> > > IRQ-unsafe TX) during netconsole registration, this would enable more
> > > optimized behavior: netconsole could register as CON_NBCON_ATOMIC_UNSAFE
> > > only when the underlying network adapter uses IRQ-unsafe locks. For
> > > adapters with IRQ-safe implementations, netconsole could safely utilize
> > > the ->write_atomic path without restrictions.
> > 
> > This is good to read. But note that if CON_NBCON_ATOMIC_UNSAFE is not
> > set, it is expected that ->write_atomic() will also function in NMI. So
> > being IRQ-safe may not be enough.
> 
> What are the other requirements for ->write_atomic() so it could be
> executed inside a NMI?

Ideally, it should be synchronized only via the nbcon console context
API and should not need any additional lock.

Note that the nbcon console context should get synchronized with
the normal device lock via some wrappers, for example, see
uart_port_lock() in include/linux/serial_core.h.


> That brings me another point, I suppose that netconsole callbacks are
> currently being called from NMI, given it is registered as a legacy
> console, and legacy consoles are printked from inside NMIs, right?

The legacy console handling is automatically deferred in_nmi(), see
is_printk_legacy_deferred().

The only exception is panic() where the consoles are explicitly
flushed. It is not 100% safe. But the risk of a deadlock is reduced
by calling bust_spinlocks(1) which sets oops_in_progress.
Console drivers use trylock when oops_in_progress is set.

I could imagine that we allow the trick with oops_in_progress
and trylock also in the "unsafe" write_atomic() callbacks.
But it would be better to use only the nbcon context ownership
as suggested above.

Best Regards,
Petr