linux-kernel - Re: printk's threaded legacy console + fbcon => schedule where it should not

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXDYEVlkFgxdSVSG@pathway.suse.cz>
Date: Wed, 21 Jan 2026 14:43:45 +0100
From: Petr Mladek <pmladek@...e.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	linux-fbdev@...r.kernel.org,
	John Ogness <john.ogness@...utronix.de>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>, Simona Vetter <simona@...ll.ch>,
	Helge Deller <deller@....de>
Subject: Re: printk's threaded legacy console + fbcon => schedule where it
 should not

On Tue 2026-01-20 11:08:45, Steven Rostedt wrote:
> On Wed, 14 Jan 2026 15:59:55 +0100
> Sebastian Andrzej Siewior <bigeasy@...utronix.de> wrote:
> 
> > @@ -3362,22 +3362,6 @@ void console_unlock(void)
> >  }
> >  EXPORT_SYMBOL(console_unlock);
> >  
> > -/**
> > - * console_conditional_schedule - yield the CPU if required
> 
> Egad! That goes all the way back to 2002:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=a880f45a48be2956d2c78a839c472287d54435c1
>
> > - *
> > - * If the console code is currently allowed to sleep, and
> > - * if this CPU should yield the CPU to another task, do
> > - * so here.
> > - *
> > - * Must be called within console_lock();.
> > - */
> > -void __sched console_conditional_schedule(void)
> > -{
> > -	if (console_may_schedule)
> > -		cond_resched();
> > -}
> > -EXPORT_SYMBOL(console_conditional_schedule);
> 
> I'm assuming this likely isn't needed anymore. I don't know of any reason
> it needs to stay.

I know that there was a plan to get rid of cond_resched().
But what is the status now, please?

I still see more that 1k cond_resched() calls in the code:

  $> git grep cond_resched\(\) | grep "\.c:" | wc -l
  1263

And config PREEMPT_VOLUNTARY still talks about the explicit
preemption points.

> Should we just remove it and see what breaks?

Honestly, I do not feel comfortable with removing it. It is true that
it has no effect in the printk() code path. But the vt code is used
also when working on the terminal.

The vt code still uses console_lock() because it was intertwined
with printk console code since very old days. console_lock is a kind
of big kernel lock there.


Alternative solution is to get rid of the spin_trylock(). The only
purpose is to prevent race in console_flush_on_panic(). It used
to be a simple bit operation. The spin_lock() was added just to
get barriers right. But we have a great atomic_t API these days.

IMHO, it is a win-win solution because a preemptive context is
always better.

What about?

>From 0fc61b6877e9beb20429effc599bc4bc6ec3a475 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@...e.com>
Date: Wed, 21 Jan 2026 10:47:15 +0100
Subject: [RFC] tty/vt: Prevent re-entering vt_console_print() in panic()
 without spin_lock

The commit b0940003f25dd ("vt: bitlock fix") replaced a simple bit
operation with spin_lock() to get proper memory barriers.

But the code called under this lock calls console_conditional_schedule()
which calls cond_resched() when console_sem() has been acquired
in a preemptive context using console_lock(). Note that the semaphore
can be taken also in an atomic context using console_trylock()
which is used by printk().

One solution would be to remove console_conditional_schedule().
It does not have any effect in the printk() code path anyway.
But the affected VT code is not used just by printk(). And
the cond_resched() calls were likely added for a reason.

Instead, convert the spin_lock back to an atomic operation with
proper barriers. The only purpose of the lock is to prevent
a concurrent access to the guarded code in
console_flush_on_panic() where console_lock() is ignored.
Using a full featured spin_trylock, just to get memory barriers
right, looks like an overkill anyway.

Fixes: b0940003f25dd ("vt: bitlock fix")
Closes: https://lore.kernel.org/all/20260114145955.d924Z-zu@linutronix.de/
Signed-off-by: Petr Mladek <pmladek@...e.com>
---
 drivers/tty/vt/vt.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 59b4b5e126ba..5be64d1bba91 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3353,15 +3353,19 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
 {
 	struct vc_data *vc = vc_cons[fg_console].d;
 	unsigned char c;
-	static DEFINE_SPINLOCK(printing_lock);
+	static atomic_t printing_lock = ATOMIC_INIT(0);
 	const ushort *start;
 	ushort start_x, cnt;
 	int kmsg_console;
 
 	WARN_CONSOLE_UNLOCKED();
 
-	/* this protects against concurrent oops only */
-	if (!spin_trylock(&printing_lock))
+	/*
+	 * Prevent concurrent printing in console_flush_on_panic() where
+	 * console_lock is ignored. Easier (serial) console drivers
+	 * have bigger chance to get the messages out.
+	 */
+	if (atomic_cmpxchg_acquire(&printing_lock, 0, 1) != 0)
 		return;
 
 	kmsg_console = vt_get_kmsg_redirect();
@@ -3422,7 +3426,7 @@ static void vt_console_print(struct console *co, const char *b, unsigned count)
 	notify_update(vc);
 
 quit:
-	spin_unlock(&printing_lock);
+	atomic_set_release(&printing_lock, 0);
 }
 
 static struct tty_driver *vt_console_device(struct console *c, int *index)
-- 
2.52.0

Best Regards,
Petr