Received-SPF: pass (google.com: domain of linux-kernel+bounces-213228-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 13 Jun 2024 14:44:53 +0200
From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org
Subject: atomic_flush vs boot consoles - was: Re: [PATCH printk v2 05/18]
 printk: Atomic print in printk context on shutdown
Message-ID: <ZmrpxWxUb8k4swrA@pathway.suse.cz>
References: <20240603232453.33992-1-john.ogness@linutronix.de>
 <20240603232453.33992-6-john.ogness@linutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240603232453.33992-6-john.ogness@linutronix.de>

On Tue 2024-06-04 01:30:40, John Ogness wrote:
> For nbcon consoles, normally the printing is handled by the
> dedicated console printing threads. However, on shutdown the
> printing threads may not get a chance to print the final
> messages.
> 
> When shutting down or rebooting (system_state > SYSTEM_RUNNING),
> perform atomic printing from the printk() caller context.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
>  kernel/printk/nbcon.c  | 5 +++--
>  kernel/printk/printk.c | 7 ++++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2389,12 +2389,17 @@ asmlinkage int vprintk_emit(int facility, int level,
>  		 *
>  		 * - When this CPU is in panic.
>  		 *
> +		 * - During shutdown, since the printing threads may not get
> +		 *   a chance to print the final messages.
> +		 *
>  		 * Note that if boot consoles are registered, the console
>  		 * lock/unlock dance must be relied upon instead because nbcon
>  		 * consoles cannot print simultaneously with boot consoles.
>  		 */
> -		if (is_panic_context)
> +		if (is_panic_context ||
> +		    (system_state > SYSTEM_RUNNING)) {
>  			nbcon_atomic_flush_pending();
> +		}
>  	}

I was curious why we do not need to add this check also to
console_flush_all() which checks con->kthread as well.

I realized that we call it only when

  #define printing_via_unlock (have_legacy_console || have_boot_console)

=> it handles nbcon consoles only when there is still a boot console.

It actually have to handle them in this case because the boot consoles
can be serialized only via console_lock().

In fact, nbcon_atomic_flush_pending() is not safe when there are boot
consoles. This is handled in vprintk_emit(). But it is not enforced
on several other locations.

I played with a possible solutions. The conditions around the console
calls are getting more and more complicated. I am going to send a
solution which looks acceptable to me.

Best Regards,
Petr