Re: [PATCH v2 1/2] Handle flushing of CPU backtraces during panic

[Petr Mladek]

On Sat 2024-08-03 17:12:30, takakura@xxxxxxxxxxxxx wrote:
> From: Ryo Takakura <takakura@xxxxxxxxxxxxx>
> 
> After panic, non-panicked CPU's has been unable to flush ringbuffer 
> while they can still write into it. This can affect CPU backtrace 
> triggered in panic only able to write into ringbuffer incapable of 
> flushing them.
> 
> Fix the issue by letting the panicked CPU handle the flushing of 
> ringbuffer right after non-panicked CPUs finished writing their
> backtraces.
> 
> Signed-off-by: Ryo Takakura <takakura@xxxxxxxxxxxxx>
> ---
>  kernel/panic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 7e2070925..f94923a63 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -252,8 +252,10 @@ void check_panic_on_warn(const char *origin)
>   */
>  static void panic_other_cpus_shutdown(bool crash_kexec)
>  {
> -	if (panic_print & PANIC_PRINT_ALL_CPU_BT)
> +	if (panic_print & PANIC_PRINT_ALL_CPU_BT) {
>  		trigger_all_cpu_backtrace();
> +		console_flush_on_panic(CONSOLE_FLUSH_PENDING);

Hmm, this is too dangerous.

console_flush_on_panic() is supposed to be called at the end on
panic() as the final desperate attempt to flush consoles.
It does not take console_lock(). It must not be called before
stopping non-panic() CPUs.

We would need to implement something like:

/**
 * console_try_flush - try to flush consoles when safe
 *
 * Context: Any, except for NMI.
 */
void console_try_flush(void)
{
	if (is_printk_legacy_deferred())
		return;

	if (console_trylock())
		console_unlock();
}

, where is_printk_legacy_deferred() is not yet in the mainline. It is a new
API proposed by the last version of a patchset adding adding write_atomic() callback,
see https://lore.kernel.org/all/20240804005138.3722656-24-john.ogness@xxxxxxxxxxxxx/

Best Regards,
Petr