From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755027AbcDDJlN (ORCPT ); Mon, 4 Apr 2016 05:41:13 -0400 Received: from mx2.suse.de ([195.135.220.15]:58998 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754730AbcDDJlL (ORCPT ); Mon, 4 Apr 2016 05:41:11 -0400 Date: Mon, 4 Apr 2016 11:41:08 +0200 From: Petr Mladek To: Jan Kara Cc: Sergey Senozhatsky , Andrew Morton , Jan Kara , Tejun Heo , Tetsuo Handa , linux-kernel@vger.kernel.org, Byungchul Park , Sergey Senozhatsky Subject: Re: [RFC][PATCH v8 1/2] printk: Make printk() completely async Message-ID: <20160404094108.GI1023@pathway.suse.cz> References: <1458834203-3392-1-git-send-email-sergey.senozhatsky@gmail.com> <1458834203-3392-2-git-send-email-sergey.senozhatsky@gmail.com> <20160331111229.GB1023@pathway.suse.cz> <20160331115250.GB6651@quack.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160331115250.GB6651@quack.suse.cz> 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 Thu 2016-03-31 13:52:50, Jan Kara wrote: > On Thu 31-03-16 13:12:29, Petr Mladek wrote: > > > #if defined CONFIG_PRINTK > > > +static int printk_kthread_func(void *data) > > > +{ > > > + while (1) { > > > + set_current_state(TASK_INTERRUPTIBLE); > > > + if (!need_flush_console) > > > + schedule(); > > > + > > > + __set_current_state(TASK_RUNNING); > > > > > > We still must do here: > > > > need_flush_console = false; > > > > Othrerwise, we might start "busy" cycling. cosole_unlock() > > sometimes returns earlly, e.g. when console_suspended is set > > or !can_use_console() returns true. > > > > Sigh, the handling of "need_flush_console" is a bit strange. > > Part of the logic depends on logbuf_lock and the other part > > must be lockless. > > Frankly, I think we are overcomplicating this. What we really need to detect > in printk_kthread_func() is whether someone appended something to the console > since we woken up. Sure, console_unlock() may have already printed that > and we would unnecessarily make one more loop over console_lock() and > console_unlock() but who cares... > > So what about having printk_kthread_func() like: > > while (1) { > set_current_state(TASK_INTERRUPTIBLE); > if (!need_flush_console) > schedule(); > __set_current_state(TASK_RUNNING); > need_flush_console = false; > console_lock(); > console_unlock(); > } > > In vprintk_emit() we do: > > if (!in_panic && printk_kthread) { > /* Offload printing to a schedulable context. */ > need_flush_console = true; > wake_up_process(printk_kthread); > } else { > ... > > This guarantees that after message was appended to the buffer in > vprintk_emit(), the message got either printed by console_unlock() or > printk_kthread is in TASK_RUNNING state and will call console_unlock() once > scheduled. It also guarantees that printk_kthread_func() won't loop forever > when there's nothing to print. And that is all we need... > > I think the simplicity of this is worth the possible extra loops in > printk_kthread_func(). I do not have strong opinion about this. I agree that the simplicity of your proposal is nice. You are much more experienced kernel developer. If you say that the potential extra loop is fine, I am fine with it as well :-) Best Regards, Petr