From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752158AbcIBH6B (ORCPT ); Fri, 2 Sep 2016 03:58:01 -0400 Received: from mail-pa0-f66.google.com ([209.85.220.66]:35739 "EHLO mail-pa0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751082AbcIBH57 (ORCPT ); Fri, 2 Sep 2016 03:57:59 -0400 Date: Fri, 2 Sep 2016 16:58:08 +0900 From: Sergey Senozhatsky To: Petr Mladek Cc: Sergey Senozhatsky , Sergey Senozhatsky , Jan Kara , 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: <20160902075808.GA26230@swordfish> References: <20160819190007.GA8275@quack2.suse.cz> <20160820052430.GA695@swordfish> <20160822041520.GA511@swordfish> <20160825210959.GA2273@dhcp128.suse.cz> <20160826015641.GA520@swordfish> <20160830092912.GP4866@pathway.suse.cz> <20160831023134.GA390@swordfish> <20160831093810.GE4554@pathway.suse.cz> <20160831125224.GA572@swordfish> <20160901085844.GW4866@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160901085844.GW4866@pathway.suse.cz> User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (09/01/16 10:58), Petr Mladek wrote: > On Wed 2016-08-31 21:52:24, Sergey Senozhatsky wrote: > > On (08/31/16 11:38), Petr Mladek wrote: > > > 2. Potential deadlocks when calling wake_up_process() by > > > async printk and console_unlock(). > > > > * there are many reasons to those recursive printk() calls -- some > > can be addressed, some cannot. for instance, it doesn't matter how many > > per-CPU buffers we use for alternative printk() once the logbuf_lock is > > corrupted. > > Yup and BTW: Peter Zijlstra wants to avoid zapping locks whenever > possible because it corrupts the state. It might solve the actual > state but it might cause deadlock by the double unlock. yes, don't really want to zap_locks() either. [..] > Great catch! From the already mentioned solutions, I would prefer > using deferred variants of WARN()/BUG()/printk() on these locations. > Together with using lockdep to find these locations. hmm... need to think more. one of the problems is that we would have to periodically "scan" for new WARNs/BUGs/etc doing all the types of random .configs > Also there is the Peter Zijlstra's idea of using a lockless > "early" console to debug the situations where it happens. > It might make sense to make such a console easy to use. aha, not really familiar with early console. > I am unable to find any other generic solution that would prevent this > from the printk() side at the moment. > > > 5. not 100% guaranteed printing on panic [..] > That might be very hard to solve in general as well. Again the PeterZ's > idea with the lockless console might help here. "need to google it". > > > I wonder how to separate the problems and make them more manageable. > > > > so I was thinking for a moment about doing the recursion detection rework > > before the async_printk. just because better recursion detection is a nice > > thing to have in the first place and it probably may help us catching some > > of the surprises that async_printk might have. but it probably will be more > > problematic than I thought. > > > > then async_printk. I have a refreshed series on my hands, addressing > > Viresh's reports. it certainly makes things better, but it doesn't > > eliminate all of the lockups/etc sources. > > We must separate historical possible lockups and new regressions. > Only regressions should block the async printk series. Old > bugs should be fixed separately to keep the series manageable. agree. > Anyway, I think that the async printk will make sense even > when we solve all the other issues. If async printk does not > cause regressions, why not make it in. sure. > > a console_unlock() doing > > wake_up_process(printk_kthread) would make it better. > > I am not sure what you mean by this. I meant that this thing local_irq_save() // or preempt_disable() ... if (console_trylock()) console_unlock(); ... local_irq_restore() // or preempt_enable() can easily lockup the system if console_trylock() was successful and there are enough messages to print. printk_kthread can't help, because here we basically enforce the `old' behavior. we have async printk, but not async console output. tweaking console_unlock() to offload the actual printing loop to printk_kthread would make the entire console output async: static void console_sync_flush_and_unlock(void) { for (;;) { ... call_console_drivers(); ... } } void console_unlock(void) { if (!MOTORMOUTH && can_printk_async()) { up(); wake_up_process(printk_kthread); return; } console_sync_flush_and_unlock(); } -ss