From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757646AbcHWNEO (ORCPT ); Tue, 23 Aug 2016 09:04:14 -0400 Received: from mx2.suse.de ([195.135.220.15]:57802 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753210AbcHWNEM (ORCPT ); Tue, 23 Aug 2016 09:04:12 -0400 Date: Tue, 23 Aug 2016 15:03:08 +0200 From: Petr Mladek To: Sergey Senozhatsky Cc: Jan Kara , Sergey Senozhatsky , Viresh Kumar , Andrew Morton , Jan Kara , Tejun Heo , Tetsuo Handa , "linux-kernel@vger.kernel.org" , Byungchul Park , vlevenetz@mm-sol.com, Greg Kroah-Hartman Subject: Re: [PATCH v10 1/2] printk: Make printk() completely async Message-ID: <20160823130308.GE4866@pathway.suse.cz> References: <20160812094447.GD7339@pathway.suse.cz> <20160818022712.GB500@swordfish> <20160818093329.GL13300@pathway.suse.cz> <20160818095144.GA425@swordfish> <20160818105629.GE26194@pathway.suse.cz> <20160819063236.GA584@swordfish> <20160819095455.GR13300@pathway.suse.cz> <20160819190007.GA8275@quack2.suse.cz> <20160820052430.GA695@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160820052430.GA695@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 Sat 2016-08-20 14:24:30, Sergey Senozhatsky wrote: > On (08/19/16 21:00), Jan Kara wrote: > > > > depending on .config BUG() may never return back -- passing control > > > > to do_exit(), so printk_deferred_exit() won't be executed. thus we > > > > probably need to have a per-cpu variable that would indicate that > > > > we are in deferred_bug. hm... but do we really need deferred BUG() > > > > in the first place? > > > > > > Good question. I am not aware of any BUG_ON() that would be called from > > > wake_up_process() but it is hard to check everything. > > > > > > A conservative approach would be to force synchronous printk from > > > BUG_ON(). > > > > Just a quick thought: Cannot we just do printk_deferred_enter() when we are > > about to call into the scheduler from printk code and printk_deferred_exit() > > when leaving it? That would look like the least error-prone way how > > handling this kind of recursion... > > interesting idea. > printk_deferred_enter() increments preempt count, so there may be additional > obstacles and, as a result, ad-hocs, that scheduler people will sincerely hate. > need to think more. I wonder if this would be acceptable at least for wake_up_process(). It seems to be the only scheduler function that we are interested in. And we might call it from vprintk > > OTOH there's also the other possible direction for the recursion when we > > are in the scheduler, holding some scheduler locks, decide to WARN which > > enters printk, that ends up calling wake_up_process() which deadlocks > > on scheduler locks... I don't see how to handle this type of recursion > > inside the printk code itself easily and so far the answer was - use > > printk_deferred() in the scheduler and don't use WARN... > > the recursion detection is really tricky, yes. it seems (and I haven't > thought of it good enough) to be a bit simpler when we operate in async > printk mode, because we remove this uncontrollable console_unlock(). > so we can do something like this: > > vprintk_emit(....) > { > local_irq_save(); > > if (this_cpu_read(in_printk)) { > log_store(BUG: printk recursion!"); > goto out; > } This does not quarantee that we have the logbug_lock. We might endup here from the raw_spin_lock() call and the lock might be owned by another CPU. I am afraid that we could only set some global variable here. > > this_cpu_write(in_printk) = 1; > > raw_spin_lock(&logbuf_lock); > log_store(); > raw_spin_unlock(&logbuf_lock); > > if (!in_sched) { > if (console_loglevel != CONSOLE_LOGLEVEL_MOTORMOUTH && > can_printk_async()) { > printk_kthread_need_flush_console = true; > wake_up_process(printk_kthread); > } > } > > this_cpu_write(in_printk) = 0; > out: > local_irq_restore(); > } > > async printk mode from this point of view is sort of atomic. This would prevent using printk_deferred() from the scheduler code. A solution would be to set the per-CPU variable only around the wake_up_process() call. Well, it is orthogonal to using printk_deferred_enter() around calling wake_up_process(). Best Regards, Petr