Hi Daniel, On Tue, Aug 30, 2022 at 04:49:45PM +0200, Daniel Vetter wrote: > I'm scratching my head why we have this printing_lock. Digging through > historical git trees shows that: > - Added in 1.1.73, and I found absolutely no reason why. > - Converted to atomic bitops in 2.1.125pre2, I guess as part of SMP > enabling/bugfixes. > - Converted to a proper spinlock in b0940003f25d ("vt: bitlock fix") > because the hand-rolled atomic version lacked necessary memory > barriers. > > Digging around in lore for that time period did also not shed further > light. > > The only reason I think this might still be relevant today is that (to > my understanding at least, ymmv) during an oops we might be printing > without console_lock held. See console_flush_on_panic() and the > comments in there - we flush out the console buffers irrespective of > whether we managed to acquire the right locks. > > The strange thing is that this reason is fairly recent, because the > console flushing was historically done without oops_in_progress set. > This only changed in c7c3f05e341a ("panic: avoid deadlocks in > re-entrant console drivers"), which removed the call to > bust_spinlocks(0) (which decrements oops_in_progress again) before > flushing out the console (which back then was open coded as a > console_trylock/unlock pair). > > Note that this entire mess should be properly fixed in the > printk/console layer, and not inflicted on each implementation. > > For now just document what's going on and check that in all other > cases callers obey the locking rules. > > v2: WARN_CONSOLE_UNLOCKED already checks for oops_in_progress > (something else that should be fixed I guess), hence remove the > open-coded check I've had. > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Jiri Slaby <jirislaby@xxxxxxxxxx> > Cc: "Ilpo Järvinen" <ilpo.jarvinen@xxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Cc: Xuezhi Zhang <zhangxuezhi1@xxxxxxxxxxx> > Cc: Yangxi Xiang <xyangxi5@xxxxxxxxx> > Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> > Cc: nick black <dankamongmen@xxxxxxxxx> > Cc: Petr Mladek <pmladek@xxxxxxxx> > Cc: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> > Cc: John Ogness <john.ogness@xxxxxxxxxxxxx> > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> It is always good to warn in case assumptions do not hold. And thanks for the comment. Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx> Hmm, I prefer to start comments with upper-case, but in vt.c there is no specific style. Sam > -- > Note that this applies on top of my earlier vt patch: > > https://lore.kernel.org/lkml/20220826202419.198535-1-daniel.vetter@xxxxxxxx/ > > Expect more, I'm digging around in here a bit ... > -Daniel > --- > drivers/tty/vt/vt.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index 4d29e4a17db7..a6be32798fad 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -3083,7 +3083,9 @@ static void vt_console_print(struct console *co, const char *b, unsigned count) > ushort start_x, cnt; > int kmsg_console; > > - /* console busy or not yet initialized */ > + WARN_CONSOLE_UNLOCKED(); > + > + /* this protects against concurrent oops only */ > if (!spin_trylock(&printing_lock)) > return; > > -- > 2.37.2