[PATCH v5 0/5] Handle NBCON consoles on KDB

[PATCH v5 0/5] Handle NBCON consoles on KDB

[Marcos Paulo de Souza]

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.

As usual, how I tested the changes:

Testing
-------

I did the tests using qemu and reapplying commit f79b163c4231
('Revert "serial: 8250: Switch to nbcon console"') created originally by
John, just to exercise the common 8250 serial from qemu. The commit can
be checked on [1]. I had to solve some conflicts since the code has been
reworked after the commit was reverted.

Then I would create three different serial entries on qemu:
-serial mon:stdio -serial pty -serial pty

And for the kernel command line I added:
earlyprintk=serial,ttyS2 console=ttyS2 console=ttyS1 console=ttyS1 kgdboc=ttyS1,115200

Without the last patch on this patchset, when KDB is triggered, the mirroring
only worked on the earlyprintk console, since it's using the legacy console.

With the last patch applied, KDB mirroring works on legacy and nbcon
console. For debugging I added some messages to be printed by KDB, showing
also the console->name and console->index, and I was able to see both
->write and ->write_atomic being called, and it all working together.

[1]: https://github.com/marcosps/linux/commit/618bd49f8533db85d9c322f9ad1cb0da22aca9ee
[2]: https://lore.kernel.org/lkml/20250825022947.1596226-1-wangjinchao600@gmail.com/

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Changes in v5:
- Added review tags from Petr
- Changes the way we detect if a CPU is running KDB.
- Link to v4: https://lore.kernel.org/r/20250915-nbcon-kgdboc-v4-0-e2b6753bb566@suse.com

Changes in v4:
- Added ifdefs to only check for KGDB if KGDB was enabled, suggested by John Ogness
- Updated comments about KDB on acquire_direct, suggested by Petr and John
- Added a new patch to export nbcon_write_context_set_buf, suggested by Petr and John
- Link to v3: https://lore.kernel.org/r/20250902-nbcon-kgdboc-v3-0-cd30a8106f1c@suse.com

Changes in v3:
- Only call nbcon_context_release if nbcon_context_exit_unsafe returns true (John Ogness)
- Dropped the prototype of console_is_usable from kernel/printk/internal. (Petr Mladek)
- Add comments to the new functions introduced (Petr Mladek)
- Flush KDB console on nbcon_kdb_release (Petr Mladek)
- Add an exception for KDB on nbcon_context_try_acquire_direct (John Ogness and Petr Mladek)
- Link to v2: https://lore.kernel.org/r/20250811-nbcon-kgdboc-v2-0-c7c72bcdeaf6@suse.com

Changes in v2:
- Set by mistake ..
- Link to v1: https://lore.kernel.org/r/20250713-nbcon-kgdboc-v1-0-51eccd9247a8@suse.com

---
Marcos Paulo de Souza (5):
      printk: nbcon: Export console_is_usable
      printk: nbcon: Introduce KDB helpers
      printk: nbcon: Allow KDB to acquire the NBCON context
      printk: nbcon: Export nbcon_write_context_set_buf
      kdb: Adapt kdb_msg_write to work with NBCON consoles

 include/linux/console.h   | 54 ++++++++++++++++++++++++++++++++++
 include/linux/kdb.h       | 15 ++++++++++
 kernel/debug/kdb/kdb_io.c | 47 ++++++++++++++++++++----------
 kernel/printk/internal.h  | 44 ----------------------------
 kernel/printk/nbcon.c     | 74 +++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 173 insertions(+), 61 deletions(-)
---
base-commit: 39c3d8a3e54a8aeb26de10365318f2c747a8eb25
change-id: 20250713-nbcon-kgdboc-efcfc37fde46

Best regards,
-- 
Marcos Paulo de Souza <mpdesouza@suse.com>

[PATCH v5 1/5] printk: nbcon: Export console_is_usable

[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

[PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

[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

[PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context

[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

[PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf

[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

[PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles

[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

Re: [PATCH v5 1/5] printk: nbcon: Export console_is_usable

[John Ogness]

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

Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

[John Ogness]

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.

> +}

Re: [PATCH v5 3/5] printk: nbcon: Allow KDB to acquire the NBCON context

[John Ogness]

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

Re: [PATCH v5 4/5] printk: nbcon: Export nbcon_write_context_set_buf

[John Ogness]

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

Re: [PATCH v5 5/5] kdb: Adapt kdb_msg_write to work with NBCON consoles

[John Ogness]

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>

Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

[Marcos Paulo de Souza]

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 :)

> 
> > +}

Re: [PATCH v5 0/5] Handle NBCON consoles on KDB

[Daniel Thompson]

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.

Re: [PATCH v5 0/5] Handle NBCON consoles on KDB

[Marcos Paulo de Souza]

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.

Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

[Marcos Paulo de Souza]

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

> 
> > +}

Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

[John Ogness]

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

Re: [PATCH v5 2/5] printk: nbcon: Introduce KDB helpers

[Petr Mladek]

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.
> 
> > +}