From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752833AbcJYEGZ (ORCPT ); Tue, 25 Oct 2016 00:06:25 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:36190 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750763AbcJYEGX (ORCPT ); Tue, 25 Oct 2016 00:06:23 -0400 Date: Tue, 25 Oct 2016 13:06:17 +0900 From: Sergey Senozhatsky To: Linus Torvalds Cc: Sergey Senozhatsky , Sergey Senozhatsky , Joe Perches , Geert Uytterhoeven , Tetsuo Handa , Linux Kernel Mailing List , Petr Mladek , Tejun Heo , Calvin Owens , Steven Rostedt , Andrew Morton Subject: Re: linux.git: printk() problem Message-ID: <20161025040617.GA565@swordfish> References: <1477249607.3561.2.camel@perches.com> <20161024140845.GA626@swordfish> <20161025015554.GA495@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (10/24/16 19:22), Linus Torvalds wrote: > On Mon, Oct 24, 2016 at 7:06 PM, Linus Torvalds > wrote: > > On Mon, Oct 24, 2016 at 6:55 PM, Sergey Senozhatsky > > wrote: > >> > >> I think cont_flush() should grab the logbuf_lock lock, because > >> it does log_store() and touches the cont.len. so something like > >> this perhaps > > > > Absolutely. Good catch. > > Actually, you can't do it the way you did (inside cont_flush), because > "cont_flush()" is already called with logbuf_lock held in most cases > (see "cont_add()"). right. my bad, realized too late. > So it's really just the timer function that needs to take the > logbuf_lock before it calls cont_flush(). yes. > So here's a new version. How does this look to you? ok, looks much better. there are several things that I want to mention here, just to make sure we don't miss anything (just my 5 cents). 1) the way we dumpstack on x86 (at least on x86) is a spaghetti of printk() and pr_cont() calls. for instance, arch/x86/kernel/dumpstack_64.c show_regs() does pr_cont() to print out the registers, while the stack and backtrace are printed with printk(). so, I assume, the backtrace now will look a bit upside-down, because cont lines are printed with the delay. correct? 2) flush on oops. not that panic printk is deadlock proof (not at all) but: a) rather unlikely, but what if BUG_ON() or panic() happens under lock_timer_base()? b) what if timer event never happens? (we are in panic, who knows) so how about skipping mod_timer in deferred_cont_flush() and just cont_flush() when we are in oops? here is probably one more thing we need to "fix" first. oops_in_progress is unreliable. x86 oops_end() does bust_spinlocks(0) before it calls panic(). panic() increments oops_in_progress but decrements it back to 0 (bust_spinlocks(0)) before it does console_flush_on_panic(). so there is (almost) no way console_flush_on_panic() can see oops_in_progress != 0. diff --git a/kernel/panic.c b/kernel/panic.c index e6480e2..8e17540 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -228,7 +228,6 @@ void panic(const char *fmt, ...) if (_crash_kexec_post_notifiers) __crash_kexec(NULL); - bust_spinlocks(0); /* * We may have ended up stopping the CPU holding the lock (in @@ -240,6 +239,7 @@ void panic(const char *fmt, ...) */ debug_locks_off(); console_flush_on_panic(); + bust_spinlocks(0); if (!panic_blink) panic_blink = no_blink; --- -ss