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

[Petr Mladek <pmladek@suse.com>]

On Fri 2026-07-03 15:03:51, John Ogness wrote:
> 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.

Good point. I see:

  + "struct nbcon_write_context" is intended for passing
     information down to the console drivers via
     .write_thread() and .write_atomic() callbacks.

  + "struct nbcon_contex" aleady contains section
    "members set by emit".

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

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.

Best Regards,
Petr