Messages in this thread Patch in this message |  | | Date | Tue, 25 Oct 2016 13:06:17 +0900 | From | Sergey Senozhatsky <> | Subject | Re: linux.git: printk() problem |
| |
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
|  |