Re: [PATCH printk v4 09/17] printk: nbcon: Rely on kthreads for normal operation

From: Petr Mladek
Date: Tue Aug 27 2024 - 11:50:40 EST


On Tue 2024-08-27 06:49:25, John Ogness wrote:
> Once the kthread is running and available
> (i.e. @printk_kthreads_running is set), the kthread becomes
> responsible for flushing any pending messages which are added
> in NBCON_PRIO_NORMAL context. Namely the legacy
> console_flush_all() and device_release() no longer flush the
> console. And nbcon_atomic_flush_pending() used by
> nbcon_cpu_emergency_exit() no longer flushes messages added
> after the emergency messages.
>
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2749,6 +2753,10 @@ void resume_console(void)
> */
> synchronize_srcu(&console_srcu);
>
> + printk_get_console_flush_type(&ft);
> + if (ft.nbcon_offload)
> + nbcon_kthreads_wake();

Who would flush the nbcon consoles in the following case?

ft.nbcon_atomic == true && ft.legacy_direct == false

pr_flush() won't call the legacy loop in this case.

We should probably update pr_flush() to call nbcon_atomic_flush_pending()
in this case.

> +
> pr_flush(1000, true);
> }
>
> @@ -3387,9 +3405,25 @@ EXPORT_SYMBOL(console_stop);
>
> void console_start(struct console *console)
> {
> + struct console_flush_type ft;
> + bool is_nbcon;
> +
> console_list_lock();
> console_srcu_write_flags(console, console->flags | CON_ENABLED);
> + is_nbcon = console->flags & CON_NBCON;
> console_list_unlock();
> +
> + /*
> + * Ensure that all SRCU list walks have completed. The related
> + * printing context must be able to see it is enabled so that
> + * it is guaranteed to wake up and resume printing.
> + */
> + synchronize_srcu(&console_srcu);
> +
> + printk_get_console_flush_type(&ft);
> + if (is_nbcon && ft.nbcon_offload)
> + nbcon_kthread_wake(console);

Same here.

> __pr_flush(console, 1000, true);
> }
> EXPORT_SYMBOL(console_start);
> @@ -4629,8 +4665,13 @@ EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
> */
> void console_try_replay_all(void)
> {
> + struct console_flush_type ft;
> +
> + printk_get_console_flush_type(&ft);
> if (console_trylock()) {
> __console_rewind_all();
> + if (ft.nbcon_offload)
> + nbcon_kthreads_wake();

And here. We in this case, we should likely call add here:

if (ft.nbcon_atomic)
nbcon_atomic_flush_pending()

> /* Consoles are flushed as part of console_unlock(). */
> console_unlock();
> }

Best Regards,
Petr