Re: Problem with nbcon console and amba-pl011 serial port

John Ogness
2025-06-06T10:35:08Z

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))