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

[Andrew Murray]

Hi John,

On Wed, 8 Jul 2026 at 09:19, John Ogness <john.ogness@xxxxxxxxxxxxx> wrote:
>
> On 2026-07-07, Petr Mladek <pmladek@xxxxxxxx> wrote:
> > To summarize this discussion:
> >
> >   + Benedikt would prefer to add the synchronous mode before
> >     moving the delay from the printk() caller to the console emit
> >     code path.
> >
> >   + The delay in the printk() caller code path did not guarantee
> >     the output but it increased the chance to see it.
> >
> >   + The synchronous mode will be even more reliable than the delay
> >     in printk() caller path.
> >
> > Please, let me know if I did not get it right,
> >
> > John, did you have any plan how to add the synchronous mode, please?
> > Does it look complicated?
> >
> > I guess that we would somehow need to "mis-use" the emergency
> > priority and force it everywhere by some global system setting.
>
> The synchronous mode would rely on the driver being nbcon. I envision
> something like this:

Is it possible to apply synchronous mode for both nbcon and legacy
consoles? For the debugging case Benedikt suggested (is this the main
use case for this feature?), it would be helpful if a developer could
indicate a preference for synchronous mode without first determining
the console type. Though I'm not sure what means you propose to
indicate such a preference.


>
> ---- BEGIN SYNC IDEA ----
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2fe9a963c823a..beb5bb3c037b5 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2456,6 +2456,8 @@ asmlinkage int vprintk_emit(int facility, int level,
>
>         if (ft.nbcon_atomic)
>                 nbcon_atomic_flush_pending();
> +       else if (have_nbcon_console_sync)
> +               nbcon_atomic_flush_sync_pending();
>
>         if (ft.nbcon_offload)
>                 nbcon_kthreads_wake();

My interest in misusing (not proposing that as a solution) the
emergency mode, is that the flush mode is nicely abstracted by
printk_get_console_flush_type. Depending on how you trigger the sync
mode, could printk_get_console_flush_type be expanded to look at a
flag and return ft.legacy_direct, ft.legacy_offload or
ft.nbcon_atomic_sync ? I.e. if a sync flag isn't set then
printk_get_console_flush_type returns based on NBCON_PRIO_type but if
it is, then it shortcuts to ft.legacy_direct, ft.legacy_offload or
ft.nbcon_atomic_sync?

I'm not familiar with this area of printk, so please forgive any
obvious issues with this. But as someone that happily uses printk to
debug the kernel, it would be nice to have an easy way to do this
without the friction of figuring out what console I have.


> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 4b03b019cd5ee..8f8f29f757614 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1200,7 +1200,7 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
>         cookie = console_srcu_read_lock();
>
>         flags = console_srcu_read_flags(con);
> -       if (console_is_usable(con, flags, false)) {
> +       if (!(flags & CON_SYNC) &&  console_is_usable(con, flags, false)) {
>                 /* Bring the sequence in @ctxt up to date */
>                 ctxt->seq = nbcon_seq_read(con);
>
> @@ -1654,7 +1654,7 @@ static void nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq)
>   *                                     write_atomic() callback
>   * @stop_seq:                  Flush up until this record
>   */
> -static void __nbcon_atomic_flush_pending(u64 stop_seq)
> +static void __nbcon_atomic_flush_pending(u64 stop_seq, bool sync_only)
>  {
>         struct console *con;
>         int cookie;
> @@ -1666,6 +1666,9 @@ static void __nbcon_atomic_flush_pending(u64 stop_seq)
>                 if (!(flags & CON_NBCON))
>                         continue;
>
> +               if (sync_only && !(flags & CON_SYNC))
> +                       continue;
> +
>                 if (!console_is_usable(con, flags, true))
>                         continue;
>
> @@ -1688,7 +1691,12 @@ static void __nbcon_atomic_flush_pending(u64 stop_seq)
>   */
>  void nbcon_atomic_flush_pending(void)
>  {
> -       __nbcon_atomic_flush_pending(prb_next_reserve_seq(prb));
> +       __nbcon_atomic_flush_pending(prb_next_reserve_seq(prb), false);
> +}
> +
> +void nbcon_atomic_flush_sync_pending(void)
> +{
> +       __nbcon_atomic_flush_pending(prb_next_reserve_seq(prb), true);
>  }
>
>  /**
> @@ -1701,7 +1709,7 @@ void nbcon_atomic_flush_pending(void)
>  void nbcon_atomic_flush_unsafe(void)
>  {
>         panic_nbcon_allow_unsafe_takeover = true;
> -       __nbcon_atomic_flush_pending(prb_next_reserve_seq(prb));
> +       __nbcon_atomic_flush_pending(prb_next_reserve_seq(prb), false);
>         panic_nbcon_allow_unsafe_takeover = false;
>  }
>
> ---- END SYNC IDEA ----
>
> BTW: The sync mode is the final piece so that PeterZ can start using
> mainline code for debugging by serial instead of his own workaround [0].
>
> John

Thanks,

Andrew Murray

>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/log/?h=debug/experimental