[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <84ldorx1z2.fsf@jogness.linutronix.de>
Date: Sun, 13 Jul 2025 22:42:17 +0206
From: John Ogness <john.ogness@...utronix.de>
To: Marcos Paulo de Souza <mpdesouza@...e.com>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, Petr Mladek <pmladek@...e.com>, Steven
Rostedt <rostedt@...dmis.org>, Sergey Senozhatsky
<senozhatsky@...omium.org>, Jason Wessel <jason.wessel@...driver.com>,
Daniel Thompson <danielt@...nel.org>, Douglas Anderson
<dianders@...omium.org>
Cc: linux-kernel@...r.kernel.org, kgdb-bugreport@...ts.sourceforge.net,
Marcos Paulo de Souza <mpdesouza@...e.com>
Subject: Re: [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles
On 2025-07-13, Marcos Paulo de Souza <mpdesouza@...e.com> wrote:
> diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> index 9b11b10b120cf07e451a7a4d92ce50f9a6c066b2..3b7365c11d06b01d487767fd89f1081da10dd2ed 100644
> --- a/kernel/debug/kdb/kdb_io.c
> +++ b/kernel/debug/kdb/kdb_io.c
> @@ -558,6 +558,25 @@ static int kdb_search_string(char *searched, char *searchfor)
> return 0;
> }
>
> +static struct nbcon_context *nbcon_acquire_ctxt(struct console *con,
> + struct nbcon_write_context *wctxt,
> + char *msg, int msg_len)
> +{
> + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +
> + ctxt->console = con;
> + ctxt->spinwait_max_us = 0;
> + ctxt->prio = NBCON_PRIO_EMERGENCY;
> + ctxt->allow_unsafe_takeover = false;
> + wctxt->outbuf = msg;
> + wctxt->len = msg_len;
> +
> + if (!nbcon_context_try_acquire(ctxt))
> + return NULL;
> +
> + return ctxt;
This function is grabbing a reference to a private member and returning
it, thus exposing internals. Can we instead create a proper API in
kernel/printk/nbcon.c for kdb?
For example, take a look at:
nbcon_device_try_acquire() and nbcon_device_release()
We could have something similar for kdb, such as:
bool *nbcon_kdb_try_acquire(struct nbcon_write_context *wctxt,
struct console *con, char *msg, int msg_len);
void nbcon_kdb_release(struct nbcon_write_context *wctxt);
> +}
> +
> static void kdb_msg_write(const char *msg, int msg_len)
> {
> struct console *c;
> @@ -589,12 +608,26 @@ static void kdb_msg_write(const char *msg, int msg_len)
> */
> cookie = console_srcu_read_lock();
> for_each_console_srcu(c) {
> - if (!(console_srcu_read_flags(c) & CON_ENABLED))
> + struct nbcon_write_context wctxt = { };
> + struct nbcon_context *ctxt;
With the above suggestion we do not need @ctxt.
> + short flags = console_srcu_read_flags(c);
> +
> + if (!console_is_usable(c, flags, true))
> continue;
> if (c == dbg_io_ops->cons)
> continue;
> - if (!c->write)
> - continue;
> +
> + /*
> + * Do not continue if the console is NBCON and the context
> + * can't be acquired.
> + */
> + if (flags & CON_NBCON) {
> + ctxt = nbcon_acquire_ctxt(c, &wctxt, (char *)msg,
> + msg_len);
> + if (!ctxt)
> + continue;
And this becomes:
if (!nbcon_kdb_try_acquire(&wctxt, c, (char *)msg, msg_len))
continue;
> + }
> +
> /*
> * Set oops_in_progress to encourage the console drivers to
> * disregard their internal spin locks: in the current calling
> @@ -605,7 +638,12 @@ static void kdb_msg_write(const char *msg, int msg_len)
> * for this calling context.
> */
> ++oops_in_progress;
> - c->write(c, msg, msg_len);
> + if (flags & CON_NBCON) {
> + c->write_atomic(c, &wctxt);
> + nbcon_context_release(ctxt);
And this becomes:
nbcon_kdb_release(&wctxt);
> + } else {
> + c->write(c, msg, msg_len);
> + }
> --oops_in_progress;
> touch_nmi_watchdog();
> }
John Ogness
Powered by blists - more mailing lists