linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).