* [PATCH 0/2] Handle NBCON consoles on KDB
@ 2025-07-13 5:09 Marcos Paulo de Souza
2025-07-13 5:09 ` [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols Marcos Paulo de Souza
2025-07-13 5:09 ` [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
0 siblings, 2 replies; 8+ messages in thread
From: Marcos Paulo de Souza @ 2025-07-13 5:09 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
After the proposed patches [1], it was suggested to start using
console_is_usable instead of checking if that console is enabled. It was
noticies that KDB was always calling con->write method, but this
callback is not usable for NBCON consoles.
As a first step into making console_is_usable public, I prepared these
two patches that makes public some nbcon functions, and also fixes the
KDB to also handle NBCON consoles.
Thanks to all reviewers of the patches posted on [1]! I hope this is the
first step into implementing all the changes suggested in that patchset.
[1]: https://lore.kernel.org/lkml/20250606-printk-cleanup-part2-v1-0-f427c743dda0@suse.com/
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Marcos Paulo de Souza (2):
printk: nbcon: Export console_is_usage and other nbcon symbols
kdb: Adapt kdb_msg_write to work with NBCON consoles
include/linux/console.h | 48 +++++++++++++++++++++++++++++++++++++++++++++++
kernel/debug/kdb/kdb_io.c | 46 +++++++++++++++++++++++++++++++++++++++++----
kernel/printk/internal.h | 41 ----------------------------------------
kernel/printk/nbcon.c | 4 ++--
4 files changed, 92 insertions(+), 47 deletions(-)
---
base-commit: d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af
change-id: 20250713-nbcon-kgdboc-efcfc37fde46
Best regards,
--
Marcos Paulo de Souza <mpdesouza@suse.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols
2025-07-13 5:09 [PATCH 0/2] Handle NBCON consoles on KDB Marcos Paulo de Souza
@ 2025-07-13 5:09 ` Marcos Paulo de Souza
2025-07-13 6:43 ` kernel test robot
` (2 more replies)
2025-07-13 5:09 ` [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
1 sibling, 3 replies; 8+ messages in thread
From: Marcos Paulo de Souza @ 2025-07-13 5:09 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 nbcon_context_try_acquire, nbcon_context_relase and
console_is_usable are going to be used in the next patch.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
include/linux/console.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
kernel/printk/internal.h | 41 -----------------------------------------
kernel/printk/nbcon.c | 4 ++--
3 files changed, 50 insertions(+), 43 deletions(-)
diff --git a/include/linux/console.h b/include/linux/console.h
index 8f10d0a85bb4536e4b0dda4e8ccbdf87978bbb4a..d5398b4be4f7ce8d1642b919952710431b53a8ad 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -599,19 +599,67 @@ static inline bool console_is_registered(const struct console *con)
hlist_for_each_entry(con, &console_list, node)
#ifdef CONFIG_PRINTK
+extern bool nbcon_context_try_acquire(struct nbcon_context *ctxt);
+extern void nbcon_context_release(struct nbcon_context *ctxt);
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 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 bool nbcon_context_try_acquire(struct nbcon_context *ctxt) { }
+static inline void nbcon_context_release(struct nbcon_context *ctxt) { }
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 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 48a24e7b309db20fdd7419f7aeda68ea7c79fd80..4ad51a98690b43fd2383a0c6e70ac4da68c34da3 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
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index fd12efcc4aeda8883773d9807bc215f6e5cdf71a..b95030018472493b1019932c3d1d92c1abd044ee 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -568,7 +568,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)
+bool nbcon_context_try_acquire(struct nbcon_context *ctxt)
{
unsigned int cpu = smp_processor_id();
struct console *con = ctxt->console;
@@ -648,7 +648,7 @@ static bool nbcon_owner_matches(struct nbcon_state *cur, int expected_cpu,
* nbcon_context_release - Release the console
* @ctxt: The nbcon context from nbcon_context_try_acquire()
*/
-static void nbcon_context_release(struct nbcon_context *ctxt)
+void nbcon_context_release(struct nbcon_context *ctxt)
{
unsigned int cpu = smp_processor_id();
struct console *con = ctxt->console;
--
2.50.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles
2025-07-13 5:09 [PATCH 0/2] Handle NBCON consoles on KDB Marcos Paulo de Souza
2025-07-13 5:09 ` [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols Marcos Paulo de Souza
@ 2025-07-13 5:09 ` Marcos Paulo de Souza
2025-07-13 20:36 ` John Ogness
1 sibling, 1 reply; 8+ messages in thread
From: Marcos Paulo de Souza @ 2025-07-13 5:09 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 the any found console,
but it won't work on NBCON ones. 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.
Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
kernel/debug/kdb/kdb_io.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 4 deletions(-)
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;
+}
+
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;
+ 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;
+ }
+
/*
* 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);
+ } else {
+ c->write(c, msg, msg_len);
+ }
--oops_in_progress;
touch_nmi_watchdog();
}
--
2.50.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols
2025-07-13 5:09 ` [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols Marcos Paulo de Souza
@ 2025-07-13 6:43 ` kernel test robot
2025-07-13 7:04 ` kernel test robot
2025-07-13 20:16 ` John Ogness
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-07-13 6:43 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, John Ogness, Sergey Senozhatsky, Jason Wessel,
Daniel Thompson, Douglas Anderson
Cc: llvm, oe-kbuild-all, linux-kernel, kgdb-bugreport,
Marcos Paulo de Souza
Hi Marcos,
kernel test robot noticed the following build warnings:
[auto build test WARNING on d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af]
url: https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/printk-nbcon-Export-console_is_usage-and-other-nbcon-symbols/20250713-131106
base: d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af
patch link: https://lore.kernel.org/r/20250713-nbcon-kgdboc-v1-1-51eccd9247a8%40suse.com
patch subject: [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols
config: x86_64-buildonly-randconfig-001-20250713 (https://download.01.org/0day-ci/archive/20250713/202507131443.p6jEjXFw-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250713/202507131443.p6jEjXFw-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507131443.p6jEjXFw-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/char/tpm/tpm_tis.c:21:
In file included from include/linux/pnp.h:16:
>> include/linux/console.h:653:76: warning: non-void function does not return a value [-Wreturn-type]
653 | static inline bool nbcon_context_try_acquire(struct nbcon_context *ctxt) { }
| ^
In file included from drivers/char/tpm/tpm_tis.c:29:
In file included from drivers/char/tpm/tpm.h:28:
include/linux/tpm_eventlog.h:167:6: warning: variable 'mapping_size' set but not used [-Wunused-but-set-variable]
167 | int mapping_size;
| ^
2 warnings generated.
vim +653 include/linux/console.h
651
652 #else
> 653 static inline bool nbcon_context_try_acquire(struct nbcon_context *ctxt) { }
654 static inline void nbcon_context_release(struct nbcon_context *ctxt) { }
655 static inline void nbcon_cpu_emergency_enter(void) { }
656 static inline void nbcon_cpu_emergency_exit(void) { }
657 static inline bool nbcon_can_proceed(struct nbcon_write_context *wctxt) { return false; }
658 static inline bool nbcon_enter_unsafe(struct nbcon_write_context *wctxt) { return false; }
659 static inline bool nbcon_exit_unsafe(struct nbcon_write_context *wctxt) { return false; }
660 static inline void nbcon_reacquire_nobuf(struct nbcon_write_context *wctxt) { }
661 static inline bool console_is_usable(struct console *con, short flags,
662 bool use_atomic) { return false; }
663 #endif
664
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols
2025-07-13 5:09 ` [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols Marcos Paulo de Souza
2025-07-13 6:43 ` kernel test robot
@ 2025-07-13 7:04 ` kernel test robot
2025-07-13 20:16 ` John Ogness
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-07-13 7:04 UTC (permalink / raw)
To: Marcos Paulo de Souza, Greg Kroah-Hartman, Petr Mladek,
Steven Rostedt, John Ogness, Sergey Senozhatsky, Jason Wessel,
Daniel Thompson, Douglas Anderson
Cc: oe-kbuild-all, linux-kernel, kgdb-bugreport,
Marcos Paulo de Souza
Hi Marcos,
kernel test robot noticed the following build errors:
[auto build test ERROR on d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af]
url: https://github.com/intel-lab-lkp/linux/commits/Marcos-Paulo-de-Souza/printk-nbcon-Export-console_is_usage-and-other-nbcon-symbols/20250713-131106
base: d0b3b7b22dfa1f4b515fd3a295b3fd958f9e81af
patch link: https://lore.kernel.org/r/20250713-nbcon-kgdboc-v1-1-51eccd9247a8%40suse.com
patch subject: [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols
config: i386-buildonly-randconfig-004-20250713 (https://download.01.org/0day-ci/archive/20250713/202507131412.Lfp8uefP-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250713/202507131412.Lfp8uefP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507131412.Lfp8uefP-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from kernel/panic.c:32:
include/linux/console.h: In function 'nbcon_context_try_acquire':
>> include/linux/console.h:653:53: warning: no return statement in function returning non-void [-Wreturn-type]
653 | static inline bool nbcon_context_try_acquire(struct nbcon_context *ctxt) { }
| ^~~~~~~~~~~~~
--
In file included from kernel/printk/printk.c:26:
include/linux/console.h: In function 'nbcon_context_try_acquire':
>> include/linux/console.h:653:53: warning: no return statement in function returning non-void [-Wreturn-type]
653 | static inline bool nbcon_context_try_acquire(struct nbcon_context *ctxt) { }
| ^~~~~~~~~~~~~
In file included from kernel/printk/printk.c:62:
kernel/printk/internal.h: At top level:
>> kernel/printk/internal.h:165:20: error: redefinition of 'console_is_usable'
165 | static inline bool console_is_usable(struct console *con, short flags,
| ^~~~~~~~~~~~~~~~~
include/linux/console.h:661:20: note: previous definition of 'console_is_usable' with type 'bool(struct console *, short int, bool)' {aka '_Bool(struct console *, short int, _Bool)'}
661 | static inline bool console_is_usable(struct console *con, short flags,
| ^~~~~~~~~~~~~~~~~
vim +/console_is_usable +165 kernel/printk/internal.h
6b93bb41f6eaa1 Thomas Gleixner 2023-09-16 164
6cb58cfebb2932 John Ogness 2024-09-04 @165 static inline bool console_is_usable(struct console *con, short flags,
6cb58cfebb2932 John Ogness 2024-09-04 166 bool use_atomic) { return false; }
864c25c83d834b John Ogness 2024-08-20 167
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols
2025-07-13 5:09 ` [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols Marcos Paulo de Souza
2025-07-13 6:43 ` kernel test robot
2025-07-13 7:04 ` kernel test robot
@ 2025-07-13 20:16 ` John Ogness
2 siblings, 0 replies; 8+ messages in thread
From: John Ogness @ 2025-07-13 20:16 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-07-13, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> Function nbcon_context_try_acquire, nbcon_context_relase and
> console_is_usable are going to be used in the next patch.
The nbcon_context is supposed is not meant to be exposed like this. I
would prefer creating a proper interface rather than having kdb code
directly modifying internal structures. I will provide more details in
the response to the 2nd patch of this series.
John Ogness
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles
2025-07-13 5:09 ` [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
@ 2025-07-13 20:36 ` John Ogness
2025-07-14 2:04 ` Marcos Paulo de Souza
0 siblings, 1 reply; 8+ messages in thread
From: John Ogness @ 2025-07-13 20: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-07-13, Marcos Paulo de Souza <mpdesouza@suse.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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles
2025-07-13 20:36 ` John Ogness
@ 2025-07-14 2:04 ` Marcos Paulo de Souza
0 siblings, 0 replies; 8+ messages in thread
From: Marcos Paulo de Souza @ 2025-07-14 2:04 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 Sun, 2025-07-13 at 22:42 +0206, John Ogness wrote:
> On 2025-07-13, Marcos Paulo de Souza <mpdesouza@suse.com> wrote:
> > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> > index
> > 9b11b10b120cf07e451a7a4d92ce50f9a6c066b2..3b7365c11d06b01d487767fd8
> > 9f1081da10dd2ed 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);
Makes sense John! Thanks for the quick review and suggestion! I'll work
on it in the next few days, but also wait for more people to take a
look as well.
>
> > +}
> > +
> > 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;
Agreed.
> > + }
> > +
> > /*
> > * 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);
Much better and cleaner!
>
> > + } else {
> > + c->write(c, msg, msg_len);
> > + }
> > --oops_in_progress;
> > touch_nmi_watchdog();
> > }
>
Thanks for the quick review!
> John Ogness
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-14 2:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-13 5:09 [PATCH 0/2] Handle NBCON consoles on KDB Marcos Paulo de Souza
2025-07-13 5:09 ` [PATCH 1/2] printk: nbcon: Export console_is_usage and other nbcon symbols Marcos Paulo de Souza
2025-07-13 6:43 ` kernel test robot
2025-07-13 7:04 ` kernel test robot
2025-07-13 20:16 ` John Ogness
2025-07-13 5:09 ` [PATCH 2/2] kdb: Adapt kdb_msg_write to work with NBCON consoles Marcos Paulo de Souza
2025-07-13 20:36 ` John Ogness
2025-07-14 2:04 ` Marcos Paulo de Souza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).