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 b61312d353 ("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 e3e8a75d2a ("[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 c7c3f05e34 ("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>
This commit is contained in:
Daniel Vetter 2022-08-30 16:50:04 +02:00 committed by Greg Kroah-Hartman
parent bf98ef69f7
commit 0a64ce6e54
4 changed files with 2 additions and 8 deletions

View File

@ -154,6 +154,7 @@ static void console_callback(struct work_struct *ignored);
static void con_driver_unregister_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 blank_screen_t(struct timer_list *unused);
static void set_palette(struct vc_data *vc); static void set_palette(struct vc_data *vc);
static void unblank_screen(void);
#define vt_get_kmsg_redirect() vt_kmsg_redirect(-1) #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 * 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... * 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); do_unblank_screen(0);
} }

View File

@ -30,7 +30,6 @@ struct vc_data *vc_deallocate(unsigned int console);
void reset_palette(struct vc_data *vc); void reset_palette(struct vc_data *vc);
void do_blank_screen(int entering_gfx); void do_blank_screen(int entering_gfx);
void do_unblank_screen(int leaving_gfx); void do_unblank_screen(int leaving_gfx);
void unblank_screen(void);
void poke_blanked_console(void); void poke_blanked_console(void);
int con_font_op(struct vc_data *vc, struct console_font_op *op); int con_font_op(struct vc_data *vc, struct console_font_op *op);
int con_set_cmap(unsigned char __user *cmap); int con_set_cmap(unsigned char __user *cmap);

View File

@ -329,9 +329,6 @@ void panic(const char *fmt, ...)
if (_crash_kexec_post_notifiers) if (_crash_kexec_post_notifiers)
__crash_kexec(NULL); __crash_kexec(NULL);
#ifdef CONFIG_VT
unblank_screen();
#endif
console_unblank(); console_unblank();
/* /*

View File

@ -22,9 +22,6 @@ void bust_spinlocks(int yes)
if (yes) { if (yes) {
++oops_in_progress; ++oops_in_progress;
} else { } else {
#ifdef CONFIG_VT
unblank_screen();
#endif
console_unblank(); console_unblank();
if (--oops_in_progress == 0) if (--oops_in_progress == 0)
wake_up_klogd(); wake_up_klogd();