From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935256AbcCQAd3 (ORCPT ); Wed, 16 Mar 2016 20:33:29 -0400 Received: from mail-pf0-f179.google.com ([209.85.192.179]:35760 "EHLO mail-pf0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932684AbcCQAd1 (ORCPT ); Wed, 16 Mar 2016 20:33:27 -0400 Date: Thu, 17 Mar 2016 09:34:50 +0900 From: Sergey Senozhatsky To: Byungchul Park Cc: Sergey Senozhatsky , 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: <20160317003450.GA538@swordfish> 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> <20160316103431.GM5220@X58A-UD3R> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160316103431.GM5220@X58A-UD3R> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (03/16/16 19:34), Byungchul Park wrote: [..] > > -- 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? yes, this is how it is. "BUG: recent printk recursion!" will be printed instead of the message. > 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. the alternative is NOT significantly better. pending bit is checked in IRQ, so one simply can do local_irq_save(); while (xxx) printk(); local_irq_restore(); and _in the worst case_ nothing will be printed to console until IRQ are enabled on this CPU. (there are some 'if's, but the worst case is just like this. http://marc.info/?l=linux-kernel&m=145734549308803). I'd probably prefer to add wake_up_process() to vprintk_emit() and do it under the logbuf lock. first, we don't suffer from disabled IRQs on current CPU, second we have somewhat better chances to break printk() recursion *in some cases*. > > -- 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. ah, we lost in patches. I was talking about yet another patch (you probably not aware of. you were not Cc'd. Sorry!) that makes console_unlock() asynchronous: http://marc.info/?l=linux-kernel&m=145750373530161 s/wake_up/wake_up_process/ is at the end of console_unlock(). while the patch belongs to another series, I still wanted to outline it here, since we were talking about printk() recursion. -ss