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

[John Ogness <john.ogness@linutronix.de>]

On 2025-11-11, Petr Mladek <pmladek@suse.com> wrote:
>> Introduce a new global variable @console_offload_blocked to flag
>
> s/console_offload_blocked/console_irqwork_blocked/

Ack.

>> 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?

My intention was to focus only on irq_work triggered directly by
printk() calls as well as transitioning NBCON from threaded to direct.

> 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>.

Earlier test versions of this patch did exactly as you are suggesting
here. But I felt like "properly avoiding" deferred/offloaded printing
via printk_get_console_flush_type() (rather than just hacking
irq_work_queue() callers to abort) was a cleaner solution. Especially
since printk_get_console_flush_type() modifications were needed anyway
in order to transition NBCON from threaded to direct.

>> @@ -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;

I would prefer to keep all the printk_get_console_flush_type() changes
since it returns proper available flush type information. If you would
like to _additionally_ short-circuit __wake_up_klogd() and
nbcon_kthreads_wake() in order to avoid all possible irq_work queueing,
I would be OK with that.

John