* [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 [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
* [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 [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
all inboxes | Powered by JetHome®