On Sat 2016-10-01 12:02:51, Sergey Senozhatsky wrote: > On (09/30/16 13:27), Petr Mladek wrote: > > > > > This patch set extends a lock-less NMI per-cpu buffers idea to > > > > > handle recursive printk() calls. The basic mechanism is pretty much > > > > > the > > > > > same -- at the beginning of a deadlock-prone section we switch to > > > > > lock-less > > > > > printk callback, and return back to a default printk implementation > > > > > at the > > > > > end; the messages are getting flushed to a logbuf buffer from a safer > > > > > context. > > > > > > > > I was skeptical but I really like this way now. > > > > > > > > The switching of the buffers is a bit hairy in this version but I > > > > think that we could make it much better. > > > > > > > > Other than that it looks like a big win. It kills a lot of > > > > printk-related pain points. And it will not be that complicated > > > > after all. > > > > > > many thanks for looking at this train wreck. > > > > > > so, like I said, it addresses printk()-recursion in *ideally* quite > > > a minimalistic way -- just several alt_printk_enter/exit calls in > > > printk.c, without ever touching any other parts of the kernel. > > > > > > gunning down printk deadlocks in general, however, requires much more > > > effort; or even a completely different approach. > > > > > > a) a lock-less printk() by default > > > um, `#define printk alt_printk'. but this will break printk() from irq. > > > and the ordering of messages from per-cpu buffers may be far from > > > correct. > > > > Well, the current vprintk_nmi() is lockless. The alternative printk() > > is going to use the same code, so it will be lockless as well. It > > means that even this patchset is supposed to avoid all possible > > deadlocks via printk() calls. > > I meant that printk-recursion and printk-deadlock can be different > scenarios. deadlocks are harder to handle > > devkmsg_open() > raw_spin_lock_irq(&logbuf_lock) > spin_dump() > printk() > raw_spin_lock_irqsave(&logbuf_lock) > > this one can be handled by alt_printk. > > devkmsg_open() > local_irq_save(); > alt_printk_enter() > raw_spin_lock(&logbuf_lock) > spin_dump() > printk() > vprintk_alt() > > but there are some that can't be handled solely in printk.c
Do you have an example of the still problematic code, please? vprintk_alt() must be lockless because the same code is used also in NMI context. If it takes a lock, it is a bug. Therefore it should not cause a deadlock. The only problem might be an infinite loop. But the loop should break once the per-CPU buffer is full. We only need to make sure that there is no printk() called before the check for the full buffer. But this reduces the error-prone part of the code to a minimum. Therefore it should be bearable. Best Regards, Petr
