summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Vetter <daniel.vetter@ffwll.ch>2022-08-30 16:50:04 +0200
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2022-09-01 16:55:35 +0200
commit0a64ce6e5442bbd96cbe9057d9ba1edab244f25b (patch)
tree91aa33a30127b04cec009504489066fe153d41bf
parentbf98ef69f75ddce0046c04d351b07f44e9b2be53 (diff)
kernel/panic: Drop unblank_screen call
console_unblank() does this too (called in both places right after), and with a lot more confidence inspiring approach to locking. Reconstructing this story is very strange: In b61312d353da ("oops handling: ensure that any oops is flushed to the mtdoops console") it is claimed that a printk(" "); flushed out the console buffer, which was removed in e3e8a75d2acf ("[PATCH] Extract and use wake_up_klogd()"). In todays kernels this is done way earlier in console_flush_on_panic with some really nasty tricks. I didn't bother to fully reconstruct this all, least because the call to bust_spinlock(0); gets moved every few years, depending upon how the wind blows (or well, who screamed loudest about the various issue each call site caused). Before that commit the only calls to console_unblank() where in s390 arch code. The other side here is the console->unblank callback, which was introduced in 2.1.31 for the vt driver. Which predates the console_unblank() function by a lot, which was added (without users) in 2.4.14.3. So pretty much impossible to guess at any motivation here. Also afaict the vt driver is the only (and always was the only) console driver implementing the unblank callback, so no idea why a call to console_unblank() was added for the mtdooops driver - the action actually flushing out the console buffers is done from console_unlock() only. Note that as prep for the s390 users the locking was adjusted in 2.5.22 (I couldn't figure out how to properly reference the BK commit from the historical git trees) from a normal semaphore to a trylock. Note that a copy of the direct unblank_screen() call was added to panic() in c7c3f05e341a ("panic: avoid deadlocks in re-entrant console drivers"), which partially inlined the bust_spinlocks(0); call. Long story short, I have no idea why the direct call to unblank_screen survived for so long (the infrastructure to do it properly existed for years), nor why it wasn't removed when the console_unblank() call was finally added. But it makes a ton more sense to finally do that than not - it's just better encapsulation to go through the console functions instead of doing a direct call, so let's dare. Plus it really does not make much sense to call the only unblank implementation there is twice, once without, and once with appropriate locking. Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jirislaby@kernel.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Cc: Xuezhi Zhang <zhangxuezhi1@coolpad.com> Cc: Yangxi Xiang <xyangxi5@gmail.com> Cc: nick black <dankamongmen@gmail.com> Cc: Petr Mladek <pmladek@suse.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Luis Chamberlain <mcgrof@kernel.org> Cc: "Guilherme G. Piccoli" <gpiccoli@igalia.com> Cc: Marco Elver <elver@google.com> Cc: John Ogness <john.ogness@linutronix.de> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Cc: David Gow <davidgow@google.com> Cc: tangmeng <tangmeng@uniontech.com> Cc: Tiezhu Yang <yangtiezhu@loongson.cn> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Petr Mladek <pmladek@suse.com> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Link: https://lore.kernel.org/r/20220830145004.430545-1-daniel.vetter@ffwll.ch Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/tty/vt/vt.c3
-rw-r--r--include/linux/vt_kern.h1
-rw-r--r--kernel/panic.c3
-rw-r--r--lib/bust_spinlocks.c3
4 files changed, 2 insertions, 8 deletions
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 4d29e4a17db7..2cb515dbda53 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -154,6 +154,7 @@ static void console_callback(struct work_struct *ignored);
static void con_driver_unregister_callback(struct work_struct *ignored);
static void blank_screen_t(struct timer_list *unused);
static void set_palette(struct vc_data *vc);
+static void unblank_screen(void);
#define vt_get_kmsg_redirect() vt_kmsg_redirect(-1)
@@ -4448,7 +4449,7 @@ EXPORT_SYMBOL(do_unblank_screen);
* call it with 1 as an argument and so force a mode restore... that may kill
* X or at least garbage the screen but would also make the Oops visible...
*/
-void unblank_screen(void)
+static void unblank_screen(void)
{
do_unblank_screen(0);
}
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index b5ab452fca5b..c1f5aebef170 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -30,7 +30,6 @@ struct vc_data *vc_deallocate(unsigned int console);
void reset_palette(struct vc_data *vc);
void do_blank_screen(int entering_gfx);
void do_unblank_screen(int leaving_gfx);
-void unblank_screen(void);
void poke_blanked_console(void);
int con_font_op(struct vc_data *vc, struct console_font_op *op);
int con_set_cmap(unsigned char __user *cmap);
diff --git a/kernel/panic.c b/kernel/panic.c
index c6eb8f8db0c0..da323209f583 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -329,9 +329,6 @@ void panic(const char *fmt, ...)
if (_crash_kexec_post_notifiers)
__crash_kexec(NULL);
-#ifdef CONFIG_VT
- unblank_screen();
-#endif
console_unblank();
/*
diff --git a/lib/bust_spinlocks.c b/lib/bust_spinlocks.c
index 8be59f84eaea..bfd53972a4d8 100644
--- a/lib/bust_spinlocks.c
+++ b/lib/bust_spinlocks.c
@@ -22,9 +22,6 @@ void bust_spinlocks(int yes)
if (yes) {
++oops_in_progress;
} else {
-#ifdef CONFIG_VT
- unblank_screen();
-#endif
console_unblank();
if (--oops_in_progress == 0)
wake_up_klogd();