Messages in this thread | | | Date | Wed, 16 Mar 2016 19:34:31 +0900 | From | Byungchul Park <> | Subject | Re: [RFC][PATCH v4 1/2] printk: Make printk() completely async |
| |
On Wed, Mar 16, 2016 at 04:56:05PM +0900, Sergey Senozhatsky wrote: > On (03/16/16 16:30), Byungchul Park wrote: > > > > Do you mean the wake_up_process() in console_unlock? > > no, I meant wake_up_process(printk_kthread), the newly added one.
I got it. You are talking about wake_up_process() in Petr's patch.
> > > -- if we are going to have wake_up_process() in wake_up_klogd_work_func(), > then we need `in_sched' message to potentially trigger a recursion chain > > wake_up_klogd_work_func()->wake_up_process()->printk()->wake_up_process()->printk()... > > to break this printk()->wake_up_process()->printk(), we need wake_up_process() to > be under the logbuf lock; so vprintk_emit()'s if (logbuf_cpu == this_cpu) will act.
I am curious about how you make the wake_up_process() call and I may want to talk about it at the next spin. Anyway, then we will lose the last message when "if (logbuf_cpu == this_cpu)" acts. Is it acceptible?
IMHO it's not a good choice to use wake_up() and friend within a printk() since it can additionally cause another recursion. Of course, it does not happen if the condition (logbuf_cpu == this_cpu) acts. But I don't think it's good to rely on the condition with losing a message. Anyway I really really want to see your next spin and talk.
> > > -- if we are going to have wake_up_process() in console_unlock(), then > > console_unlock()->{up(), wake_up_process()}->printk()->{console_lock(), console_unlock()}->{up(), wake_up_process()}->printk()... >
This cannot happen. console_lock() cannot continue because the prior console_unlock() does not release console_sem.lock yet when wake_up_process() is called. Only a deadlock exists. And my patch solves the problem so that the deadlock cannot happen.
My patch is https://lkml.org/lkml/2016/3/11/192 as you already know.
> is undetectable... by the time console_unlock() calls wake_up_process() there > are no printk() locks that this CPU owns. > > > > I said they should be kept *out of* the critical section. :-) > > Otherwise, it can recurse us forever. > > can you explain?
Sorry. I was confused. I was wrong.
> > -ss
| |