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

[Petr Mladek]

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