Re: [PATCH printk v1 1/1] printk: Avoid scheduling irq_work on suspend

On Tue 2025-11-11 15:49:22, John Ogness wrote:

Allowing irq_work to be scheduled while trying to suspend has shown to cause problems as some architectures interpret the pending interrupts as a reason to not suspend. This became a problem for printk() with the introduction of NBCON consoles. With every printk() call, NBCON console printing kthreads are woken by queueing irq_work. This means that irq_work continues to be queued due to printk() calls late in the suspend procedure.

Avoid this problem by preventing printk() from queueing irq_work once console suspending has begun. This applies to triggering NBCON and legacy deferred printing as well as klogd waiters.

Since triggering of NBCON threaded printing relies on irq_work, the pr_flush() within console_suspend_all() is used to perform the final flushing before suspending consoles and blocking irq_work queueing. NBCON consoles that are not suspended (due to the usage of the "no_console_suspend" boot argument) transition to atomic flushing.

Introduce a new global variable @console_offload_blocked to flag

s/console_offload_blocked/console_irqwork_blocked/

when irq_work queueing is to be avoided. The flag is used by printk_get_console_flush_type() to avoid allowing deferred printing and switch NBCON consoles to atomic flushing. It is also used by vprintk_emit() to avoid klogd waking.

--- a/kernel/printk/internal.h +++ b/kernel/printk/internal.h @@ -230,6 +230,8 @@ struct console_flush_type { bool legacy_offload; }; +extern bool console_irqwork_blocked;

/*

• Identify which console flushing methods should be used in the context of
• the caller.

@@ -241,7 +243,7 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft) switch (nbcon_get_default_prio()) { case NBCON_PRIO_NORMAL: if (have_nbcon_console && !have_boot_console) {

• 	if (printk_kthreads_running)
  

• 	if (printk_kthreads_running && !console_irqwork_blocked)
  	ft->nbcon_offload = true;
  else
  	ft->nbcon_atomic = true;
  

@@ -251,7 +253,7 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft) if (have_legacy_console || have_boot_console) { if (!is_printk_legacy_deferred()) ft->legacy_direct = true;

• 	else
  

• 	else if (!console_irqwork_blocked)
  	ft->legacy_offload = true;
  

  } break;

This is one possibility.

Another possibility would be to block the irq work directly in defer_console_output() and wake_up_klogd(). It would handle all situations, including printk_trigger_flush() or defer_console_output().

Or is there any reason, why these two call paths are not handled?

I do not have strong opinion. This patch makes it more explicit when defer_console_output() or wake_up_klogd() is called.

If we move the check into defer_console_output() or wake_up_klogd(), it would hide the behavior. But it will make the API more safe to use. And wake_up_klogd() is even exported via <linux/printk.h>.

@@ -264,7 +266,7 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft) if (have_legacy_console || have_boot_console) { if (!is_printk_legacy_deferred()) ft->legacy_direct = true;

• 	else
  

• 	else if (!console_irqwork_blocked)
  	ft->legacy_offload = true;
  

This change won't be needed if we move the check into defer_console_output() and wake_up_klogd().

}
break;

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 5aee9ffb16b9a..94fc4a8662d4b 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2426,7 +2429,7 @@ asmlinkage int vprintk_emit(int facility, int level, if (ft.legacy_offload) defer_console_output();

• else

• else if (!console_irqwork_blocked) wake_up_klogd();

Same here.

return printed_len;

The rest of the patch looks good to me.

Best Regards, Petr