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. > > It uses console_atomic_lock() added in 18th patch. That one uses > prb_lock() added by 2nd patch. > > Now, prb_lock() allows recursion on the same CPU. But it still needs > to wait until it is released on another CPU. > > [...] > > 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. I could not find any instances where that is the case. nmi_cpu_backtrace() used to do this, but it does not anymore. > But printk() should not make such limitation > to the rest of the system. That is something we have to decide. It is the one factor that makes prb_lock() feel a hell of a lot like BKL. > Not to say, that we would most > likely need to add a lock back into nmi_cpu_backtrace() > to keep the output sane. No. That is why CPU-IDs were added to the output. It is quite sane and easy to read. > Peter Zijlstra several times talked about fully lockless > consoles. He is using the early console for debugging, see > the patchset > https://lkml.kernel.org/r/20170928121823.430053219@xxxxxxxxxxxxx That is an interesting thread to quote. In that thread Peter actually wrote the exact implementation of prb_lock() as the method to synchronize access to the serial console. > I am not sure if it is always possible. I personally see > the following way: > > 1. Make the printk ring buffer fully lockless. Then we reduce > the problem only to console locking. And we could > have a per-console-driver lock (no the big lock like > prb_lock()). A fully lockless ring buffer is an option. But as you said, it only reduces the window, which is why I decided it is not so important (at least for now). Creating a per-console-driver lock would probably be a good idea anyway as long as we can guarantee the ordering (which shouldn't be a problem as long as emergency console ordering remains fixed and emergency writers always follow that ordering). > 2. I am afraid that we need to add some locking between CPUs > to avoid mixing characters from directly printed messages. That is exactly what console_atomic_lock() (actually prb_lock) is! John Ogness [0] https://lkml.kernel.org/r/87pnrvs707.fsf@xxxxxxxxxxxxx