[PATCH printk] printk/nbcon: Change nbcon_irq_work to IRQ_WORK_LAZY

[PATCH printk] printk/nbcon: Change nbcon_irq_work to IRQ_WORK_LAZY

[John Ogness]

For some platforms it is a problem to queue irq_work when entering
cpuidle states. Since nbcon uses irq_work for waking the printing
kthreads, any printk() calls when entering cpuidle states can lead
to the affected hardware hanging. Tegra20 and Tegra30 are examples
of such platforms. Avoiding raising the irq_work IRQ has shown to
circumvent the problem.

Change the nbcon_irq_work to be IRQ_WORK_LAZY, thus not raising
an IRQ upon irq_work queuing. The irq_work is then handled on the
next interrupt (worst case, kernel tick). This additional delay is
acceptable because nbcon_irq_work is only responsible for
non-emergency deferred printing, which is delayed anyway. This also
has the benefit of not needing to raise an IRQ for each printk()
call.

Link: https://lore.kernel.org/lkml/f3757a75-0ba1-4558-bf57-f19ab7e59a4c@nvidia.com
Fixes: 76f258bf3f2a ("printk: nbcon: Introduce printer kthreads")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/nbcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index a5921a84a80ed..ad79e30afd647 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1782,7 +1782,7 @@ bool nbcon_alloc(struct console *con)
 	}
 
 	rcuwait_init(&con->rcuwait);
-	init_irq_work(&con->irq_work, nbcon_irq_work);
+	con->irq_work = IRQ_WORK_INIT_LAZY(nbcon_irq_work);
 	atomic_long_set(&ACCESS_PRIVATE(con, nbcon_prev_seq), -1UL);
 	nbcon_state_set(con, &state);
 

base-commit: ffe0486b139e45cd9c9ca2584f04a1910fe4f8a6
-- 
2.47.3

Re: [PATCH printk] printk/nbcon: Change nbcon_irq_work to IRQ_WORK_LAZY

[Sebastian Andrzej Siewior]

On 2026-08-27 20:53:38 [+0206], John Ogness wrote:
> For some platforms it is a problem to queue irq_work when entering
> cpuidle states. Since nbcon uses irq_work for waking the printing
> kthreads, any printk() calls when entering cpuidle states can lead
> to the affected hardware hanging. Tegra20 and Tegra30 are examples
> of such platforms. Avoiding raising the irq_work IRQ has shown to
> circumvent the problem.
>
> Change the nbcon_irq_work to be IRQ_WORK_LAZY, thus not raising
> an IRQ upon irq_work queuing. The irq_work is then handled on the
> next interrupt (worst case, kernel tick). This additional delay is
> acceptable because nbcon_irq_work is only responsible for
> non-emergency deferred printing, which is delayed anyway. This also
> has the benefit of not needing to raise an IRQ for each printk()
> call.

This makes sense regardless of the Tegra issue. I'm still curious what
happens on Tegra ;)

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> Link: https://lore.kernel.org/lkml/f3757a75-0ba1-4558-bf57-f19ab7e59a4c@nvidia.com
> Fixes: 76f258bf3f2a ("printk: nbcon: Introduce printer kthreads")
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
>  kernel/printk/nbcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index a5921a84a80ed..ad79e30afd647 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1782,7 +1782,7 @@ bool nbcon_alloc(struct console *con)
>  	}
>  
>  	rcuwait_init(&con->rcuwait);
> -	init_irq_work(&con->irq_work, nbcon_irq_work);
> +	con->irq_work = IRQ_WORK_INIT_LAZY(nbcon_irq_work);
>  	atomic_long_set(&ACCESS_PRIVATE(con, nbcon_prev_seq), -1UL);
>  	nbcon_state_set(con, &state);
>  
> 
> base-commit: ffe0486b139e45cd9c9ca2584f04a1910fe4f8a6

Sebastian

Re: [PATCH printk] printk/nbcon: Change nbcon_irq_work to IRQ_WORK_LAZY

[John Ogness]

On 2026-08-27, Sashiko wrote:
>> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
>> index a5921a84a80ed..ad79e30afd647 100644
>> --- a/kernel/printk/nbcon.c
>> +++ b/kernel/printk/nbcon.c
>> @@ -1782,7 +1782,7 @@ bool nbcon_alloc(struct console *con)
>>  	}
>>  
>>  	rcuwait_init(&con->rcuwait);
>> -	init_irq_work(&con->irq_work, nbcon_irq_work);
>> +	con->irq_work = IRQ_WORK_INIT_LAZY(nbcon_irq_work);
>
> This is a pre-existing issue, but does missing irq_work_sync() during
> console unregistration lead to a use-after-free and lockless list corruption,
> which is heavily exacerbated by the shift to IRQ_WORK_LAZY here?
>
> In a rapid unregister/re-register cycle, overwriting con->irq_work without a
> prior sync corrupts the lockless llist if the old irq_work was still queued.
>
> When a console is unregistered (e.g., via module unload or hotplug unplug),
> nbcon_free() stops the printing kthread:
>
> kernel/printk/nbcon.c:nbcon_free() {
>     ...
>     if (printk_kthreads_running) {
>         nbcon_kthread_stop(con);
>     ...
> }
>
> However, it does not call irq_work_sync(&con->irq_work) before the driver
> frees the console structure. 
>
> Since this patch changes the initialization to IRQ_WORK_LAZY, the irq_work
> execution is deferred until the next timer tick.
>
> This massively widens the race window, guaranteeing the irq_work remains
> queued on the CPU's lazy_list while the console is being freed.
>
> The asynchronous callback then dereferences the freed console object:
>
> kernel/printk/nbcon.c:nbcon_irq_work() {
>     struct console *con = container_of(irq_work, struct console, irq_work);
>    
>     nbcon_kthread_wake(con);
> }
>
> Should nbcon_free() be updated to include irq_work_sync(&con->irq_work) to
> flush pending work before the driver is allowed to free the console memory?

Yes, this is needed. I will post a v2 series with a preceeding patch to
add the irq_work_sync().

John

Re: [PATCH printk] printk/nbcon: Change nbcon_irq_work to IRQ_WORK_LAZY

[Petr Mladek]

On Thu 2026-08-27 20:53:38, John Ogness wrote:
> For some platforms it is a problem to queue irq_work when entering
> cpuidle states. Since nbcon uses irq_work for waking the printing
> kthreads, any printk() calls when entering cpuidle states can lead
> to the affected hardware hanging. Tegra20 and Tegra30 are examples
> of such platforms. Avoiding raising the irq_work IRQ has shown to
> circumvent the problem.
> 
> Change the nbcon_irq_work to be IRQ_WORK_LAZY, thus not raising
> an IRQ upon irq_work queuing. The irq_work is then handled on the
> next interrupt (worst case, kernel tick). This additional delay is
> acceptable because nbcon_irq_work is only responsible for
> non-emergency deferred printing, which is delayed anyway. This also
> has the benefit of not needing to raise an IRQ for each printk()
> call.
> 
> Link: https://lore.kernel.org/lkml/f3757a75-0ba1-4558-bf57-f19ab7e59a4c@nvidia.com
> Fixes: 76f258bf3f2a ("printk: nbcon: Introduce printer kthreads")
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

I am going to wait for v2 which would fix the preexisting problem
reported by Sashiko. Anyway, I am going to queue this for 7.3-rcX
to prevent regressions caused by converting the uart 8250 console
driver into nbcon API.

Best Regards,
Petr