mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Breno Leitao <leitao@debian.org>, Petr Mladek <pmladek@suse.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Rio <rioo.tsukatsukii@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1] panic: Flush unsafe consoles before panic reboot
Date: Mon, 02 Nov 2026 23:20:42 +0106	[thread overview]
Message-ID: <87y0ba3mp9.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <auTaBZlNbpm3jgvb@gmail.com>

On 2026-10-30, Breno Leitao <leitao@debian.org> wrote:
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index d0bd2d0cf8990b..d00a8ce5cb618c 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -140,6 +140,13 @@ void __noclone __crash_kexec(struct pt_regs *regs)
>  
>  			crash_setup_regs(&fixed_regs, regs);
>  			crash_save_vmcoreinfo();
> +			/*
> +			 * Flush the unsafe consoles with the other CPUs
> +			 * stopped, before machine_crash_shutdown() tears
> +			 * down what they may need.
> +			 */
> +			crash_smp_send_stop();

I advise against this if it is just for console flushing since it is
changing behavior. If it is generally a good idea, then ok.

> +			nbcon_atomic_unsafe_only_flush();
>  			machine_crash_shutdown(&fixed_regs);
>  			crash_cma_clear_pending_dma();
>  			machine_kexec(kexec_crash_image);
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 91e48d33b7dbbd..e14b1b60828130 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -723,6 +723,9 @@ void vpanic(const char *fmt, va_list args)
>  		panic_console_replay)
>  		console_flush_on_panic(CONSOLE_REPLAY_ALL);
>  
> +	/* The other CPUs are stopped, flush the unsafe consoles too.
> +	nbcon_atomic_unsafe_only_flush();
> +

As Petr mentioned, this is too early. It should be after the final
printk().

>  	if (!panic_blink)
>  		panic_blink = no_blink;
>  
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 5f8afaca886650..a43143eca1a015 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1653,8 +1653,9 @@ static void nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq)
>   * __nbcon_atomic_flush_pending - Flush all nbcon consoles using their
>   *					write_atomic() callback
>   * @stop_seq:			Flush up until this record
> + * @unsafe_only:		Flush only the CON_NBCON_ATOMIC_UNSAFE consoles
>   */
> -static void __nbcon_atomic_flush_pending(u64 stop_seq)
> +static void __nbcon_atomic_flush_pending(u64 stop_seq, bool unsafe_only)
>  {
>  	struct console *con;
>  	int cookie;
> @@ -1666,6 +1667,9 @@ static void __nbcon_atomic_flush_pending(u64 stop_seq)
>  		if (!(flags & CON_NBCON))
>  			continue;
>  
> +		if (unsafe_only && !(flags & CON_NBCON_ATOMIC_UNSAFE))
> +			continue;
> +
>  		if (!console_is_usable(con, flags, true))
>  			continue;
>  
> @@ -1688,7 +1692,7 @@ 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);
>  }
>  
>  /**
> @@ -1701,7 +1705,21 @@ 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;
> +}
> +
> +/**
> + * nbcon_atomic_unsafe_only_flush - Flush only the CON_NBCON_ATOMIC_UNSAFE
> + *	consoles, allowing unsafe hostile takeovers
> + *
> + * Like nbcon_atomic_flush_unsafe(), but consoles with a safe write_atomic()
> + * are left alone. Meant for the panic CPU once the other CPUs are stopped.
> + */
> +void nbcon_atomic_unsafe_only_flush(void)
> +{
> +	panic_nbcon_allow_unsafe_takeover = true;
> +	__nbcon_atomic_flush_pending(prb_next_reserve_seq(prb), true);
>  	panic_nbcon_allow_unsafe_takeover = false;
>  }

The rest looks ok to me.

John

      parent reply	other threads:[~2026-11-02 22:14 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15 10:01 Ryan Roberts
2026-09-15 10:06 ` Ryan Roberts
2026-09-15 18:49   ` Bradley Morgan
2026-09-16  7:44     ` Ryan Roberts
2026-09-16 14:58       ` Bradley Morgan
2026-09-16 13:08 ` Petr Mladek
2026-10-22 20:35   ` Gustavo Luiz Duarte
2026-10-23  7:57   ` John Ogness
2026-10-23 10:36     ` Petr Mladek
2026-10-27 16:12       ` Breno Leitao
2026-10-27 17:43         ` Breno Leitao
2026-10-27 18:01           ` John Ogness
2026-10-28 11:24             ` Breno Leitao
2026-10-28 10:52     ` Breno Leitao
2026-10-28 14:37       ` John Ogness
2026-10-28 21:15         ` Breno Leitao
2026-10-29  9:20           ` John Ogness
2026-10-29 10:58             ` Breno Leitao
2026-10-29 13:53               ` John Ogness
2026-10-29 14:37                 ` Petr Mladek
2026-10-30 17:56                   ` Breno Leitao
2026-11-02 10:28                     ` Petr Mladek
2026-11-02 13:27                       ` Breno Leitao
2026-11-02 22:22                       ` John Ogness
2026-11-02 22:14                     ` John Ogness [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y0ba3mp9.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rioo.tsukatsukii@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=ryan.roberts@arm.com \
    --cc=senozhatsky@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®