On (10/16/18 14:54), Peter Zijlstra wrote:
> On Tue, Oct 16, 2018 at 09:27:34PM +0900, Sergey Senozhatsky wrote:
> > per-CPU printk_safe _semi-magic_ makes some things simple to handle.
> > We can't just remove per-CPU buffers and add a wake_up_process() at
> > the bottom of vprintk_emit(). Because this will deadlock:
> >
> > printk()
> > wake_up_process()
> > try_to_wake_up()
> > raw_spin_lock_irqsave()
> > <NMI>
> > printk()
> > wake_up_process()
> > try_to_wake_up()
> > raw_spin_lock_irqsave()
> >
> > So we still need some amount of per-CPU printk() semi-magic anyway.
>
> All we need is 4 max-line-length buffers per-CPU. Nothing more.
OK, similar to what Steven did with cpu_buffer->current_context.
> The above trainwreck is the direct result of forcing synchronous
> printk'ing (which I'm otherwise a big fan of, but regular console
> drivers stink and are unfixable).
Yep.
> > And printk-kthread offloding will not eliminate the need of
> > printk_deferred().
>
> Why not? printk() will reduce to a lockless buffer insert. IOW _all_
> printk is deferred.
Aha! Interesting. I didn't realize you were talking about
"all printk()-s are deferred".
OK, jump to the last part of this mail.
> All you need are 4 max-line-length buffers per CPU and a global/shared
> lockless buffer.
>
> printk will determine the current context:
>
> task, softirq, hardirq or NMI
>
> and pick the corresponding per-cpu line buffer and do the vsnprintf()
> thing. Then we have the actual line length and content. With the length
> we reserve the bytes from the global buffer, we memcpy into the buffer
> and commit.
>
> Done.
>
> The printk thread will observe it lags behind the buffer head and will
> start printk-ing crud from task context.
[you can skip this part]
This probably will be a bit more hairy. logbuf is written to by many
sources and is read from by many sides, including user-space [both read()
and write()]. So we will need more flags/magic around memcpy(). A simple,
"grab the logbuf entry, set the proper offset to point to the next available
logbuf record and then do memcpy()" won't suffice. We need a flag for
"memcpy() complete, we can read this entry". Otherwise:
CPU0 CPU1 CPU2 CPU3
printk printk printk_kthread
logbuf_entry A logbuf_entry B syslog(read all) call_console_drivers
memcpy memcpy read unfinished print unfinished
A and B A and B
[..]
> > We do, however, have loads of problems with all those dependencies which
> > come from serial drivers and friends: timekeeping, scheduler (scheduler
> > is brilliant and cool, but we do have some deadlocks in printk because of
> > it ;), tty, net, MM, wq, etc. So I generally like the idea of "detached
> > serial consoles" (that's what I call it). IOW, elimination of the direct
> > printk -> serial console path.
>
> Right; we need to get rid of that in the generic case. Only allow
> lockless consoles (earlycon) to be used synchonously. With maybe a
> special case for the BUG/PANIC case to push things out ASAP.
[..]
> > So, unless I'm missing something, things are not entirely that simple:
> > - throw away printk_safe semi-magic
> > - add a lockless logbuf
> > - add wake_up_process() to vprintk_emit().
>
> No, no wakups. irq_work to wake the printk-thread, at most.
All right. OK. So we are on the same page here:
printk has internal locks - logbuf spin_lock; and external locks - all
the scheduler locks, console_sem, net, tty, wq, you name it. printk() is
not aware of those external locks; the only way to fix it is to remove
them from printk(). And that's why
"turn printk() into printk_deferred() and fix printk() deadlocks
in general case"
was my final proposal at the 2016 KS, NM, USA [1] (grep for printk_deferred).
I mentioned this idea several times since then, and even sent a patch, doing
this "printk is now printk_deferred unless we are in panic" thing. As far
as I remember, back then the idea/patch were rejected [2], and one of
reviewers even hinted that I was crazy :-) I have absolutely no issues
with that, but, considering past experiences, I'd really like to:
- Have more opinions on this. People please speak out.
- Have clear "let's do it" from Cc-ed people.
If we are really doing this, then let's split it and have
incremental changes. Namely, what I suggest is:
- keep internal printk lock - logbuf lock for now; we know how to
handle it. I promise.
- keep printk_safe for now, we need it to deal with logbuf lock
- keep printk_safe completely internal to printk
- add printk_kthread
- do printk()->irq_work()->wake_up_process(printk_kthread)
change and remove external locks dependency
- use direct printk() for panic() case
- do something about early_printk
That's big enough already.
>From there, once we land this thing, we can start building new logbuf,
stealing code from Steven, improving per-CPU buffers and so on.
Are we doing this?
[1] https://lwn.net/Articles/705938/
[2]
https://lore.kernel.org/lkml/20170202090722.gw6...@twins.programming.kicks-ass.net/
-ss
