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

[Andrew Murray]

On Fri, 3 Jul 2026 at 13:57, John Ogness <john.ogness@xxxxxxxxxxxxx> wrote:
>
> Hi,
>
> Sorry I am so late to this party.
>
> On 2026-06-30, Andrew Murray <amurray@xxxxxxxxxxxxxxxxxxxx> 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.

That's no problem, I'll update on the next series.


>
> > 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) {
>

Good spot, that was sloppy of me. I'll fix.

Thanks,

Andrew Murray

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