* [PATCH v5 1/5] printk: nbcon: Export console_is_usable
2025-09-30 17:21 [PATCH v5 0/5] Handle NBCON consoles on KDB Marcos Paulo de Souza
@ 2025-09-30 17:21 ` Marcos Paulo de Souza
2025-10-01 14:36 ` John Ogness
2025-09-30 17:21 ` [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers Marcos Paulo de Souza
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-09-30 17:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
The helper will be used on KDB code in the next commits.
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
include/linux/console.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
kernel/printk/internal.h | 44 --------------------------------------------
2 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/include/linux/console.h b/include/linux/console.h
index 8f10d0a85bb4536e4b0dda4e8ccbdf87978bbb4a..67af483574727c00eea1d5a1eacc994755c92607 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -605,6 +605,48 @@ extern bool nbcon_can_proceed(struct nbcon_write_context *wctxt);
extern bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt);
extern bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt);
extern void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt);
+
+/*
+ * Check if the given console is currently capable and allowed to print
+ * records. Note that this function does not consider the current context,
+ * which can also play a role in deciding if @con can be used to print
+ * records.
+ */
+static inline bool console_is_usable(struct console *con, short flags, bool use_atomic)
+{
+ if (!(flags & CON_ENABLED))
+ return false;
+
+ if ((flags & CON_SUSPENDED))
+ return false;
+
+ if (flags & CON_NBCON) {
+ /* The write_atomic() callback is optional. */
+ if (use_atomic && !con->write_atomic)
+ return false;
+
+ /*
+ * For the !use_atomic case, @printk_kthreads_running is not
+ * checked because the write_thread() callback is also used
+ * via the legacy loop when the printer threads are not
+ * available.
+ */
+ } else {
+ if (!con->write)
+ return false;
+ }
+
+ /*
+ * Console drivers may assume that per-cpu resources have been
+ * allocated. So unless they're explicitly marked as being able to
+ * cope (CON_ANYTIME) don't call them until this CPU is officially up.
+ */
+ if (!cpu_online(raw_smp_processor_id()) && !(flags & CON_ANYTIME))
+ return false;
+
+ return true;
+}
+
#else
static inline void nbcon_cpu_emergency_enter(void) { }
static inline void nbcon_cpu_emergency_exit(void) { }
@@ -612,6 +654,8 @@ static inline bool nbcon_can_proceed(struct nbcon_write_context *wctxt) { return
static inline bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt) { return false; }
static inline bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt) { return false; }
static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
+static inline bool console_is_usable(struct console *con, short flags,
+ bool use_atomic) { return false; }
#endif
extern int console_set_on_cmdline;
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index f72bbfa266d6c9bbc533661c40386aa5f0df6c8f..7238da161ff9814fe8a22e4836624e50ee5b71d3 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -112,47 +112,6 @@ bool nbcon_kthread_create(struct console *con);
void nbcon_kthread_stop(struct console *con);
void nbcon_kthreads_wake(void);
-/*
- * Check if the given console is currently capable and allowed to print
- * records. Note that this function does not consider the current context,
- * which can also play a role in deciding if @con can be used to print
- * records.
- */
-static inline bool console_is_usable(struct console *con, short flags, bool use_atomic)
-{
- if (!(flags & CON_ENABLED))
- return false;
-
- if ((flags & CON_SUSPENDED))
- return false;
-
- if (flags & CON_NBCON) {
- /* The write_atomic() callback is optional. */
- if (use_atomic && !con->write_atomic)
- return false;
-
- /*
- * For the !use_atomic case, @printk_kthreads_running is not
- * checked because the write_thread() callback is also used
- * via the legacy loop when the printer threads are not
- * available.
- */
- } else {
- if (!con->write)
- return false;
- }
-
- /*
- * Console drivers may assume that per-cpu resources have been
- * allocated. So unless they're explicitly marked as being able to
- * cope (CON_ANYTIME) don't call them until this CPU is officially up.
- */
- if (!cpu_online(raw_smp_processor_id()) && !(flags & CON_ANYTIME))
- return false;
-
- return true;
-}
-
/**
* nbcon_kthread_wake - Wake up a console printing thread
* @con: Console to operate on
@@ -204,9 +163,6 @@ static inline bool nbcon_legacy_emit_next_record(struct console *con, bool *hand
static inline void nbcon_kthread_wake(struct console *con) { }
static inline void nbcon_kthreads_wake(void) { }
-static inline bool console_is_usable(struct console *con, short flags,
- bool use_atomic) { return false; }
-
#endif /* CONFIG_PRINTK */
extern bool have_boot_console;
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v5 1/5] printk: nbcon: Export console_is_usable
2025-09-30 17:21 ` [PATCH v5 1/5] printk: nbcon: Export console_is_usable Marcos Paulo de Souza
@ 2025-10-01 14:36 ` John Ogness
0 siblings, 0 replies; 17+ messages in thread
From: John Ogness @ 2025-10-01 14:36 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> The helper will be used on KDB code in the next commits.
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> include/linux/console.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> kernel/printk/internal.h | 44 --------------------------------------------
> 2 files changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 8f10d0a85bb4536e4b0dda4e8ccbdf87978bbb4a..67af483574727c00eea1d5a1eacc994755c92607 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -605,6 +605,48 @@ extern bool nbcon_can_proceed(struct nbcon_write_context *wctxt);
> extern bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt);
> extern bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt);
> extern void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt);
> +
> +/*
> + * Check if the given console is currently capable and allowed to print
> + * records. Note that this function does not consider the current context,
> + * which can also play a role in deciding if @con can be used to print
> + * records.
> + */
> +static inline bool console_is_usable(struct console *con, short flags, bool use_atomic)
> +{
> + if (!(flags & CON_ENABLED))
> + return false;
> +
> + if ((flags & CON_SUSPENDED))
> + return false;
> +
> + if (flags & CON_NBCON) {
> + /* The write_atomic() callback is optional. */
> + if (use_atomic && !con->write_atomic)
> + return false;
> +
> + /*
> + * For the !use_atomic case, @printk_kthreads_running is not
> + * checked because the write_thread() callback is also used
> + * via the legacy loop when the printer threads are not
> + * available.
> + */
> + } else {
> + if (!con->write)
> + return false;
> + }
> +
> + /*
> + * Console drivers may assume that per-cpu resources have been
> + * allocated. So unless they're explicitly marked as being able to
> + * cope (CON_ANYTIME) don't call them until this CPU is officially up.
> + */
> + if (!cpu_online(raw_smp_processor_id()) && !(flags & CON_ANYTIME))
> + return false;
> +
> + return true;
> +}
> +
> #else
> static inline void nbcon_cpu_emergency_enter(void) { }
> static inline void nbcon_cpu_emergency_exit(void) { }
> @@ -612,6 +654,8 @@ static inline bool nbcon_can_proceed(struct nbcon_write_context *wctxt) { return
> static inline bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt) { return false; }
> static inline bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt) { return false; }
> static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
> +static inline bool console_is_usable(struct console *con, short flags,
> + bool use_atomic) { return false; }
> #endif
>
> extern int console_set_on_cmdline;
> diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
> index f72bbfa266d6c9bbc533661c40386aa5f0df6c8f..7238da161ff9814fe8a22e4836624e50ee5b71d3 100644
> --- a/kernel/printk/internal.h
> +++ b/kernel/printk/internal.h
> @@ -112,47 +112,6 @@ bool nbcon_kthread_create(struct console *con);
> void nbcon_kthread_stop(struct console *con);
> void nbcon_kthreads_wake(void);
>
> -/*
> - * Check if the given console is currently capable and allowed to print
> - * records. Note that this function does not consider the current context,
> - * which can also play a role in deciding if @con can be used to print
> - * records.
> - */
> -static inline bool console_is_usable(struct console *con, short flags, bool use_atomic)
> -{
> - if (!(flags & CON_ENABLED))
> - return false;
> -
> - if ((flags & CON_SUSPENDED))
> - return false;
> -
> - if (flags & CON_NBCON) {
> - /* The write_atomic() callback is optional. */
> - if (use_atomic && !con->write_atomic)
> - return false;
> -
> - /*
> - * For the !use_atomic case, @printk_kthreads_running is not
> - * checked because the write_thread() callback is also used
> - * via the legacy loop when the printer threads are not
> - * available.
> - */
> - } else {
> - if (!con->write)
> - return false;
> - }
> -
> - /*
> - * Console drivers may assume that per-cpu resources have been
> - * allocated. So unless they're explicitly marked as being able to
> - * cope (CON_ANYTIME) don't call them until this CPU is officially up.
> - */
> - if (!cpu_online(raw_smp_processor_id()) && !(flags & CON_ANYTIME))
> - return false;
> -
> - return true;
> -}
> -
> /**
> * nbcon_kthread_wake - Wake up a console printing thread
> * @con: Console to operate on
> @@ -204,9 +163,6 @@ static inline bool nbcon_legacy_emit_next_record(struct console *con, bool *hand
> static inline void nbcon_kthread_wake(struct console *con) { }
> static inline void nbcon_kthreads_wake(void) { }
>
> -static inline bool console_is_usable(struct console *con, short flags,
> - bool use_atomic) { return false; }
> -
> #endif /* CONFIG_PRINTK */
>
> extern bool have_boot_console;
This also needs the required includes moved over as well. smp.h is
probably more appropriate than the higher level percpu.h:
diff --git a/include/linux/console.h b/include/linux/console.h
index b34c5a0b86303..9406342b27db4 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -19,6 +19,7 @@
#include <linux/irq_work.h>
#include <linux/rculist.h>
#include <linux/rcuwait.h>
+#include <linux/smp.h>
#include <linux/types.h>
#include <linux/vesa.h>
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 077927ed56c5f..7203b7b969c06 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -3,7 +3,6 @@
* internal.h - printk internal definitions
*/
#include <linux/console.h>
-#include <linux/percpu.h>
#include <linux/types.h>
#if defined(CONFIG_PRINTK) && defined(CONFIG_SYSCTL)
On a side note, we are missing <linux/rcuwait.h> in
kernel/printk/internal.h. It currently relies on console.h as a
proxy. But I guess that is out of scope for this series.
John
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers
2025-09-30 17:21 [PATCH v5 0/5] Handle NBCON consoles on KDB Marcos Paulo de Souza
2025-09-30 17:21 ` [PATCH v5 1/5] printk: nbcon: Export console_is_usable Marcos Paulo de Souza
@ 2025-09-30 17:21 ` Marcos Paulo de Souza
2025-10-01 14:56 ` John Ogness
2025-09-30 17:21 ` [PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context Marcos Paulo de Souza
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-09-30 17:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
These helpers will be used when calling console->write_atomic on
KDB code in the next patch. It's basically the same implementation
as nbcon_device_try_acquire, but using NBCON_PRIO_EMERGENCY when
acquiring the context.
If the acquire succeeds, the message and message length are assigned to
nbcon_write_context so ->write_atomic can print the message.
After release try to flush the console since there may be a backlog of
messages in the ringbuffer. The kthread console printers do not get a
chance to run while kdb is active.
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
include/linux/console.h | 6 +++++
kernel/printk/nbcon.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)
diff --git a/include/linux/console.h b/include/linux/console.h
index 67af483574727c00eea1d5a1eacc994755c92607..b34c5a0b86303e2fb4583fa467d8be43761cf756 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -605,6 +605,9 @@ extern bool nbcon_can_proceed(struct nbcon_write_context *wctxt);
extern bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt);
extern bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt);
extern void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt);
+extern bool nbcon_kdb_try_acquire(struct console *con,
+ struct nbcon_write_context *wctxt);
+extern void nbcon_kdb_release(struct nbcon_write_context *wctxt);
/*
* Check if the given console is currently capable and allowed to print
@@ -654,6 +657,9 @@ static inline bool nbcon_can_proceed(struct nbcon_write_context *wctxt) { return
static inline bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt) { return false; }
static inline bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt) { return false; }
static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
+static inline bool nbcon_kdb_try_acquire(struct console *con,
+ struct nbcon_write_context *wctxt) { return false; }
+static inline void nbcon_kdb_release(struct console *con) { }
static inline bool console_is_usable(struct console *con, short flags,
bool use_atomic) { return false; }
#endif
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 558ef31779760340ce42608294d91d5401239f1d..c23abed5933527cb7c6bcc42057fadbb44a43446 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1855,3 +1855,69 @@ void nbcon_device_release(struct console *con)
console_srcu_read_unlock(cookie);
}
EXPORT_SYMBOL_GPL(nbcon_device_release);
+
+/**
+ * nbcon_kdb_try_acquire - Try to acquire nbcon console, enter unsafe
+ * section, and initialized nbcon write context
+ * @con: The nbcon console to acquire
+ * @wctxt: The nbcon write context to be used on success
+ *
+ * Context: Under console_srcu_read_lock() for emiting a single kdb message
+ * using the given con->write_atomic() callback. Can be called
+ * only when the console is usable at the moment.
+ *
+ * Return: True if the console was acquired. False otherwise.
+ *
+ * kdb emits messages on consoles registered for printk() without
+ * storing them into the ring buffer. It has to acquire the console
+ * ownerhip so that it could call con->write_atomic() callback a safe way.
+ *
+ * This function acquires the nbcon console using priority NBCON_PRIO_EMERGENCY
+ * and marks it unsafe for handover/takeover.
+ */
+bool nbcon_kdb_try_acquire(struct console *con,
+ struct nbcon_write_context *wctxt)
+{
+ struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
+
+ memset(ctxt, 0, sizeof(*ctxt));
+ ctxt->console = con;
+ ctxt->prio = NBCON_PRIO_EMERGENCY;
+
+ if (!nbcon_context_try_acquire(ctxt, false))
+ return false;
+
+ if (!nbcon_context_enter_unsafe(ctxt))
+ return false;
+
+ return true;
+}
+
+/**
+ * nbcon_kdb_release - Exit unsafe section and release the nbcon console
+ *
+ * @wctxt: The nbcon write context initialized by a successful
+ * nbcon_kdb_try_acquire()
+ *
+ * Context: Under console_srcu_read_lock() for emiting a single kdb message
+ * using the given con->write_atomic() callback. Can be called
+ * only when the console is usable at the moment.
+ */
+void nbcon_kdb_release(struct nbcon_write_context *wctxt)
+{
+ struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
+
+ if (!nbcon_context_exit_unsafe(ctxt))
+ return;
+
+ nbcon_context_release(ctxt);
+
+ /*
+ * Flush any new printk() messages added when the console was blocked.
+ * Only the console used by the given write context was blocked.
+ * The console was locked only when the write_atomic() callback
+ * was usable.
+ */
+ __nbcon_atomic_flush_pending_con(ctxt->console,
+ prb_next_reserve_seq(prb), false);
+}
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers
2025-09-30 17:21 ` [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers Marcos Paulo de Souza
@ 2025-10-01 14:56 ` John Ogness
2025-10-02 11:23 ` Marcos Paulo de Souza
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: John Ogness @ 2025-10-01 14:56 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 558ef31779760340ce42608294d91d5401239f1d..c23abed5933527cb7c6bcc42057fadbb44a43446 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1855,3 +1855,69 @@ void nbcon_device_release(struct console *con)
> console_srcu_read_unlock(cookie);
> }
> EXPORT_SYMBOL_GPL(nbcon_device_release);
> +
> +/**
> + * nbcon_kdb_try_acquire - Try to acquire nbcon console, enter unsafe
> + * section, and initialized nbcon write context
initialize ---^^^^^^^^^^^
And technically it is not initializing the write context, just the
console ownership context. I'm not sure it is really necessary to
mention that.
> + * @con: The nbcon console to acquire
> + * @wctxt: The nbcon write context to be used on success
> + *
> + * Context: Under console_srcu_read_lock() for emiting a single kdb message
emitting ---^^^^^^^
"./scripts/checkpatch.pl --codespell" is your friend. ;-)
> + * using the given con->write_atomic() callback. Can be called
> + * only when the console is usable at the moment.
> + *
> + * Return: True if the console was acquired. False otherwise.
> + *
> + * kdb emits messages on consoles registered for printk() without
> + * storing them into the ring buffer. It has to acquire the console
> + * ownerhip so that it could call con->write_atomic() callback a safe way.
> + *
> + * This function acquires the nbcon console using priority NBCON_PRIO_EMERGENCY
> + * and marks it unsafe for handover/takeover.
> + */
> +bool nbcon_kdb_try_acquire(struct console *con,
> + struct nbcon_write_context *wctxt)
> +{
> + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +
> + memset(ctxt, 0, sizeof(*ctxt));
> + ctxt->console = con;
> + ctxt->prio = NBCON_PRIO_EMERGENCY;
> +
> + if (!nbcon_context_try_acquire(ctxt, false))
> + return false;
> +
> + if (!nbcon_context_enter_unsafe(ctxt))
> + return false;
> +
> + return true;
> +}
> +
> +/**
> + * nbcon_kdb_release - Exit unsafe section and release the nbcon console
> + *
> + * @wctxt: The nbcon write context initialized by a successful
> + * nbcon_kdb_try_acquire()
> + *
> + * Context: Under console_srcu_read_lock() for emiting a single kdb message
emitting ---^^^^^^^
> + * using the given con->write_atomic() callback. Can be called
> + * only when the console is usable at the moment.
I do not think the "Context" is relevant. It must be called if
a previous call to nbcon_kdb_try_acquire() was successful.
> + */
> +void nbcon_kdb_release(struct nbcon_write_context *wctxt)
> +{
> + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> +
> + if (!nbcon_context_exit_unsafe(ctxt))
> + return;
> +
> + nbcon_context_release(ctxt);
> +
> + /*
> + * Flush any new printk() messages added when the console was blocked.
> + * Only the console used by the given write context was blocked.
> + * The console was locked only when the write_atomic() callback
> + * was usable.
> + */
> + __nbcon_atomic_flush_pending_con(ctxt->console,
> + prb_next_reserve_seq(prb), false);
This can all be one line. 100 characters is the official limit for code.
> +}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers
2025-10-01 14:56 ` John Ogness
@ 2025-10-02 11:23 ` Marcos Paulo de Souza
2025-10-02 19:03 ` Marcos Paulo de Souza
2025-10-06 13:58 ` Petr Mladek
2 siblings, 0 replies; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-10-02 11:23 UTC (permalink / raw)
To: John Ogness, Greg Kroah-Hartman, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport
On Wed, 2025-10-01 at 17:02 +0206, John Ogness wrote:
> On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> > diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> > index
> > 558ef31779760340ce42608294d91d5401239f1d..c23abed5933527cb7c6bcc420
> > 57fadbb44a43446 100644
> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > @@ -1855,3 +1855,69 @@ void nbcon_device_release(struct console
> > *con)
> > console_srcu_read_unlock(cookie);
> > }
> > EXPORT_SYMBOL_GPL(nbcon_device_release);
> > +
> > +/**
> > + * nbcon_kdb_try_acquire - Try to acquire nbcon console, enter
> > unsafe
> > + * section, and initialized nbcon
> > write context
>
> initialize ---^^^^^^^^^^^
>
> And technically it is not initializing the write context, just the
> console ownership context. I'm not sure it is really necessary to
> mention that.
>
> > + * @con: The nbcon console to acquire
> > + * @wctxt: The nbcon write context to be used on success
> > + *
> > + * Context: Under console_srcu_read_lock() for emiting a
> > single kdb message
>
> emitting ---^^^^^^^
>
> "./scripts/checkpatch.pl --codespell" is your friend. ;-)
Thanks! I was relying on b4 prep --check to handle cases like this, but
it seems that it doesn't add --codespell to checkpatch.pl. I'll check
what needs to be done.
>
> > + * using the given con->write_atomic() callback. Can
> > be called
> > + * only when the console is usable at the moment.
> > + *
> > + * Return: True if the console was acquired. False otherwise.
> > + *
> > + * kdb emits messages on consoles registered for printk() without
> > + * storing them into the ring buffer. It has to acquire the
> > console
> > + * ownerhip so that it could call con->write_atomic() callback a
> > safe way.
> > + *
> > + * This function acquires the nbcon console using priority
> > NBCON_PRIO_EMERGENCY
> > + * and marks it unsafe for handover/takeover.
> > + */
> > +bool nbcon_kdb_try_acquire(struct console *con,
> > + struct nbcon_write_context *wctxt)
> > +{
> > + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > + memset(ctxt, 0, sizeof(*ctxt));
> > + ctxt->console = con;
> > + ctxt->prio = NBCON_PRIO_EMERGENCY;
> > +
> > + if (!nbcon_context_try_acquire(ctxt, false))
> > + return false;
> > +
> > + if (!nbcon_context_enter_unsafe(ctxt))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +/**
> > + * nbcon_kdb_release - Exit unsafe section and release the nbcon
> > console
> > + *
> > + * @wctxt: The nbcon write context initialized by a
> > successful
> > + * nbcon_kdb_try_acquire()
> > + *
> > + * Context: Under console_srcu_read_lock() for emiting a
> > single kdb message
>
> emitting ---^^^^^^^
>
> > + * using the given con->write_atomic() callback. Can
> > be called
> > + * only when the console is usable at the moment.
>
> I do not think the "Context" is relevant. It must be called if
> a previous call to nbcon_kdb_try_acquire() was successful.
>
> > + */
> > +void nbcon_kdb_release(struct nbcon_write_context *wctxt)
> > +{
> > + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > + if (!nbcon_context_exit_unsafe(ctxt))
> > + return;
> > +
> > + nbcon_context_release(ctxt);
> > +
> > + /*
> > + * Flush any new printk() messages added when the console
> > was blocked.
> > + * Only the console used by the given write context
> > was blocked.
> > + * The console was locked only when the write_atomic()
> > callback
> > + * was usable.
> > + */
> > + __nbcon_atomic_flush_pending_con(ctxt->console,
> > +
> > prb_next_reserve_seq(prb), false);
>
> This can all be one line. 100 characters is the official limit for
> code.
In the past I sent a patch to another subsystem using more than 80
chars, and the maintainer asked me to break the line at 80. But since
you guys review printk, and this is a printk file, I'll follow your
suggestion for 100 chars in this case :)
>
> > +}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers
2025-10-01 14:56 ` John Ogness
2025-10-02 11:23 ` Marcos Paulo de Souza
@ 2025-10-02 19:03 ` Marcos Paulo de Souza
2025-10-06 8:31 ` John Ogness
2025-10-06 13:58 ` Petr Mladek
2 siblings, 1 reply; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-10-02 19:03 UTC (permalink / raw)
To: John Ogness, Greg Kroah-Hartman, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport
On Wed, 2025-10-01 at 17:02 +0206, John Ogness wrote:
> On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> > diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> > index
> > 558ef31779760340ce42608294d91d5401239f1d..c23abed5933527cb7c6bcc420
> > 57fadbb44a43446 100644
> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > @@ -1855,3 +1855,69 @@ void nbcon_device_release(struct console
> > *con)
> > console_srcu_read_unlock(cookie);
> > }
> > EXPORT_SYMBOL_GPL(nbcon_device_release);
> > +
> > +/**
> > + * nbcon_kdb_try_acquire - Try to acquire nbcon console, enter
> > unsafe
> > + * section, and initialized nbcon
> > write context
>
> initialize ---^^^^^^^^^^^
>
> And technically it is not initializing the write context, just the
> console ownership context. I'm not sure it is really necessary to
> mention that.
>
> > + * @con: The nbcon console to acquire
> > + * @wctxt: The nbcon write context to be used on success
> > + *
> > + * Context: Under console_srcu_read_lock() for emiting a
> > single kdb message
>
> emitting ---^^^^^^^
>
> "./scripts/checkpatch.pl --codespell" is your friend. ;-)
>
> > + * using the given con->write_atomic() callback. Can
> > be called
> > + * only when the console is usable at the moment.
> > + *
> > + * Return: True if the console was acquired. False otherwise.
> > + *
> > + * kdb emits messages on consoles registered for printk() without
> > + * storing them into the ring buffer. It has to acquire the
> > console
> > + * ownerhip so that it could call con->write_atomic() callback a
> > safe way.
> > + *
> > + * This function acquires the nbcon console using priority
> > NBCON_PRIO_EMERGENCY
> > + * and marks it unsafe for handover/takeover.
> > + */
> > +bool nbcon_kdb_try_acquire(struct console *con,
> > + struct nbcon_write_context *wctxt)
> > +{
> > + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > + memset(ctxt, 0, sizeof(*ctxt));
> > + ctxt->console = con;
> > + ctxt->prio = NBCON_PRIO_EMERGENCY;
> > +
> > + if (!nbcon_context_try_acquire(ctxt, false))
> > + return false;
> > +
> > + if (!nbcon_context_enter_unsafe(ctxt))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +/**
> > + * nbcon_kdb_release - Exit unsafe section and release the nbcon
> > console
> > + *
> > + * @wctxt: The nbcon write context initialized by a
> > successful
> > + * nbcon_kdb_try_acquire()
> > + *
> > + * Context: Under console_srcu_read_lock() for emiting a
> > single kdb message
>
> emitting ---^^^^^^^
>
> > + * using the given con->write_atomic() callback. Can
> > be called
> > + * only when the console is usable at the moment.
>
> I do not think the "Context" is relevant. It must be called if
> a previous call to nbcon_kdb_try_acquire() was successful.
>
> > + */
> > +void nbcon_kdb_release(struct nbcon_write_context *wctxt)
> > +{
> > + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > + if (!nbcon_context_exit_unsafe(ctxt))
> > + return;
> > +
> > + nbcon_context_release(ctxt);
> > +
> > + /*
> > + * Flush any new printk() messages added when the console
> > was blocked.
> > + * Only the console used by the given write context
> > was blocked.
> > + * The console was locked only when the write_atomic()
> > callback
> > + * was usable.
> > + */
> > + __nbcon_atomic_flush_pending_con(ctxt->console,
> > +
> > prb_next_reserve_seq(prb), false);
>
> This can all be one line. 100 characters is the official limit for
> code.
I fixed all your suggestions and published my branch here[1]. The only
change that I didn't fixed was about the "Context:" change that you
suggested, since I'll let Petr to thing about it, since he usually has
comments about it.
[1]: https://github.com/marcosps/linux/tree/nbcon-kgdboc-v5
>
> > +}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers
2025-10-02 19:03 ` Marcos Paulo de Souza
@ 2025-10-06 8:31 ` John Ogness
0 siblings, 0 replies; 17+ messages in thread
From: John Ogness @ 2025-10-06 8:31 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport
On 2025-10-02, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> I fixed all your suggestions and published my branch here[1]. The only
> change that I didn't fixed was about the "Context:" change that you
> suggested, since I'll let Petr to thing about it, since he usually has
> comments about it.
>
> [1]: https://github.com/marcosps/linux/tree/nbcon-kgdboc-v5
Thanks. For v6 you can add:
Reviewed-by: John Ogness <john.ogness@linutronix.de>
for the rest of the series (patches 1-4).
John
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers
2025-10-01 14:56 ` John Ogness
2025-10-02 11:23 ` Marcos Paulo de Souza
2025-10-02 19:03 ` Marcos Paulo de Souza
@ 2025-10-06 13:58 ` Petr Mladek
2 siblings, 0 replies; 17+ messages in thread
From: Petr Mladek @ 2025-10-06 13:58 UTC (permalink / raw)
To: John Ogness
Cc: Marcos Paulo de Souza, Greg Kroah-Hartman, Steven Rostedt,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson, linux-kernel, kgdb-bugreport
On Wed 2025-10-01 17:02:41, John Ogness wrote:
> On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> > diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> > index 558ef31779760340ce42608294d91d5401239f1d..c23abed5933527cb7c6bcc42057fadbb44a43446 100644
> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > +/**
> > + * nbcon_kdb_release - Exit unsafe section and release the nbcon console
> > + *
> > + * @wctxt: The nbcon write context initialized by a successful
> > + * nbcon_kdb_try_acquire()
> > + *
> > + * Context: Under console_srcu_read_lock() for emiting a single kdb message
>
> emitting ---^^^^^^^
>
> > + * using the given con->write_atomic() callback. Can be called
> > + * only when the console is usable at the moment.
>
> I do not think the "Context" is relevant. It must be called if
> a previous call to nbcon_kdb_try_acquire() was successful.
Makes sense. I am fine with removing the "Context:" secion completely
from nbcon_kdb_release().
Just to be sure. I think that we do not need to mention that it can
be called only when nbcon_kdb_try_acquire() succeeded. It is kind
of obvious.
Best Regards,
Petr
> > + */
> > +void nbcon_kdb_release(struct nbcon_write_context *wctxt)
> > +{
> > + struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
> > +
> > + if (!nbcon_context_exit_unsafe(ctxt))
> > + return;
> > +
> > + nbcon_context_release(ctxt);
> > +
> > + /*
> > + * Flush any new printk() messages added when the console was blocked.
> > + * Only the console used by the given write context was blocked.
> > + * The console was locked only when the write_atomic() callback
> > + * was usable.
> > + */
> > + __nbcon_atomic_flush_pending_con(ctxt->console,
> > + prb_next_reserve_seq(prb), false);
>
> This can all be one line. 100 characters is the official limit for code.
>
> > +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context
2025-09-30 17:21 [PATCH v5 0/5] Handle NBCON consoles on KDB Marcos Paulo de Souza
2025-09-30 17:21 ` [PATCH v5 1/5] printk: nbcon: Export console_is_usable Marcos Paulo de Souza
2025-09-30 17:21 ` [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers Marcos Paulo de Souza
@ 2025-09-30 17:21 ` Marcos Paulo de Souza
2025-10-01 15:05 ` John Ogness
2025-09-30 17:21 ` [PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf Marcos Paulo de Souza
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-09-30 17:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
KDB can interrupt any console to execute the "mirrored printing" at any
time, so add an exception to nbcon_context_try_acquire_direct to allow
to get the context if the current CPU is the same as kdb_printf_cpu.
This change will be necessary for the next patch, which fixes
kdb_msg_write to work with NBCON consoles by calling ->write_atomic on
such consoles. But to print it first needs to acquire the ownership of
the console, so nbcon_context_try_acquire_direct is fixed here.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
include/linux/kdb.h | 15 +++++++++++++++
kernel/printk/nbcon.c | 6 +++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index ecbf819deeca118f27e98bf71bb37dd27a257ebb..36b46c85733fe1df28cde7760e5c26e96de40c05 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -207,11 +207,26 @@ static inline const char *kdb_walk_kallsyms(loff_t *pos)
/* Dynamic kdb shell command registration */
extern int kdb_register(kdbtab_t *cmd);
extern void kdb_unregister(kdbtab_t *cmd);
+
+/* Return true when KDB as locked for printing a message on this CPU. */
+static inline
+bool kdb_printf_on_this_cpu(void)
+{
+ /*
+ * We can use raw_smp_processor_id() here because the task could
+ * not get migrated when KDB has locked for printing on this CPU.
+ */
+ return unlikely(READ_ONCE(kdb_printf_cpu) == raw_smp_processor_id());
+}
+
#else /* ! CONFIG_KGDB_KDB */
static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; }
static inline void kdb_init(int level) {}
static inline int kdb_register(kdbtab_t *cmd) { return 0; }
static inline void kdb_unregister(kdbtab_t *cmd) {}
+
+static inline bool kdb_printf_on_this_cpu(void) { return false };
+
#endif /* CONFIG_KGDB_KDB */
enum {
KDB_NOT_INITIALIZED,
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index c23abed5933527cb7c6bcc42057fadbb44a43446..6e4641350fe8985438f53bcd32f3adf72d9d6835 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -10,6 +10,7 @@
#include <linux/export.h>
#include <linux/init.h>
#include <linux/irqflags.h>
+#include <linux/kdb.h>
#include <linux/kthread.h>
#include <linux/minmax.h>
#include <linux/panic.h>
@@ -249,13 +250,16 @@ static int nbcon_context_try_acquire_direct(struct nbcon_context *ctxt,
* since all non-panic CPUs are stopped during panic(), it
* is safer to have them avoid gaining console ownership.
*
- * If this acquire is a reacquire (and an unsafe takeover
+ * One exception is when kdb has locked for printing on this CPU.
+ *
+ * Second exception is a reacquire (and an unsafe takeover
* has not previously occurred) then it is allowed to attempt
* a direct acquire in panic. This gives console drivers an
* opportunity to perform any necessary cleanup if they were
* interrupted by the panic CPU while printing.
*/
if (panic_on_other_cpu() &&
+ !kdb_printf_on_this_cpu() &&
(!is_reacquire || cur->unsafe_takeover)) {
return -EPERM;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context
2025-09-30 17:21 ` [PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context Marcos Paulo de Souza
@ 2025-10-01 15:05 ` John Ogness
0 siblings, 0 replies; 17+ messages in thread
From: John Ogness @ 2025-10-01 15:05 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> diff --git a/include/linux/kdb.h b/include/linux/kdb.h
> index ecbf819deeca118f27e98bf71bb37dd27a257ebb..36b46c85733fe1df28cde7760e5c26e96de40c05 100644
> --- a/include/linux/kdb.h
> +++ b/include/linux/kdb.h
> @@ -207,11 +207,26 @@ static inline const char *kdb_walk_kallsyms(loff_t *pos)
> /* Dynamic kdb shell command registration */
> extern int kdb_register(kdbtab_t *cmd);
> extern void kdb_unregister(kdbtab_t *cmd);
> +
> +/* Return true when KDB as locked for printing a message on this CPU. */
> +static inline
> +bool kdb_printf_on_this_cpu(void)
> +{
> + /*
> + * We can use raw_smp_processor_id() here because the task could
> + * not get migrated when KDB has locked for printing on this CPU.
> + */
> + return unlikely(READ_ONCE(kdb_printf_cpu) == raw_smp_processor_id());
> +}
> +
> #else /* ! CONFIG_KGDB_KDB */
> static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; }
> static inline void kdb_init(int level) {}
> static inline int kdb_register(kdbtab_t *cmd) { return 0; }
> static inline void kdb_unregister(kdbtab_t *cmd) {}
> +
> +static inline bool kdb_printf_on_this_cpu(void) { return false };
> +
> #endif /* CONFIG_KGDB_KDB */
> enum {
> KDB_NOT_INITIALIZED,
The include for raw_smp_processor_id() is also needed:
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 36b46c85733fe..db9d73b12a1af 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -14,6 +14,7 @@
*/
#include <linux/list.h>
+#include <linux/smp.h>
/* Shifted versions of the command enable bits are be used if the command
* has no arguments (see kdb_check_flags). This allows commands, such as
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf
2025-09-30 17:21 [PATCH v5 0/5] Handle NBCON consoles on KDB Marcos Paulo de Souza
` (2 preceding siblings ...)
2025-09-30 17:21 ` [PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context Marcos Paulo de Souza
@ 2025-09-30 17:21 ` Marcos Paulo de Souza
2025-10-01 15:08 ` John Ogness
2025-09-30 17:21 ` [PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
2025-10-02 14:01 ` [PATCH v5 0/5] Handle NBCON consoles on KDB Daniel Thompson
5 siblings, 1 reply; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-09-30 17:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
This function will be used in the next patch to allow a driver to set
both the message and message length of a nbcon_write_context. This is
necessary because the function also initializes the ->unsafe_takeover
struct member. By using this helper we ensure that the struct is
initialized correctly.
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
include/linux/console.h | 4 ++++
kernel/printk/nbcon.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/linux/console.h b/include/linux/console.h
index b34c5a0b86303e2fb4583fa467d8be43761cf756..e0fc2608bd9d6a886f5ddc56d26f19b21ae8663d 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -602,6 +602,8 @@ static inline bool console_is_registered(const struct console *con)
extern void nbcon_cpu_emergency_enter(void);
extern void nbcon_cpu_emergency_exit(void);
extern bool nbcon_can_proceed(struct nbcon_write_context *wctxt);
+extern void nbcon_write_context_set_buf(struct nbcon_write_context *wctxt,
+ char *buf, unsigned int len);
extern bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt);
extern bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt);
extern void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt);
@@ -654,6 +656,8 @@ static inline bool console_is_usable(struct console *con, short flags, bool use_
static inline void nbcon_cpu_emergency_enter(void) { }
static inline void nbcon_cpu_emergency_exit(void) { }
static inline bool nbcon_can_proceed(struct nbcon_write_context *wctxt) { return false; }
+static inline void nbcon_write_context_set_buf(struct nbcon_write_context *wctxt,
+ char *buf, unsigned int len) { }
static inline bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt) { return false; }
static inline bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt) { return false; }
static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 6e4641350fe8985438f53bcd32f3adf72d9d6835..2492b14bd272562378c4cdb465eea2269638e127 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -854,7 +854,7 @@ static bool __nbcon_context_update_unsafe(struct nbcon_context *ctxt, bool unsaf
return nbcon_context_can_proceed(ctxt, &cur);
}
-static void nbcon_write_context_set_buf(struct nbcon_write_context *wctxt,
+void nbcon_write_context_set_buf(struct nbcon_write_context *wctxt,
char *buf, unsigned int len)
{
struct nbcon_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf
2025-09-30 17:21 ` [PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf Marcos Paulo de Souza
@ 2025-10-01 15:08 ` John Ogness
0 siblings, 0 replies; 17+ messages in thread
From: John Ogness @ 2025-10-01 15:08 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 6e4641350fe8985438f53bcd32f3adf72d9d6835..2492b14bd272562378c4cdb465eea2269638e127 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -854,7 +854,7 @@ static bool __nbcon_context_update_unsafe(struct nbcon_context *ctxt, bool unsaf
> return nbcon_context_can_proceed(ctxt, &cur);
> }
>
> -static void nbcon_write_context_set_buf(struct nbcon_write_context *wctxt,
> +void nbcon_write_context_set_buf(struct nbcon_write_context *wctxt,
> char *buf, unsigned int len)
Could we also update the whitespace on the 2nd line? And it would all
fit on one line if you want.
John
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles
2025-09-30 17:21 [PATCH v5 0/5] Handle NBCON consoles on KDB Marcos Paulo de Souza
` (3 preceding siblings ...)
2025-09-30 17:21 ` [PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf Marcos Paulo de Souza
@ 2025-09-30 17:21 ` Marcos Paulo de Souza
2025-10-01 15:21 ` John Ogness
2025-10-02 14:01 ` [PATCH v5 0/5] Handle NBCON consoles on KDB Daniel Thompson
5 siblings, 1 reply; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-09-30 17:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
Function kdb_msg_write was calling con->write for any found console,
but it won't work on NBCON consoles. In this case we should acquire the
ownership of the console using NBCON_PRIO_EMERGENCY, since printing
kdb messages should only be interrupted by a panic.
At this point, the console is required to use the atomic callback. The
console is skipped if the write_atomic callback is not set or if the
context could not be acquired. The validation of NBCON is done by the
console_is_usable helper. The context is released right after
write_atomic finishes.
The oops_in_progress handling is only needed in the legacy consoles,
so it was moved around the con->write callback.
Suggested-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
kernel/debug/kdb/kdb_io.c | 47 ++++++++++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 9b11b10b120cf07e451a7a4d92ce50f9a6c066b2..7f41ce5d1b4b2b9970f7c84d8df40d13c9e9a084 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -589,24 +589,41 @@ 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))
+ 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;
- /*
- * Set oops_in_progress to encourage the console drivers to
- * disregard their internal spin locks: in the current calling
- * context the risk of deadlock is a bigger problem than risks
- * due to re-entering the console driver. We operate directly on
- * oops_in_progress rather than using bust_spinlocks() because
- * the calls bust_spinlocks() makes on exit are not appropriate
- * for this calling context.
- */
- ++oops_in_progress;
- c->write(c, msg, msg_len);
- --oops_in_progress;
+
+ if (flags & CON_NBCON) {
+ struct nbcon_write_context wctxt = { };
+
+ /*
+ * Do not continue if the console is NBCON and the context
+ * can't be acquired.
+ */
+ if (!nbcon_kdb_try_acquire(c, &wctxt))
+ continue;
+
+ nbcon_write_context_set_buf(&wctxt, (char *)msg, msg_len);
+
+ c->write_atomic(c, &wctxt);
+ nbcon_kdb_release(&wctxt);
+ } else {
+ /*
+ * Set oops_in_progress to encourage the console drivers to
+ * disregard their internal spin locks: in the current calling
+ * context the risk of deadlock is a bigger problem than risks
+ * due to re-entering the console driver. We operate directly on
+ * oops_in_progress rather than using bust_spinlocks() because
+ * the calls bust_spinlocks() makes on exit are not appropriate
+ * for this calling context.
+ */
+ ++oops_in_progress;
+ c->write(c, msg, msg_len);
+ --oops_in_progress;
+ }
touch_nmi_watchdog();
}
console_srcu_read_unlock(cookie);
--
2.51.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles
2025-09-30 17:21 ` [PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
@ 2025-10-01 15:21 ` John Ogness
0 siblings, 0 replies; 17+ messages in thread
From: John Ogness @ 2025-10-01 15:21 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson
Cc: linux-kernel, kgdb-bugreport, Marcos Paulo de Souza
On 2025-09-30, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> Function kdb_msg_write was calling con->write for any found console,
> but it won't work on NBCON consoles. In this case we should acquire the
> ownership of the console using NBCON_PRIO_EMERGENCY, since printing
> kdb messages should only be interrupted by a panic.
>
> At this point, the console is required to use the atomic callback. The
> console is skipped if the write_atomic callback is not set or if the
> context could not be acquired. The validation of NBCON is done by the
> console_is_usable helper. The context is released right after
> write_atomic finishes.
>
> The oops_in_progress handling is only needed in the legacy consoles,
> so it was moved around the con->write callback.
>
> Suggested-by: Petr Mladek <pmladek@suse.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Reviewed-by: John Ogness <john.ogness@linutronix.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v5 0/5] Handle NBCON consoles on KDB
2025-09-30 17:21 [PATCH v5 0/5] Handle NBCON consoles on KDB Marcos Paulo de Souza
` (4 preceding siblings ...)
2025-09-30 17:21 ` [PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
@ 2025-10-02 14:01 ` Daniel Thompson
2025-10-02 16:20 ` Marcos Paulo de Souza
5 siblings, 1 reply; 17+ messages in thread
From: Daniel Thompson @ 2025-10-02 14:01 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson, linux-kernel, kgdb-bugreport
On Tue, Sep 30, 2025 at 02:21:06PM -0300, Marcos Paulo de Souza wrote:
> In v5 only patch three was changed, changing the check for KDB CPU, as suggested
> by Petr Mladek. Also, it was based on the recent panic API [2], which now sits on
> -mm tree.
Do you keep this work in a git tree anywhere? I wanted to point the kgdb
test suite at it but the patches don't apply cleanly (even after I
grabbed the patches mentioned in [2].
Daniel.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v5 0/5] Handle NBCON consoles on KDB
2025-10-02 14:01 ` [PATCH v5 0/5] Handle NBCON consoles on KDB Daniel Thompson
@ 2025-10-02 16:20 ` Marcos Paulo de Souza
0 siblings, 0 replies; 17+ messages in thread
From: Marcos Paulo de Souza @ 2025-10-02 16:20 UTC (permalink / raw)
To: Daniel Thompson
Cc: Greg Kroah-Hartman, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jason Wessel, Daniel Thompson,
Douglas Anderson, linux-kernel, kgdb-bugreport
githubOn Thu, 2025-10-02 at 15:01 +0100, Daniel Thompson wrote:
> On Tue, Sep 30, 2025 at 02:21:06PM -0300, Marcos Paulo de Souza
> wrote:
> > In v5 only patch three was changed, changing the check for KDB CPU,
> > as suggested
> > by Petr Mladek. Also, it was based on the recent panic API [2],
> > which now sits on
> > -mm tree.
>
> Do you keep this work in a git tree anywhere? I wanted to point the
> kgdb
> test suite at it but the patches don't apply cleanly (even after I
> grabbed the patches mentioned in [2].
Yep... my mistake! I had rebased my patches on top on -mm tree just top
get those patches that I mentioned in [2], but then I rebased back to
master, which had some conflicts...
So, now I just pushed my changes on top on [2], which you can access
here[3]. I also included the revert of the code that converted 8250 to
NBCON, which is easier to test it using qemu, as I presented on the
cover-leter.
Thanks a lot for reviewing and testing the patchset!
[3]: https://github.com/marcosps/linux/tree/nbcon-kgdboc-v5
>
>
> Daniel.
^ permalink raw reply [flat|nested] 17+ messages in thread