All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>,
	"Toshiyuki Sato (Fujitsu)" <fj6611ie@fujitsu.com>
Cc: 'Michael Kelley' <mhklinux@outlook.com>,
	'Ryo Takakura' <ryotkkr98@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: Problem with nbcon console and amba-pl011 serial port
Date: Fri, 06 Jun 2025 12:41:06 +0206	[thread overview]
Message-ID: <84cybhcg4l.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <84frgdcgug.fsf@jogness.linutronix.de>

On 2025-06-06, John Ogness <john.ogness@linutronix.de> wrote:
> On 2025-06-05, Petr Mladek <pmladek@suse.com> wrote:
>> The question is if it is worth it. Is the clean up really important?
>
> I must admit that I am not happy about encouraging the proposed solution
> so far (i.e. expecting driver authors to create special unsafe code in
> the panic situation). It leads down the "hope and pray" path that nbcon
> was designed to fix.
>
> What if during non-panic-CPU shutdown, we allow reacquires to succeed
> only for _direct_ acquires? The below diff shows how this could be
> implemented. Since it only supports direct acquires, it does not violate
> any state rules. And also, since it only involves the reacquire, there
> is no ugly battling for console printing between the panic and non-panic
> CPUs.

Thinking about it some more, since it does not involve printing, why not
just always allow reacquiring directly in panic?

This simplifies the diff significantly.

John

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index d60596777d278..26c229b7f56ea 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -235,7 +235,8 @@ static void nbcon_seq_try_update(struct nbcon_context *ctxt, u64 new_seq)
  *			the handover acquire method.
  */
 static int nbcon_context_try_acquire_direct(struct nbcon_context *ctxt,
-					    struct nbcon_state *cur)
+					    struct nbcon_state *cur,
+					    bool ignore_other_cpu_in_panic)
 {
 	unsigned int cpu = smp_processor_id();
 	struct console *con = ctxt->console;
@@ -249,7 +250,7 @@ static int nbcon_context_try_acquire_direct(struct nbcon_context *ctxt,
 		 * nbcon_waiter_matches(). In particular, the assumption that
 		 * lower priorities are ignored during panic.
 		 */
-		if (other_cpu_in_panic())
+		if (other_cpu_in_panic() && !ignore_other_cpu_in_panic)
 			return -EPERM;
 
 		if (ctxt->prio <= cur->prio || ctxt->prio <= cur->req_prio)
@@ -568,7 +569,7 @@ static struct printk_buffers panic_nbcon_pbufs;
  * in an unsafe state. Otherwise, on success the caller may assume
  * the console is not in an unsafe state.
  */
-static bool nbcon_context_try_acquire(struct nbcon_context *ctxt)
+static bool nbcon_context_try_acquire(struct nbcon_context *ctxt, bool ignore_other_cpu_in_panic)
 {
 	unsigned int cpu = smp_processor_id();
 	struct console *con = ctxt->console;
@@ -577,7 +578,7 @@ static bool nbcon_context_try_acquire(struct nbcon_context *ctxt)
 
 	nbcon_state_read(con, &cur);
 try_again:
-	err = nbcon_context_try_acquire_direct(ctxt, &cur);
+	err = nbcon_context_try_acquire_direct(ctxt, &cur, ignore_other_cpu_in_panic);
 	if (err != -EBUSY)
 		goto out;
 
@@ -913,7 +914,7 @@ void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt)
 {
 	struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
 
-	while (!nbcon_context_try_acquire(ctxt))
+	while (!nbcon_context_try_acquire(ctxt, true))
 		cpu_relax();
 
 	nbcon_write_context_set_buf(wctxt, NULL, 0);
@@ -1101,7 +1102,7 @@ static bool nbcon_emit_one(struct nbcon_write_context *wctxt, bool use_atomic)
 		cant_migrate();
 	}
 
-	if (!nbcon_context_try_acquire(ctxt))
+	if (!nbcon_context_try_acquire(ctxt, false))
 		goto out;
 
 	/*
@@ -1486,7 +1487,7 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
 	ctxt->prio			= nbcon_get_default_prio();
 	ctxt->allow_unsafe_takeover	= allow_unsafe_takeover;
 
-	if (!nbcon_context_try_acquire(ctxt))
+	if (!nbcon_context_try_acquire(ctxt, false))
 		return -EPERM;
 
 	while (nbcon_seq_read(con) < stop_seq) {
@@ -1784,7 +1785,7 @@ bool nbcon_device_try_acquire(struct console *con)
 	ctxt->console	= con;
 	ctxt->prio	= NBCON_PRIO_NORMAL;
 
-	if (!nbcon_context_try_acquire(ctxt))
+	if (!nbcon_context_try_acquire(ctxt, false))
 		return false;
 
 	if (!nbcon_context_enter_unsafe(ctxt))


  reply	other threads:[~2025-06-06 10:47 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03  3:18 Problem with nbcon console and amba-pl011 serial port Michael Kelley
2025-06-03  9:03 ` Ryo Takakura
2025-06-03  9:36 ` Toshiyuki Sato (Fujitsu)
2025-06-03 10:13   ` John Ogness
2025-06-03 10:44     ` John Ogness
2025-06-04  1:22       ` Toshiyuki Sato (Fujitsu)
2025-06-04  7:44         ` John Ogness
2025-06-04  8:11           ` Russell King (Oracle)
2025-06-03 11:09     ` John Ogness
2025-06-04  4:11       ` Toshiyuki Sato (Fujitsu)
2025-06-04  7:52         ` John Ogness
2025-06-04 11:08         ` Petr Mladek
2025-06-04 11:50           ` John Ogness
2025-06-04 13:42             ` Petr Mladek
2025-06-05  5:27               ` Toshiyuki Sato (Fujitsu)
2025-06-05 13:39                 ` Petr Mladek
2025-06-06  6:46                   ` Toshiyuki Sato (Fujitsu)
2025-06-06 10:19                   ` John Ogness
2025-06-06 10:35                     ` John Ogness [this message]
2025-06-06 14:01                     ` Petr Mladek
2025-06-06 16:58                       ` John Ogness
2025-06-05  2:49         ` Michael Kelley
2025-06-05  6:22           ` Toshiyuki Sato (Fujitsu)
2025-06-05  7:42             ` John Ogness
2025-06-09  3:38               ` Michael Kelley

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=84cybhcg4l.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=fj6611ie@fujitsu.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mhklinux@outlook.com \
    --cc=pmladek@suse.com \
    --cc=ryotkkr98@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.