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

[Petr Mladek]

On Wed 2026-07-08 12:42:22, John Ogness wrote:
> On 2026-07-07, Petr Mladek <pmladek@xxxxxxxx> wrote:
> >> 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().
> >
> > True. The question is if the proper solution is worth the complexity.
> > We would need to pass the information down two level of the API.
> > It would require adding a new (output) parameter to console_flush_one_record(),
> > nbcon_legacy_emit_next_record(), and console_emit_next_record().
> >
> > It is not that complicated but these functions are already hairy
> > enough so we should be careful.
> 
> Fair enough. But then it should be consistent and
> console_emit_next_record() should perform the delay before allowing the
> handover. Something like this:

Great catch!

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2fe9a963c823a..62fd6a5ebef66 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3161,6 +3161,8 @@ static bool console_emit_next_record(struct console *con, bool *handover, int co
>  		 */
>  
>  		con->write(con, outbuf, pmsg.outbuf_len);
> +		printk_delay(false);
> +
>  		con->seq = pmsg.seq + 1;
>  	} else {
>  		/*
> @@ -3182,6 +3184,7 @@ static bool console_emit_next_record(struct console *con, bool *handover, int co
>  		printk_legacy_allow_spinlock_enter();
>  		con->write(con, outbuf, pmsg.outbuf_len);
>  		printk_legacy_allow_spinlock_exit();
> +		printk_delay(true);

It is pity that this is inside printk_safe context with IRQs
disabled. A solution might be to call it before the write.

But maybe, this is good enough. This code path is called from
vprintk_emit() and it might be in an atomic context anyway.
Also it increases the chance of successful handover which
might help as well.

Anyway, it would be nice to mention these pitfalls into commit message.

>  
>  		start_critical_timings();
>  
> 
> John