From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Joe Perches <joe@perches.com>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Petr Mladek <pmladek@suse.cz>, Tejun Heo <tj@kernel.org>,
Calvin Owens <calvinowens@fb.com>,
Steven Rostedt <rostedt@goodmis.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: linux.git: printk() problem
Date: Tue, 25 Oct 2016 13:06:17 +0900 [thread overview]
Message-ID: <20161025040617.GA565@swordfish> (raw)
In-Reply-To: <CA+55aFwKYnrMJr_vSE+GfDGszeUGyd=CPUD15-zZ8yWQW61GBA@mail.gmail.com>
On (10/24/16 19:22), Linus Torvalds wrote:
> On Mon, Oct 24, 2016 at 7:06 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Mon, Oct 24, 2016 at 6:55 PM, Sergey Senozhatsky
> > <sergey.senozhatsky.work@gmail.com> 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
next prev parent reply other threads:[~2016-10-25 4:06 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-12 13:30 linux.git: printk() problem Tetsuo Handa
2016-10-12 14:35 ` Michal Hocko
2016-10-12 16:08 ` Joe Perches
2016-10-13 6:26 ` Michal Hocko
2016-10-13 9:29 ` Joe Perches
2016-10-13 10:04 ` Michal Hocko
2016-10-13 10:20 ` Joe Perches
2016-10-13 11:06 ` Michal Hocko
2016-10-12 15:47 ` Linus Torvalds
2016-10-12 16:16 ` Joe Perches
2016-10-12 16:54 ` Linus Torvalds
2016-10-12 18:50 ` [PATCH] acpi_os_vprintf: Use printk_get_level() to avoid unnecessary KERN_CONT Joe Perches
2016-10-13 21:59 ` Rafael J. Wysocki
2016-10-23 9:22 ` linux.git: printk() problem Geert Uytterhoeven
2016-10-23 18:11 ` Linus Torvalds
2016-10-23 19:06 ` Joe Perches
2016-10-23 19:32 ` Linus Torvalds
2016-10-23 19:46 ` Linus Torvalds
2016-10-24 11:15 ` Geert Uytterhoeven
2016-10-24 14:08 ` Sergey Senozhatsky
2016-10-24 14:23 ` Sergey Senozhatsky
2016-10-24 17:54 ` Linus Torvalds
2016-10-24 17:55 ` Linus Torvalds
2016-10-25 1:55 ` Sergey Senozhatsky
2016-10-25 2:06 ` Linus Torvalds
2016-10-25 2:22 ` Linus Torvalds
2016-10-25 4:06 ` Sergey Senozhatsky [this message]
2016-10-25 4:13 ` Joe Perches
2016-10-25 4:15 ` Linus Torvalds
2016-10-25 4:44 ` Sergey Senozhatsky
2016-10-25 14:44 ` Petr Mladek
2016-11-09 15:47 ` Petr Mladek
2016-10-25 2:24 ` Sergey Senozhatsky
2016-10-23 20:33 ` Joe Perches
2016-10-23 21:13 ` Linus Torvalds
2016-10-25 14:42 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161025040617.GA565@swordfish \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=calvinowens@fb.com \
--cc=geert@linux-m68k.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=pmladek@suse.cz \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky@gmail.com \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).