From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966502AbcCPKen (ORCPT ); Wed, 16 Mar 2016 06:34:43 -0400 Received: from LGEAMRELO11.lge.com ([156.147.23.51]:44215 "EHLO lgeamrelo11.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966339AbcCPKem (ORCPT ); Wed, 16 Mar 2016 06:34:42 -0400 X-Original-SENDERIP: 156.147.1.125 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 10.177.222.33 X-Original-MAILFROM: byungchul.park@lge.com Date: Wed, 16 Mar 2016 19:34:31 +0900 From: Byungchul Park To: Sergey Senozhatsky Cc: Sergey Senozhatsky , Jan Kara , Andrew Morton , Jan Kara , Petr Mladek , Tejun Heo , Tetsuo Handa , linux-kernel@vger.kernel.org Subject: Re: [RFC][PATCH v4 1/2] printk: Make printk() completely async Message-ID: <20160316103431.GM5220@X58A-UD3R> References: <1457964820-4642-1-git-send-email-sergey.senozhatsky@gmail.com> <1457964820-4642-2-git-send-email-sergey.senozhatsky@gmail.com> <20160315100323.GF17942@quack.suse.cz> <20160315140738.GA773@swordfish> <20160316053944.GJ5220@X58A-UD3R> <20160316065851.GC3217@swordfish> <20160316073007.GL5220@X58A-UD3R> <20160316075605.GE3217@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160316075605.GE3217@swordfish> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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