[PATCH printk v2] printk: Postpone kthread startup

[PATCH printk v2] printk: Postpone kthread startup

[John Ogness]

There have been various bug reports related to SMP systems hanging
on boot. The reports vary but seem to have some overlap with regard
to missed IPI's during SMP setup. The reports also show that
reverting NBCON console usage allows the bugs to disappear.

The NBCON kthreads for console printing are started very early
during boot using early_initcall(). Since these kthreads are
asynchronously enabling/disabling interrupts during early boot, they
may be interfering with early SMP setup on some hardware platforms.

Starting the console printing kthreads later has shown to avoid the
hanging on systems that could previously reproduce the issue.

Postpone kthread starting until after all arch initcalls have
completed.

Reported-by: Derek Barbosa <debarbos@redhat.com>
Closes: https://lore.kernel.org/linux-rt-users/arPmYPbFLf2EldIl@debarbos-thinkpadt14gen5.rmtusma.csb
Fixes: 76f258bf3f2a ("printk: nbcon: Introduce printer kthreads")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 v1: https://lore.kernel.org/lkml/87ecdrqfhw.fsf@jogness.linutronix.de

 changes since v1:
 - Reword commit message.
 - Rebase from 6.6-rt to 7.3-rc.

 kernel/printk/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0ed86b5e8b160..9a205f94bfac7 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3864,7 +3864,7 @@ static int __init printk_set_kthreads_ready(void)
 
 	return 0;
 }
-early_initcall(printk_set_kthreads_ready);
+subsys_initcall(printk_set_kthreads_ready);
 #endif /* CONFIG_PRINTK */
 
 static int __read_mostly keep_bootcon;

base-commit: 38dfd607b58b63b5488d5756786ee4ee499eebb8
-- 
2.47.3

Re: [PATCH printk v2] printk: Postpone kthread startup

[John Ogness]

On 2026-10-16, sashiko.dev wrote:
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 0ed86b5e8b160..9a205f94bfac7 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -3864,7 +3864,7 @@ static int __init printk_set_kthreads_ready(void)
>>
>>  	return 0;
>>  }
>> -early_initcall(printk_set_kthreads_ready);
>> +subsys_initcall(printk_set_kthreads_ready);
>
> Can this delay expose a regression where a use-after-free occurs
> during failure paths?
>
> If nbcon_kthread_create(con) fails, it calls
> unregister_console_locked(con).  This unregisters the console and
> recursively calls printk_kthreads_check_locked().
>
> The recursive call restarts iterating over console_list and may
> unregister subsequent consoles, including the tmp node held by the
> outer loop:
>
> kernel/printk/printk.c:printk_kthreads_check_locked() {
> 	hlist_for_each_entry_safe(con, tmp, &console_list, node) {
>		if (!(con->flags & CON_NBCON))
>			continue;
>
>		if (!nbcon_kthread_create(con))
>			unregister_console_locked(con);
>	}
> }
>
> kernel/printk/printk.c:unregister_console_locked() {
>	...
>	/* Changed console list, may require printer threads to start/stop. */
>	printk_kthreads_check_locked();
>	...
> }
>
> Because hlist_for_each_entry_safe relies on the tmp pointer, the outer
> loop will later process the already unregistered tmp console and call
> nbcon_kthread_create(tmp) on it.
>
> If this succeeds on the retry, it spawns a zombie kthread for a
> detached console. If the detached console is dynamically allocated and
> freed by its driver's exit routine, the kthread and the outer loop
> will dereference freed memory.
>
> Can the loop in printk_kthreads_check_locked() handle the recursive
> unregistration of the tmp pointer safely without resulting in a
> use-after-free?

The case Sashiko presents is possible (even if unlikely). I would not
associate it with this patch though. It is a pre-existing
condition. Probably the cleanest fix is to pull
printk_kthreads_check_locked() out of
unregister_console_locked(). Something like this:

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0ed86b5e8b160..0a87f49b3908a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4322,9 +4322,6 @@ static int unregister_console_locked(struct console *console)
 	if (console->exit)
 		res = console->exit(console);
 
-	/* Changed console list, may require printer threads to start/stop. */
-	printk_kthreads_check_locked();
-
 	return res;
 }
 
@@ -4333,7 +4330,12 @@ int unregister_console(struct console *console)
 	int res;
 
 	console_list_lock();
+
 	res = unregister_console_locked(console);
+
+	/* Changed console list, may require printer threads to start/stop. */
+	printk_kthreads_check_locked();
+
 	console_list_unlock();
 	return res;
 }
@@ -4471,6 +4473,9 @@ static int __init printk_late_init(void)
 		}
 
 		unregister_console_locked(con);
+
+		/* Changed console list, may require printer threads to start/stop. */
+		printk_kthreads_check_locked();
 	}
 	console_list_unlock();

I can submit an official patch after the 7.4 merge window.

John

Re: [PATCH printk v2] printk: Postpone kthread startup

[Petr Mladek]

On Fri 2026-10-16 15:00:54, John Ogness wrote:
> On 2026-10-16, sashiko.dev wrote:
> >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> >> index 0ed86b5e8b160..9a205f94bfac7 100644
> >> --- a/kernel/printk/printk.c
> >> +++ b/kernel/printk/printk.c
> >> @@ -3864,7 +3864,7 @@ static int __init printk_set_kthreads_ready(void)
> >>
> >>  	return 0;
> >>  }
> >> -early_initcall(printk_set_kthreads_ready);
> >> +subsys_initcall(printk_set_kthreads_ready);
> >
> > Can this delay expose a regression where a use-after-free occurs
> > during failure paths?
> >
> > If nbcon_kthread_create(con) fails, it calls
> > unregister_console_locked(con).  This unregisters the console and
> > recursively calls printk_kthreads_check_locked().
> >
> > The recursive call restarts iterating over console_list and may
> > unregister subsequent consoles, including the tmp node held by the
> > outer loop:
> >
> > kernel/printk/printk.c:printk_kthreads_check_locked() {
> > 	hlist_for_each_entry_safe(con, tmp, &console_list, node) {
> >		if (!(con->flags & CON_NBCON))
> >			continue;
> >
> >		if (!nbcon_kthread_create(con))
> >			unregister_console_locked(con);
> >	}
> > }
> >
> > kernel/printk/printk.c:unregister_console_locked() {
> >	...
> >	/* Changed console list, may require printer threads to start/stop. */
> >	printk_kthreads_check_locked();
> >	...
> > }
> >
> > Because hlist_for_each_entry_safe relies on the tmp pointer, the outer
> > loop will later process the already unregistered tmp console and call
> > nbcon_kthread_create(tmp) on it.

Heh, I have missed this recursion. It is obviously problematic.

> > If this succeeds on the retry, it spawns a zombie kthread for a
> > detached console. If the detached console is dynamically allocated and
> > freed by its driver's exit routine, the kthread and the outer loop
> > will dereference freed memory.
> >
> > Can the loop in printk_kthreads_check_locked() handle the recursive
> > unregistration of the tmp pointer safely without resulting in a
> > use-after-free?
> 
> The case Sashiko presents is possible (even if unlikely). I would not
> associate it with this patch though. It is a pre-existing
> condition. Probably the cleanest fix is to pull
> printk_kthreads_check_locked() out of
> unregister_console_locked(). Something like this:

Breaking the regression is an obvious solution.

Alternative solution might be to split printk_kthreads_check_locked()
into two similar functions. One for creating kthreads and 2nd for
removing them. We need to combine creating+removing only when
a kthread can't be created. The removal part should not need
recursion in principle. But it might require duplication of
the various checks.

Your proposal looks easier.

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 0ed86b5e8b160..0a87f49b3908a 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -4322,9 +4322,6 @@ static int unregister_console_locked(struct console *console)
>  	if (console->exit)
>  		res = console->exit(console);
>  
> -	/* Changed console list, may require printer threads to start/stop. */
> -	printk_kthreads_check_locked();
> -
>  	return res;
>  }
>  
> @@ -4333,7 +4330,12 @@ int unregister_console(struct console *console)
>  	int res;
>  
>  	console_list_lock();
> +
>  	res = unregister_console_locked(console);
> +
> +	/* Changed console list, may require printer threads to start/stop. */

It might make sense to warn about the potential recursion. Otherwise,
someone might try to optimize it back. And the problem is far from
obvious. We should not rely on Sashiko...

Something like:

	/*
	 * Changed console list, may require printer threads to	start/stop.
	 * Do this outside of unregister_console_locked() to avoid regression.
	 */

Or we could add a comment above printk_kthreads_check_locked(). But it
would be easier to overlook.

> +	printk_kthreads_check_locked();
> +
>  	console_list_unlock();
>  	return res;
>  }
> @@ -4471,6 +4473,9 @@ static int __init printk_late_init(void)
>  		}
>  
>  		unregister_console_locked(con);
> +
> +		/* Changed console list, may require printer threads to start/stop. */

Same here.

> +		printk_kthreads_check_locked();
>  	}
>  	console_list_unlock();
> 
> I can submit an official patch after the 7.4 merge window.

That would be great. Thanks in advance.

Best Regards,
Petr

Re: [PATCH printk v2] printk: Postpone kthread startup

[Derek Barbosa]

On Fri, Oct 16, 2026 at 04:43:47PM +0200, Petr Mladek wrote:
> On Fri 2026-10-16 15:00:54, John Ogness wrote:
> > On 2026-10-16, sashiko.dev wrote:
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 0ed86b5e8b160..0a87f49b3908a 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -4322,9 +4322,6 @@ static int unregister_console_locked(struct console *console)
> >  	if (console->exit)
> >  		res = console->exit(console);
> >  
> > -	/* Changed console list, may require printer threads to start/stop. */
> > -	printk_kthreads_check_locked();
> > -
> >  	return res;
> >  }
> >  
> > @@ -4333,7 +4330,12 @@ int unregister_console(struct console *console)
> >  	int res;
> >  
> >  	console_list_lock();
> > +
> >  	res = unregister_console_locked(console);
> > +
> > +	/* Changed console list, may require printer threads to start/stop. */
> 
> It might make sense to warn about the potential recursion. Otherwise,
> someone might try to optimize it back. And the problem is far from
> obvious. We should not rely on Sashiko...
> 
> Something like:
> 
> 	/*
> 	 * Changed console list, may require printer threads to	start/stop.
> 	 * Do this outside of unregister_console_locked() to avoid regression.
> 	 */

Sorry for chiming in here. It might be an obvious (and therefore useless)
comment, but could we expand this warning to say:

    /*
     * Changed console list, may require printer threads to start/stop.
     * Perform this check outside of unregister_console_locked() call sites
     * to avoid use-after-free scenarios.
     /

to elaborate on what "regression" could mean?

> Or we could add a comment above printk_kthreads_check_locked(). But it
> would be easier to overlook.
> 

I could be wrong, but iss there any harm in doing both (outside of being
redundant)? Explaining the "design pattern" above the function could be helpful

    /*
     * If appropriate, start nbcon kthreads and set @printk_kthreads_running.
     * If any kthreads fail to start, unregister those consoles.
     *
     * Refrain from calling this function within routines that may iterate over
     * the active console list to perform any (un)registration.
     *
     * Must be called under console_list_lock().
     */

IIUC it just makes it a little more clear that the old behavior, where the inner
loop was mutating the outer loop's iterator, should be avoided.


> > +	printk_kthreads_check_locked();
> > +
> >  	console_list_unlock();
> >  	return res;
> >  }
> > @@ -4471,6 +4473,9 @@ static int __init printk_late_init(void)
> >  		}
> >  
> >  		unregister_console_locked(con);
> > +
> > +		/* Changed console list, may require printer threads to start/stop. */
> 
> Same here.
> 
> > +		printk_kthreads_check_locked();
> >  	}
> >  	console_list_unlock();
> > 
> > I can submit an official patch after the 7.4 merge window.
> 
> That would be great. Thanks in advance.
> 
> Best Regards,
> Petr
> 

-- 
Derek <debarbos@redhat.com>

Re: [PATCH printk v2] printk: Postpone kthread startup

[Derek Barbosa]

On Fri, Oct 16, 2026 at 11:00:34AM +0206, John Ogness wrote:
> There have been various bug reports related to SMP systems hanging
> on boot. The reports vary but seem to have some overlap with regard
> to missed IPI's during SMP setup. The reports also show that
> reverting NBCON console usage allows the bugs to disappear.
> 
> The NBCON kthreads for console printing are started very early
> during boot using early_initcall(). Since these kthreads are
> asynchronously enabling/disabling interrupts during early boot, they
> may be interfering with early SMP setup on some hardware platforms.
> 
> Starting the console printing kthreads later has shown to avoid the
> hanging on systems that could previously reproduce the issue.
> 
> Postpone kthread starting until after all arch initcalls have
> completed.
> 
> Reported-by: Derek Barbosa <debarbos@redhat.com>
> Closes: https://lore.kernel.org/linux-rt-users/arPmYPbFLf2EldIl@debarbos-thinkpadt14gen5.rmtusma.csb
> Fixes: 76f258bf3f2a ("printk: nbcon: Introduce printer kthreads")
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
>  v1: https://lore.kernel.org/lkml/87ecdrqfhw.fsf@jogness.linutronix.de
> 
>  changes since v1:
>  - Reword commit message.
>  - Rebase from 6.6-rt to 7.3-rc.
> 
>  kernel/printk/printk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 0ed86b5e8b160..9a205f94bfac7 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3864,7 +3864,7 @@ static int __init printk_set_kthreads_ready(void)
>  
>  	return 0;
>  }
> -early_initcall(printk_set_kthreads_ready);
> +subsys_initcall(printk_set_kthreads_ready);
>  #endif /* CONFIG_PRINTK */
>  
>  static int __read_mostly keep_bootcon;
> 
> base-commit: 38dfd607b58b63b5488d5756786ee4ee499eebb8
> -- 
> 2.47.3
> 

In the v1 thread, I mentioned that I would leave the reproducer spinning in the
test environment for a few more hours. It has been well over 9 hours. No signs
of the existing issue popping up.

Tested-by: Derek Barbosa <debarbos@redhat.com>

-- 
Derek <debarbos@redhat.com>

Re: [PATCH printk v2] printk: Postpone kthread startup

[Petr Mladek]

On Fri 2026-10-16 09:36:41, Derek Barbosa wrote:
> On Fri, Oct 16, 2026 at 11:00:34AM +0206, John Ogness wrote:
> > There have been various bug reports related to SMP systems hanging
> > on boot. The reports vary but seem to have some overlap with regard
> > to missed IPI's during SMP setup. The reports also show that
> > reverting NBCON console usage allows the bugs to disappear.
> > 
> > The NBCON kthreads for console printing are started very early
> > during boot using early_initcall(). Since these kthreads are
> > asynchronously enabling/disabling interrupts during early boot, they
> > may be interfering with early SMP setup on some hardware platforms.
> > 
> > Starting the console printing kthreads later has shown to avoid the
> > hanging on systems that could previously reproduce the issue.
> > 
> > Postpone kthread starting until after all arch initcalls have
> > completed.
> > 
> > Reported-by: Derek Barbosa <debarbos@redhat.com>
> > Closes: https://lore.kernel.org/linux-rt-users/arPmYPbFLf2EldIl@debarbos-thinkpadt14gen5.rmtusma.csb
> > Fixes: 76f258bf3f2a ("printk: nbcon: Introduce printer kthreads")
> > Signed-off-by: John Ogness <john.ogness@linutronix.de>
> > ---
> >  v1: https://lore.kernel.org/lkml/87ecdrqfhw.fsf@jogness.linutronix.de
> > 
> >  changes since v1:
> >  - Reword commit message.
> >  - Rebase from 6.6-rt to 7.3-rc.
> > 
> >  kernel/printk/printk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 0ed86b5e8b160..9a205f94bfac7 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3864,7 +3864,7 @@ static int __init printk_set_kthreads_ready(void)
> >  
> >  	return 0;
> >  }
> > -early_initcall(printk_set_kthreads_ready);
> > +subsys_initcall(printk_set_kthreads_ready);
> >  #endif /* CONFIG_PRINTK */
> >  
> >  static int __read_mostly keep_bootcon;
> > 
> > base-commit: 38dfd607b58b63b5488d5756786ee4ee499eebb8
> > -- 
> > 2.47.3
> > 
> 
> In the v1 thread, I mentioned that I would leave the reproducer spinning in the
> test environment for a few more hours. It has been well over 9 hours. No signs
> of the existing issue popping up.
> 
> Tested-by: Derek Barbosa <debarbos@redhat.com>

Thanks a lot for testing this. It is good to know that it helped.

I am a bit nervous that we do not know what is the culprit.
For example, I wonder if the same problem might happen during
CPU hot-plug at runtime.

Anyway, I had a brain storming session with John and we were not able
to find any hole in the nbcon API design. We did not find any location
where it could cause deadlock. All busy loops are under
con->device_lock(), aka port->lock. And the port lock is taken with
IRQs disabled. And neither the con->device_lock() nor the busy loop
is done in NMI.

So, this rather looks like an existing problem. And the parallel
handling of the serial console just made it more likely to happen.

The converted uart 8250 is being used on many production systems
which are running the RT kernel and got this change from the external
RT patchset. And I have heard about this problem for the first time just
a few weeks ago. This makes me believe that it affects only small
amount of HW.

This change clearly helped. Which makes me even more confident
that we could keep the uart 8250 driver converted in 7.3.
I am going to commit this patch and create a pull request...

Best Regards,
Petr

Re: [PATCH printk v2] printk: Postpone kthread startup

[Derek Barbosa]

On Fri, Oct 16, 2026 at 05:06:27PM +0200, Petr Mladek wrote:
> On Fri 2026-10-16 09:36:41, Derek Barbosa wrote:
> > On Fri, Oct 16, 2026 at 11:00:34AM +0206, John Ogness wrote:
> > > There have been various bug reports related to SMP systems hanging
> > > on boot. The reports vary but seem to have some overlap with regard
> > > to missed IPI's during SMP setup. The reports also show that
> > > reverting NBCON console usage allows the bugs to disappear.
> > > 
> > > The NBCON kthreads for console printing are started very early
> > > during boot using early_initcall(). Since these kthreads are
> > > asynchronously enabling/disabling interrupts during early boot, they
> > > may be interfering with early SMP setup on some hardware platforms.
> > > 
> > > Starting the console printing kthreads later has shown to avoid the
> > > hanging on systems that could previously reproduce the issue.
> > > 
> > > Postpone kthread starting until after all arch initcalls have
> > > completed.
> > 
> > In the v1 thread, I mentioned that I would leave the reproducer spinning in the
> > test environment for a few more hours. It has been well over 9 hours. No signs
> > of the existing issue popping up.
> > 
> > Tested-by: Derek Barbosa <debarbos@redhat.com>
> 
> Thanks a lot for testing this. It is good to know that it helped.
> 
> I am a bit nervous that we do not know what is the culprit.
> For example, I wonder if the same problem might happen during
> CPU hot-plug at runtime.

FWIW, this fixes a corner-case that seems to affect older platforms that had
been noted to have "issues" with some workloads on older firmware/microcode.

For the cpu hot-plug example, I am unsure of how to properly stress/test that
condition to quell these concerns. I could run a modified version of John's old
console_blast script and take some CPUs on/off-line in a loop with:

    $ echo 1 > /sys/devices/system/cpu/cpu$N/online
    # do some delay here
    $ echo 0 > /sys/devices/system/cpu/cpu$N/online

but I don't think that does a good job of emulating the original bug, where
kicking on the console-printing-kthreads very early in the init process (prior
to smp's secondary cpu bringup) could interfere with SMP setup. I could always
give it a shot.

> 
> Anyway, I had a brain storming session with John and we were not able
> to find any hole in the nbcon API design. We did not find any location
> where it could cause deadlock. All busy loops are under
> con->device_lock(), aka port->lock. And the port lock is taken with
> IRQs disabled. And neither the con->device_lock() nor the busy loop
> is done in NMI.
> 
> So, this rather looks like an existing problem. And the parallel
> handling of the serial console just made it more likely to happen.

I also hold this to be true. Considering I haven't seen anything quite like this
(up until this point) even with older versions of the code, I am chalking it up
to some sort perfect-storm of bad interactions.

> 
> The converted uart 8250 is being used on many production systems
> which are running the RT kernel and got this change from the external
> RT patchset. And I have heard about this problem for the first time just
> a few weeks ago. This makes me believe that it affects only small
> amount of HW.
> 
> This change clearly helped. Which makes me even more confident
> that we could keep the uart 8250 driver converted in 7.3.
> I am going to commit this patch and create a pull request...
> 
> Best Regards,
> Petr
> 

Cheers,

-- 
Derek <debarbos@redhat.com>