Re: [RFC PATCH v1 20/25] serial: 8250: implement write_atomic

[Petr Mladek]

On Fri 2019-03-08 05:05:12, John Ogness wrote:
> On 2019-02-27, Petr Mladek <pmladek@xxxxxxxx> wrote:
> >>>> Implement a non-sleeping NMI-safe write_atomic console function in
> >>>> order to support emergency printk messages.
> >>>
> >>> OK, it would be safe when prb_lock() is the only lock taken
> >>> in the NMI handler.
> >> 
> >> Which is the case. As I wrote to you already [0], NMI contexts are
> >> _never_ allowed to do things that rely on waiting forever for other
> >> CPUs.
> >
> > Who says _never_? I agree that it is not reasonable. But the
> > history shows that it happens.
> 
> Right, which is why it would need to become policy.
> 
> The emergency messages (aka write_atomic) introduce a new requirement to
> the kernel because this callback must be callable from any context. The
> console drivers must have some way of synchronizing. The CPU-reentrant
> spin lock is the only solution I am aware of.

I am not in position to decide if this requirement is acceptable
or not. We would need an opinion from Linus at minimum.

But it is not acceptable from my point of view. Note that
many spinlocks might be safely used in NMI in principle.

You want to introduce a single spinlock just because
printk() could be called anywhere. Most other subsystems
do not have this problem because they a more self-contained.

> If the ringbuffer was fully lockless

Exactly, I think that a solution would be fully lockless logbuffer.


> we should be able to have per-console CPU-reentrant spin locks
> as long as the ordering is preserved, which I expect shouldn't
> be a problem. If any NMI context needed a spin lock for its own
> purposes, it would need to use the CPU-reentrant spin lock of
> the first console so as to preserve the ordering in case of a panic.

IMHO, this is not acceptable. It would be nice to have direct output
from NMI but the cost looks to high here.

A solution would be to defer the console to irq_work or so.
We might also consider using trylock in NMI and have
the irq_work just as a fallback.

Best Regars,
Petr