From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753650AbcHRC1R (ORCPT ); Wed, 17 Aug 2016 22:27:17 -0400 Received: from mail-pa0-f66.google.com ([209.85.220.66]:34788 "EHLO mail-pa0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753474AbcHRC1P (ORCPT ); Wed, 17 Aug 2016 22:27:15 -0400 Date: Thu, 18 Aug 2016 11:27:12 +0900 From: Sergey Senozhatsky To: Petr Mladek Cc: Viresh Kumar , Jan Kara , Andrew Morton , Sergey Senozhatsky , Jan Kara , Tejun Heo , Tetsuo Handa , "linux-kernel@vger.kernel.org" , Byungchul Park , Sergey Senozhatsky , vlevenetz@mm-sol.com, Greg Kroah-Hartman Subject: Re: [PATCH v10 1/2] printk: Make printk() completely async Message-ID: <20160818022712.GB500@swordfish> References: <1459789048-1337-1-git-send-email-sergey.senozhatsky@gmail.com> <1459789048-1337-2-git-send-email-sergey.senozhatsky@gmail.com> <20160404155149.a3e3307def2d1315e2099c63@linux-foundation.org> <20160406082758.GA3554@quack.suse.cz> <20160812094447.GD7339@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160812094447.GD7339@pathway.suse.cz> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, really sorry for very long reply. On (08/12/16 11:44), Petr Mladek wrote: [..] > IMHO, this is fine. We force the synchronous mode in critical > situations anyway. yes, I think it makes sense to lower the priority (we also have briefly discussed this in private emails with Viresh). I'd still prefer to have forced sync-printk on suspend/hibernate/etc., though. > But I was curious if we could hit a printk from the wake_up_process(). > The change above causes using the fair scheduler and there is > the following call chain [*] > > vprintk_emit() > -> wake_up_process() > -> try_to_wake_up() > -> ttwu_queue() > -> ttwu_do_activate() > -> ttwu_activate() > -> activate_task() > -> enqueue_task() > -> enqueue_task_fair() via p->sched_class->enqueue_task > -> cfs_rq_of() > -> task_of() > -> WARN_ON_ONCE(!entity_is_task(se)) > > We should never trigger this because printk_kthread is a task. > But what if the date gets inconsistent? > > Then there is the following chain: > > vprintk_emit() > -> wake_up_process() > -> try_to_wake_up() > -> ttwu_queue() > -> ttwu_do_activate() > -> ttwu_activate() > -> activate_task() > -> enqueue_task() > -> enqueue_task_fair() via p->sched_class->enqueue_task > ->hrtick_update() > -> hrtick_start_fair() > -> WARN_ON(task_rq(p) != rq) > > This looks like another paranoid consistency check that might be > triggered when the scheduler gets messed. > > I see few possible solutions: > > 1. Replace the WARN_ONs by printk_deferred(). > > This is the usual solution but it would make debugging less convenient. what I did internally was a combination of #1 and #3: I introduced a dump_stack_deferred() function which is basically (almost) a copy-past of dump_stack() from lib/dump_stack.c with the difference that it calls printk_deferred(). and added a WARN_ON_DEFERRED() macro. > 2. Force synchronous printk inside WARN()/BUG() macros. will it help? semaphore up() calls wake_up_process() regardless the context. not to mention that we still may have spin_dump() enabled. > 3. Force printk_deferred() inside WARN()/BUG() macros via the per-CPU > printk_func. > > It might be elegant. But we do not want this outside the scheduler > code. Therefore we would need special variants of WARN_*_SCHED() > BUG_*_SCHED() macros. > > I personally prefer the 2nd solution. What do you think about it, > please? I personally think a combo of #1 and #3 is a bit better than plain #2. -ss