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

[John Ogness <john.ogness@linutronix.de>]

On 2025-09-29, Petr Mladek <pmladek@suse.com> wrote:
> On Fri 2025-09-26 08:17:53, Breno Leitao wrote:
>> 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.

I would also like to add that write_atomic() calls are synchronized
against "unsafe sections"[0] of write_thread() and write_atomic()

... EXCEPT ...

As a last resort during panic, nbcon will perform hostile
takeovers. This means write_atomic() can be called even though the
driver is in unsafe sections. So your write_atomic() could be called
when your write_thread() is in an unsafe section. However,
write_atomic() can detect this by looking at unsafe_takeover of struct
nbcon_write_context. If true, you know your write_thread() was in an
unsafe section and you might need to be more careful or avoid certain
actions.

John

[0] Code surround by nbcon_enter_unsafe()/nbcon_exit_unsafe().