Re: [PATCH v2 3/4] printk: nbcon: move printk_delay to console emiting code

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

Hi,

Sorry I am so late to this party.

On 2026-06-30, Andrew Murray <amurray@thegoodpenguin.co.uk> wrote:
> diff --git a/include/linux/console.h b/include/linux/console.h
> index d624200cfc1708bf73925892a466efe0c95c5586..3478b556c0eb9579530409dc6fbb9b5a8bff581c 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -290,6 +290,8 @@ struct nbcon_context {
>   * @outbuf:		Pointer to the text buffer for output
>   * @len:		Length to write
>   * @unsafe_takeover:	If a hostile takeover in an unsafe state has occurred
> + * @emitted:		The write context attempted to emit the message. Might
> + *			be incomplete.
>   * @cpu:		CPU on which the message was generated
>   * @pid:		PID of the task that generated the message
>   * @comm:		Name of the task that generated the message
> @@ -298,7 +300,8 @@ struct nbcon_write_context {
>  	struct nbcon_context	__private ctxt;
>  	char			*outbuf;
>  	unsigned int		len;
> -	bool			unsafe_takeover;
> +	unsigned char		unsafe_takeover : 1;
> +	unsigned char		emitted		: 1;

This is the wrong structure to add this flag. This structure is for
the nbcon drivers.

struct nbcon_context would be the correct structure.

> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 4b03b019cd5ee25d68e9ace84392045e91241a7f..ae45cb0589c0effafc66f1756bdaecd1c1e53ab9 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1525,6 +1532,8 @@ bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
>  	}
>  
>  	progress = nbcon_emit_one(&wctxt, use_atomic);
> +	if (progress && wctxt.emitted)
> +		printk_delay(use_atomic);
>  
>  	if (use_atomic) {
>  		start_critical_timings();

This is too deep (also pointed out by Sashiko) because it multiplies the
delay times the number of consoles. For the legacy printing, it would be
more appropriate to put the delay inside console_flush_all() and
legacy_kthread_func().

> @@ -1584,6 +1593,8 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq)
>  			if (!nbcon_context_try_acquire(ctxt, false))
>  				return -EPERM;
>  
> +			wctxt.emitted = 0;
> +
>  			/*
>  			 * nbcon_emit_next_record() returns false when
>  			 * the console was handed over or taken over.
> @@ -1600,6 +1611,8 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq)
>  			if (nbcon_seq_read(con) < stop_seq)
>  				err = -ENOENT;
>  			break;
> +		} else if (wctxt.emitted > 0) {
> +			printk_delay(true);

@emitted is a flag:

		} else if (wctxt.emitted) {

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index cc203327247aa4f81f55b907c66ac88f30ce6da8..5278d9cb19e4177a00998fba5c1438251e033578 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3211,6 +3208,8 @@ static bool console_emit_next_record(struct console *con, bool *handover, int co
>  		*handover = console_lock_spinning_disable_and_check(cookie);
>  		printk_safe_exit_irqrestore(flags);
>  	}
> +	printk_delay(true);
> +

Again, too deep. Let console_flush_all() and legacy_kthread_func()
perform the delay appropriately between each message.

John Ogness