[PATCH v2 0/4] printk cleanup - part 2

[PATCH v2 0/4] printk cleanup - part 2

[Marcos Paulo de Souza]

The first part can be found here[1]. The proposed changes do not
change the functionality of printk, but were suggestions made by
Petr Mladek. I already have more patches for a part 3 ,but I would like
to see these ones merged first.

I did the testing with VMs, checking suspend and resume cycles, and it worked
as expected.

Thanks for reviewing!

[1]: https://lore.kernel.org/lkml/20250226-printk-renaming-v1-0-0b878577f2e6@suse.com/

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Changes in v2:
- Squashed patches 1 and 3 (CON_SUSPEND usage) and now is the last patch
  of the series, suggested by Petr Mladek
- Moved commit 4 as the first one in the series, and it was changed to
  use console_is_usable helper, suggested by Petr Mladek
- Moved commit 5 as the second commit in the series, and adjusted to use
  console_is_usable helper, suggested by Petr Mladek
- The patch 6 was dropped, since it was implemented in a different patchset
  (https://lore.kernel.org/lkml/20250902-nbcon-kgdboc-v3-0-cd30a8106f1c@suse.com/)
- Patch 7 was moved as third patch, and is using the console_is_usable,
  suggested by Petr Mladek
- Patch 2 was dropped from this patchset, and will be included in the
  next cleanup patchset.
- Link to v1: https://lore.kernel.org/r/20250606-printk-cleanup-part2-v1-0-f427c743dda0@suse.com

---
Marcos Paulo de Souza (4):
      drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT
      arch: um: kmsg_dump: Use console_is_usable
      printk: Use console_is_usable on console_unblank
      printk: Make console_{suspend,resume} handle CON_SUSPENDED

 arch/um/kernel/kmsg_dump.c  |  2 +-
 drivers/tty/serial/kgdboc.c |  1 -
 drivers/tty/tty_io.c        |  2 +-
 kernel/printk/printk.c      | 17 +++++++----------
 4 files changed, 9 insertions(+), 13 deletions(-)
---
base-commit: 887c7f05d40eb51ba3f38fd71d5e6b4aff4bb8a2
change-id: 20250601-printk-cleanup-part2-38f8d5108099

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

[PATCH v2 1/4] drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT

[Marcos Paulo de Souza]

The original code tried to find a console that has CON_BOOT _or_
CON_ENABLED flag set. The flag CON_ENABLED is set to all registered
consoles, so in this case this check is always true, even for the
CON_BOOT consoles.

The initial intent of the kgdboc_earlycon_init was to get a console
early (CON_BOOT) or later on in the process (CON_ENABLED). The
code was using for_each_console macro, meaning that all console structs
were previously registered on the printk() machinery. At this point,
any console found on for_each_console is safe for kgdboc_earlycon_init
to use.

Dropping the check makes the code cleaner, and avoids further confusion
by future readers of the code.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 drivers/tty/serial/kgdboc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 85f6c5a76e0f..5a955c80a853 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -577,7 +577,6 @@ static int __init kgdboc_earlycon_init(char *opt)
 	console_list_lock();
 	for_each_console(con) {
 		if (con->write && con->read &&
-		    (con->flags & (CON_BOOT | CON_ENABLED)) &&
 		    (!opt || !opt[0] || strcmp(con->name, opt) == 0))
 			break;
 	}

-- 
2.51.1

[PATCH v2 2/4] arch: um: kmsg_dump: Use console_is_usable

[Marcos Paulo de Souza]

All consoles found on for_each_console are registered, meaning that all
of them have the CON_ENABLED flag set. Since NBCON was introduced it's
important to check if a given console also implements the NBCON callbacks.
The function console_is_usable does exactly that.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 arch/um/kernel/kmsg_dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index 419021175272..fc0f543d1d8e 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -31,7 +31,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
 		 * expected to output the crash information.
 		 */
 		if (strcmp(con->name, "ttynull") != 0 &&
-		    (console_srcu_read_flags(con) & CON_ENABLED)) {
+		    console_is_usable(con, console_srcu_read_flags(con), true)) {
 			break;
 		}
 	}

-- 
2.51.1

[PATCH v2 3/4] printk: Use console_is_usable on console_unblank

[Marcos Paulo de Souza]

The macro for_each_console_srcu iterates over all registered consoles. It's
implied that all registered consoles have CON_ENABLED flag set, making
the check for the flag unnecessary. Call console_is_usable function to
fully verify if the given console is usable before calling the ->unblank
callback.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 kernel/printk/printk.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index cb79d1d2e6e5..fed98a18e830 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3374,12 +3374,10 @@ void console_unblank(void)
 	 */
 	cookie = console_srcu_read_lock();
 	for_each_console_srcu(c) {
-		short flags = console_srcu_read_flags(c);
-
-		if (flags & CON_SUSPENDED)
+		if (!console_is_usable(c, console_srcu_read_flags(c), true))
 			continue;
 
-		if ((flags & CON_ENABLED) && c->unblank) {
+		if (c->unblank) {
 			found_unblank = true;
 			break;
 		}
@@ -3416,12 +3414,10 @@ void console_unblank(void)
 
 	cookie = console_srcu_read_lock();
 	for_each_console_srcu(c) {
-		short flags = console_srcu_read_flags(c);
-
-		if (flags & CON_SUSPENDED)
+		if (!console_is_usable(c, console_srcu_read_flags(c), true))
 			continue;
 
-		if ((flags & CON_ENABLED) && c->unblank)
+		if (c->unblank)
 			c->unblank();
 	}
 	console_srcu_read_unlock(cookie);

-- 
2.51.1

[PATCH v2 4/4] printk: Make console_{suspend,resume} handle CON_SUSPENDED

[Marcos Paulo de Souza]

Since commit 9e70a5e109a4 ("printk: Add per-console suspended state")
the CON_SUSPENDED flag was introced, and this flag was being checked
on console_is_usable function, which returns false if the console is
suspended.

To make the behavior consistent, change show_cons_active to look for
consoles that are not suspended, instead of checking CON_ENABLED.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 drivers/tty/tty_io.c   | 2 +-
 kernel/printk/printk.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e2d92cf70eb7..1b2ce0f36010 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3554,7 +3554,7 @@ static ssize_t show_cons_active(struct device *dev,
 			continue;
 		if (!(c->flags & CON_NBCON) && !c->write)
 			continue;
-		if ((c->flags & CON_ENABLED) == 0)
+		if (c->flags & CON_SUSPENDED)
 			continue;
 		cs[i++] = c;
 		if (i >= ARRAY_SIZE(cs))
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fed98a18e830..fe7c956f73bd 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3542,7 +3542,7 @@ void console_suspend(struct console *console)
 {
 	__pr_flush(console, 1000, true);
 	console_list_lock();
-	console_srcu_write_flags(console, console->flags & ~CON_ENABLED);
+	console_srcu_write_flags(console, console->flags | CON_SUSPENDED);
 	console_list_unlock();
 
 	/*
@@ -3555,13 +3555,14 @@ void console_suspend(struct console *console)
 }
 EXPORT_SYMBOL(console_suspend);
 
+/* Unset CON_SUSPENDED flag so the console can start printing again. */
 void console_resume(struct console *console)
 {
 	struct console_flush_type ft;
 	bool is_nbcon;
 
 	console_list_lock();
-	console_srcu_write_flags(console, console->flags | CON_ENABLED);
+	console_srcu_write_flags(console, console->flags & ~CON_SUSPENDED);
 	is_nbcon = console->flags & CON_NBCON;
 	console_list_unlock();
 

-- 
2.51.1

Re: [PATCH v2 1/4] drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT

[Petr Mladek]

On Fri 2025-11-21 15:50:33, Marcos Paulo de Souza wrote:
> The original code tried to find a console that has CON_BOOT _or_
> CON_ENABLED flag set. The flag CON_ENABLED is set to all registered
> consoles, so in this case this check is always true, even for the
> CON_BOOT consoles.
> 
> The initial intent of the kgdboc_earlycon_init was to get a console
> early (CON_BOOT) or later on in the process (CON_ENABLED). The
> code was using for_each_console macro, meaning that all console structs
> were previously registered on the printk() machinery. At this point,
> any console found on for_each_console is safe for kgdboc_earlycon_init
> to use.
> 
> Dropping the check makes the code cleaner, and avoids further confusion
> by future readers of the code.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>

I agree that the check is superfluous and can be removed:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v2 2/4] arch: um: kmsg_dump: Use console_is_usable

[Petr Mladek]

On Fri 2025-11-21 15:50:34, Marcos Paulo de Souza wrote:
> All consoles found on for_each_console are registered, meaning that all
> of them have the CON_ENABLED flag set. Since NBCON was introduced it's
> important to check if a given console also implements the NBCON callbacks.
> The function console_is_usable does exactly that.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>

Makes sense:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v2 3/4] printk: Use console_is_usable on console_unblank

[Petr Mladek]

On Fri 2025-11-21 15:50:35, Marcos Paulo de Souza wrote:
> The macro for_each_console_srcu iterates over all registered consoles. It's
> implied that all registered consoles have CON_ENABLED flag set, making
> the check for the flag unnecessary. Call console_is_usable function to
> fully verify if the given console is usable before calling the ->unblank
> callback.
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>

Makes sense:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v2 4/4] printk: Make console_{suspend,resume} handle CON_SUSPENDED

[Petr Mladek]

On Fri 2025-11-21 15:50:36, Marcos Paulo de Souza wrote:
> Since commit 9e70a5e109a4 ("printk: Add per-console suspended state")
> the CON_SUSPENDED flag was introced, and this flag was being checked
> on console_is_usable function, which returns false if the console is
> suspended.
> 
> To make the behavior consistent, change show_cons_active to look for
> consoles that are not suspended, instead of checking CON_ENABLED.
> 
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -3554,7 +3554,7 @@ static ssize_t show_cons_active(struct device *dev,
>  			continue;
>  		if (!(c->flags & CON_NBCON) && !c->write)
>  			continue;
> -		if ((c->flags & CON_ENABLED) == 0)
> +		if (c->flags & CON_SUSPENDED)

I believe that we could and should replace

		if (!(c->flags & CON_NBCON) && !c->write)
			continue;
		if (c->flags & CON_SUSPENDED)
			continue;

with

		if (!console_is_usable(c, c->flags, true) &&
		    !console_is_usable(c, c->flags, false))
			continue;

It would make the value compatible with all other callers/users of
the console drivers.

The variant using two console_is_usable() calls with "true/false"
parameters is inspited by __pr_flush().

>  			continue;
>  		cs[i++] = c;
>  		if (i >= ARRAY_SIZE(cs))
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fed98a18e830..fe7c956f73bd 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3542,7 +3542,7 @@ void console_suspend(struct console *console)
>  {
>  	__pr_flush(console, 1000, true);
>  	console_list_lock();
> -	console_srcu_write_flags(console, console->flags & ~CON_ENABLED);
> +	console_srcu_write_flags(console, console->flags | CON_SUSPENDED);

This is the same flag which is set also by the console_suspend_all()
API. Now, as discussed at
https://lore.kernel.org/lkml/844j4lepak.fsf@jogness.linutronix.de/

   + console_suspend()/console_resume() API is used by few console
     drivers to suspend the console when the related HW device
     gets suspended.

   + console_suspend_all()/console_resume_all() is used by
     the power management subsystem to call down/up all consoles
     when the system is going down/up. It is a big hammer approach.

We need to distinguish the two APIs so that console drivers which were
suspended by both APIs stay suspended until they get resumed by both
APIs. I mean:

	// This should suspend all consoles unless it is not disabled
	// by "no_console_suspend" API.
	console_suspend_all();
	// This suspends @con even when "no_console_suspend" parameter
	// is used. It is needed because the HW is going to be suspended.
	// It has no effect when the consoles were already suspended
	// by the big hammer API.
	console_suspend(con);

	// This might resume the console when "no_console_suspend" option
	// is used. The driver should work because the HW was resumed.
	// But it should stay suspended when all consoles are supposed
	// to stay suspended because of the big hammer API.
	console_resume(con);
	// This should resume all consoles.
	console_resume_all();

Other behavior would be unexpected and untested. It might cause regression.

I see two solutions:

   + add another CON_SUSPENDED_ALL flag
   + add back "consoles_suspended" global variable

I prefer adding back the "consoles_suspended" global variable because
it is a global state...

The global state should be synchronized the same way as the current
per-console flag (write under console_list_lock, read under
console_srcu_read_lock()).

Also it should be checked by console_is_usable() API. Otherwise, we
would need to update all callers.

This brings a challenge how to make it safe and keep the API sane.
I propose to create:

  + __console_is_usable() where the "consoles_suspended" value will
    be passed as parameter. It might be used directly under
    console_list_lock().

  + console_is_usable() with the existing parameters. It will check
    the it was called under console_srcu_read_lock, read
    the global "consoles_suspend" and pass it to
    __console_is_usable().


>  	console_list_unlock();
>  
>  	/*

I played with the code to make sure that it looked sane
and I ended with the following changes on top of this patch.

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 1b2ce0f36010..fda4683d12f1 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3552,9 +3552,8 @@ static ssize_t show_cons_active(struct device *dev,
 	for_each_console(c) {
 		if (!c->device)
 			continue;
-		if (!(c->flags & CON_NBCON) && !c->write)
-			continue;
-		if (c->flags & CON_SUSPENDED)
+		if (!__console_is_usable(c, c->flags, consoles_suspended, true) &&
+		    !__console_is_usable(c, c->flags, consoles_suspended, false))
 			continue;
 		cs[i++] = c;
 		if (i >= ARRAY_SIZE(cs))
diff --git a/include/linux/console.h b/include/linux/console.h
index 5f17321ed962..090490ef570f 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -496,6 +496,7 @@ extern void console_list_lock(void) __acquires(console_mutex);
 extern void console_list_unlock(void) __releases(console_mutex);
 
 extern struct hlist_head console_list;
+extern bool consoles_suspended;
 
 /**
  * console_srcu_read_flags - Locklessly read flags of a possibly registered
@@ -548,6 +549,47 @@ static inline void console_srcu_write_flags(struct console *con, short flags)
 	WRITE_ONCE(con->flags, flags);
 }
 
+/**
+ * consoles_suspended_srcu_read - Locklessly read the global flag for
+ *				suspending all consoles.
+ *
+ * The global "consoles_suspended" flag is synchronized using console_list_lock
+ * and console_srcu_read_lock. It is the same approach as CON_SUSSPENDED flag.
+ * See console_srcu_read_flags() for more details.
+ *
+ * Context: Any context.
+ * Return: The current value of the global "consoles_suspended" flag.
+ */
+static inline short consoles_suspended_srcu_read(void)
+{
+	WARN_ON_ONCE(!console_srcu_read_lock_is_held());
+
+	/*
+	 * The READ_ONCE() matches the WRITE_ONCE() when "consoles_suspended"
+	 * is modified with consoles_suspended_srcu_write().
+	 */
+	return data_race(READ_ONCE(consoles_suspended));
+}
+
+/**
+ * consoles_suspended_srcu_write - Write the global flag for suspending
+ *			all consoles.
+ * @suspend:	new value to write
+ *
+ * The write must be done under the console_list_lock. The caller is responsible
+ * for calling synchronize_srcu() to make sure that all callers checking the
+ * usablility of registered consoles see the new state.
+ *
+ * Context: Any context.
+ */
+static inline void consoles_suspended_srcu_write(bool suspend)
+{
+	lockdep_assert_console_list_lock_held();
+
+	/* This matches the READ_ONCE() in consoles_suspended_srcu_read(). */
+	WRITE_ONCE(consoles_suspended, suspend);
+}
+
 /* Variant of console_is_registered() when the console_list_lock is held. */
 static inline bool console_is_registered_locked(const struct console *con)
 {
@@ -617,13 +659,15 @@ extern bool nbcon_kdb_try_acquire(struct console *con,
 extern void nbcon_kdb_release(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.
+ * This variant might be called under console_list_lock where both
+ * @flags and @all_suspended flags can be read directly.
  */
-static inline bool console_is_usable(struct console *con, short flags, bool use_atomic)
+static inline bool __console_is_usable(struct console *con, short flags,
+				       bool all_suspended, bool use_atomic)
 {
+	if (all_suspended)
+		return false;
+
 	if (!(flags & CON_ENABLED))
 		return false;
 
@@ -666,6 +710,20 @@ static inline bool console_is_usable(struct console *con, short flags, bool use_
 	return true;
 }
 
+/*
+ * 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)
+{
+	bool all_suspended = consoles_suspended_srcu_read();
+
+	return __console_is_usable(con, flags, all_suspended, use_atomic);
+}
+
 #else
 static inline void nbcon_cpu_emergency_enter(void) { }
 static inline void nbcon_cpu_emergency_exit(void) { }
@@ -678,6 +736,8 @@ 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 nbcon_write_context *wctxt) { }
+static inline bool __console_is_usable(struct console *con, short flags,
+				       bool all_suspended, bool use_atomic) { return false; }
 static inline bool console_is_usable(struct console *con, short flags,
 				     bool use_atomic) { return false; }
 #endif
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 23a14e8c7a49..12247df07420 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -104,6 +104,13 @@ DEFINE_STATIC_SRCU(console_srcu);
  */
 int __read_mostly suppress_printk;
 
+/*
+ * Global flag for calling down all consoles during suspend.
+ * There is also a per-console flag which is used when the related
+ * device HW gets disabled, see CON_SUSPEND.
+ */
+bool consoles_suspended;
+
 #ifdef CONFIG_LOCKDEP
 static struct lockdep_map console_lock_dep_map = {
 	.name = "console_lock"
@@ -2731,8 +2738,6 @@ MODULE_PARM_DESC(console_no_auto_verbose, "Disable console loglevel raise to hig
  */
 void console_suspend_all(void)
 {
-	struct console *con;
-
 	if (console_suspend_enabled)
 		pr_info("Suspending console(s) (use no_console_suspend to debug)\n");
 
@@ -2749,8 +2754,7 @@ void console_suspend_all(void)
 		return;
 
 	console_list_lock();
-	for_each_console(con)
-		console_srcu_write_flags(con, con->flags | CON_SUSPENDED);
+	consoles_suspended_srcu_write(true);
 	console_list_unlock();
 
 	/*
@@ -2765,7 +2769,6 @@ void console_suspend_all(void)
 void console_resume_all(void)
 {
 	struct console_flush_type ft;
-	struct console *con;
 
 	/*
 	 * Allow queueing irq_work. After restoring console state, deferred
@@ -2776,8 +2779,7 @@ void console_resume_all(void)
 
 	if (console_suspend_enabled) {
 		console_list_lock();
-		for_each_console(con)
-			console_srcu_write_flags(con, con->flags & ~CON_SUSPENDED);
+		consoles_suspended_srcu_write(false);
 		console_list_unlock();
 
 		/*

Best Regards,
Petr

Re: [PATCH v2 0/4] printk cleanup - part 2

[Petr Mladek]

On Fri 2025-11-21 15:50:32, Marcos Paulo de Souza wrote:
> The first part can be found here[1]. The proposed changes do not
> change the functionality of printk, but were suggestions made by
> Petr Mladek. I already have more patches for a part 3 ,but I would like
> to see these ones merged first.
> 
> I did the testing with VMs, checking suspend and resume cycles, and it worked
> as expected.
> 
> Thanks for reviewing!

> Marcos Paulo de Souza (4):
>       drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT
>       arch: um: kmsg_dump: Use console_is_usable
>       printk: Use console_is_usable on console_unblank

These three patches were simple, straightforward, and ready for linux
next.

I have comitted them into printk/linux.git, branch rework/nbcon-in-kdb.
I am going to push them for 6.19.

>       printk: Make console_{suspend,resume} handle CON_SUSPENDED

This patch still need some love and v3.

Best Regards,
Petr

Re: [PATCH v2 0/4] printk cleanup - part 2

[Daniel Thompson]

On Thu, Nov 27, 2025 at 04:18:40PM +0100, Petr Mladek wrote:
> On Fri 2025-11-21 15:50:32, Marcos Paulo de Souza wrote:
> > The first part can be found here[1]. The proposed changes do not
> > change the functionality of printk, but were suggestions made by
> > Petr Mladek. I already have more patches for a part 3 ,but I would like
> > to see these ones merged first.
> >
> > I did the testing with VMs, checking suspend and resume cycles, and it worked
> > as expected.
> >
> > Thanks for reviewing!
>
> > Marcos Paulo de Souza (4):
> >       drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT
> >       arch: um: kmsg_dump: Use console_is_usable
> >       printk: Use console_is_usable on console_unblank
>
> These three patches were simple, straightforward, and ready for linux
> next.
>
> I have comitted them into printk/linux.git, branch rework/nbcon-in-kdb.
> I am going to push them for 6.19.

I pointed the kgdb test suite at this branch (as I did for the earlier
part of the patchset, although I think I forgot to post about it).

The console coverage is fairly modest (I think just 8250 and PL011
drivers, with and without earlycon) and the suite exercises features
rather than crash resilience. Nevertheless and FWIW, the tests didn't
pick up any regressions. Yay!


Daniel.

Re: [PATCH v2 0/4] printk cleanup - part 2

[Petr Mladek]

On Fri 2025-11-28 09:52:24, Daniel Thompson wrote:
> On Thu, Nov 27, 2025 at 04:18:40PM +0100, Petr Mladek wrote:
> > On Fri 2025-11-21 15:50:32, Marcos Paulo de Souza wrote:
> > > The first part can be found here[1]. The proposed changes do not
> > > change the functionality of printk, but were suggestions made by
> > > Petr Mladek. I already have more patches for a part 3 ,but I would like
> > > to see these ones merged first.
> > >
> > > I did the testing with VMs, checking suspend and resume cycles, and it worked
> > > as expected.
> > >
> > > Thanks for reviewing!
> >
> > > Marcos Paulo de Souza (4):
> > >       drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT
> > >       arch: um: kmsg_dump: Use console_is_usable
> > >       printk: Use console_is_usable on console_unblank
> >
> > These three patches were simple, straightforward, and ready for linux
> > next.
> >
> > I have comitted them into printk/linux.git, branch rework/nbcon-in-kdb.
> > I am going to push them for 6.19.
> 
> I pointed the kgdb test suite at this branch (as I did for the earlier
> part of the patchset, although I think I forgot to post about it).
> 
> The console coverage is fairly modest (I think just 8250 and PL011
> drivers, with and without earlycon) and the suite exercises features
> rather than crash resilience. Nevertheless and FWIW, the tests didn't
> pick up any regressions. Yay!

Great news! Thanks a lot for doing the test and sharing results.

Best Regards,
Petr

Re: [PATCH v2 0/4] printk cleanup - part 2

[Marcos Paulo de Souza]

On Fri, 2025-11-28 at 09:52 +0000, Daniel Thompson wrote:
> On Thu, Nov 27, 2025 at 04:18:40PM +0100, Petr Mladek wrote:
> > On Fri 2025-11-21 15:50:32, Marcos Paulo de Souza wrote:
> > > The first part can be found here[1]. The proposed changes do not
> > > change the functionality of printk, but were suggestions made by
> > > Petr Mladek. I already have more patches for a part 3 ,but I
> > > would like
> > > to see these ones merged first.
> > > 
> > > I did the testing with VMs, checking suspend and resume cycles,
> > > and it worked
> > > as expected.
> > > 
> > > Thanks for reviewing!
> > 
> > > Marcos Paulo de Souza (4):
> > >       drivers: serial: kgdboc: Drop checks for CON_ENABLED and
> > > CON_BOOT
> > >       arch: um: kmsg_dump: Use console_is_usable
> > >       printk: Use console_is_usable on console_unblank
> > 
> > These three patches were simple, straightforward, and ready for
> > linux
> > next.
> > 
> > I have comitted them into printk/linux.git, branch rework/nbcon-in-
> > kdb.
> > I am going to push them for 6.19.
> 
> I pointed the kgdb test suite at this branch (as I did for the
> earlier
> part of the patchset, although I think I forgot to post about it).
> 
> The console coverage is fairly modest (I think just 8250 and PL011
> drivers, with and without earlycon) and the suite exercises features
> rather than crash resilience. Nevertheless and FWIW, the tests didn't
> pick up any regressions. Yay!

Thanks Daniel! I remember that you said that you would run the
testsuite in the previous patchset, but didn't want to bother asking
you (I believe that if you found anything you would point it out either
way :) ).

> 
> 
> Daniel.

Re: [PATCH v2 4/4] printk: Make console_{suspend,resume} handle CON_SUSPENDED

[Marcos Paulo de Souza]

On Thu, 2025-11-27 at 10:49 +0100, Petr Mladek wrote:
> On Fri 2025-11-21 15:50:36, Marcos Paulo de Souza wrote:
> > Since commit 9e70a5e109a4 ("printk: Add per-console suspended
> > state")
> > the CON_SUSPENDED flag was introced, and this flag was being
> > checked
> > on console_is_usable function, which returns false if the console
> > is
> > suspended.
> > 
> > To make the behavior consistent, change show_cons_active to look
> > for
> > consoles that are not suspended, instead of checking CON_ENABLED.
> > 
> > --- a/drivers/tty/tty_io.c
> > +++ b/drivers/tty/tty_io.c
> > @@ -3554,7 +3554,7 @@ static ssize_t show_cons_active(struct device
> > *dev,
> >  			continue;
> >  		if (!(c->flags & CON_NBCON) && !c->write)
> >  			continue;
> > -		if ((c->flags & CON_ENABLED) == 0)
> > +		if (c->flags & CON_SUSPENDED)
> 
> I believe that we could and should replace
> 
> 		if (!(c->flags & CON_NBCON) && !c->write)
> 			continue;
> 		if (c->flags & CON_SUSPENDED)
> 			continue;
> 
> with
> 
> 		if (!console_is_usable(c, c->flags, true) &&
> 		    !console_is_usable(c, c->flags, false))
> 			continue;
> 
> It would make the value compatible with all other callers/users of
> the console drivers.

Thanks Petr. I already have a local branch that will reduce the
console_is_usable of usages like to be called just once, so I'll work
on this change on top of it.

> 
> The variant using two console_is_usable() calls with "true/false"
> parameters is inspited by __pr_flush().
> 
> >  			continue;
> >  		cs[i++] = c;
> >  		if (i >= ARRAY_SIZE(cs))
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index fed98a18e830..fe7c956f73bd 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -3542,7 +3542,7 @@ void console_suspend(struct console *console)
> >  {
> >  	__pr_flush(console, 1000, true);
> >  	console_list_lock();
> > -	console_srcu_write_flags(console, console->flags &
> > ~CON_ENABLED);
> > +	console_srcu_write_flags(console, console->flags |
> > CON_SUSPENDED);
> 
> This is the same flag which is set also by the console_suspend_all()
> API. Now, as discussed at
> https://lore.kernel.org/lkml/844j4lepak.fsf@jogness.linutronix.de/
> 
>    + console_suspend()/console_resume() API is used by few console
>      drivers to suspend the console when the related HW device
>      gets suspended.
> 
>    + console_suspend_all()/console_resume_all() is used by
>      the power management subsystem to call down/up all consoles
>      when the system is going down/up. It is a big hammer approach.
> 
> We need to distinguish the two APIs so that console drivers which
> were
> suspended by both APIs stay suspended until they get resumed by both
> APIs. I mean:
> 
> 	// This should suspend all consoles unless it is not
> disabled
> 	// by "no_console_suspend" API.
> 	console_suspend_all();
> 	// This suspends @con even when "no_console_suspend"
> parameter
> 	// is used. It is needed because the HW is going to be
> suspended.
> 	// It has no effect when the consoles were already suspended
> 	// by the big hammer API.
> 	console_suspend(con);
> 
> 	// This might resume the console when "no_console_suspend"
> option
> 	// is used. The driver should work because the HW was
> resumed.
> 	// But it should stay suspended when all consoles are
> supposed
> 	// to stay suspended because of the big hammer API.
> 	console_resume(con);
> 	// This should resume all consoles.
> 	console_resume_all();
> 
> Other behavior would be unexpected and untested. It might cause
> regression.
> 
> I see two solutions:
> 
>    + add another CON_SUSPENDED_ALL flag
>    + add back "consoles_suspended" global variable
> 
> I prefer adding back the "consoles_suspended" global variable because
> it is a global state...
> 
> The global state should be synchronized the same way as the current
> per-console flag (write under console_list_lock, read under
> console_srcu_read_lock()).
> 
> Also it should be checked by console_is_usable() API. Otherwise, we
> would need to update all callers.
> 
> This brings a challenge how to make it safe and keep the API sane.
> I propose to create:
> 
>   + __console_is_usable() where the "consoles_suspended" value will
>     be passed as parameter. It might be used directly under
>     console_list_lock().
> 
>   + console_is_usable() with the existing parameters. It will check
>     the it was called under console_srcu_read_lock, read
>     the global "consoles_suspend" and pass it to
>     __console_is_usable().
> 
> 

Makes sense. Thanks a lot for the suggestion.

> >  	console_list_unlock();
> >  
> >  	/*
> 
> I played with the code to make sure that it looked sane
> and I ended with the following changes on top of this patch.
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 1b2ce0f36010..fda4683d12f1 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -3552,9 +3552,8 @@ static ssize_t show_cons_active(struct device
> *dev,
>  	for_each_console(c) {
>  		if (!c->device)
>  			continue;
> -		if (!(c->flags & CON_NBCON) && !c->write)
> -			continue;
> -		if (c->flags & CON_SUSPENDED)
> +		if (!__console_is_usable(c, c->flags,
> consoles_suspended, true) &&
> +		    !__console_is_usable(c, c->flags,
> consoles_suspended, false))
>  			continue;
>  		cs[i++] = c;
>  		if (i >= ARRAY_SIZE(cs))
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 5f17321ed962..090490ef570f 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -496,6 +496,7 @@ extern void console_list_lock(void)
> __acquires(console_mutex);
>  extern void console_list_unlock(void) __releases(console_mutex);
>  
>  extern struct hlist_head console_list;
> +extern bool consoles_suspended;
>  
>  /**
>   * console_srcu_read_flags - Locklessly read flags of a possibly
> registered
> @@ -548,6 +549,47 @@ static inline void
> console_srcu_write_flags(struct console *con, short flags)
>  	WRITE_ONCE(con->flags, flags);
>  }
>  
> +/**
> + * consoles_suspended_srcu_read - Locklessly read the global flag
> for
> + *				suspending all consoles.
> + *
> + * The global "consoles_suspended" flag is synchronized using
> console_list_lock
> + * and console_srcu_read_lock. It is the same approach as
> CON_SUSSPENDED flag.
> + * See console_srcu_read_flags() for more details.
> + *
> + * Context: Any context.
> + * Return: The current value of the global "consoles_suspended"
> flag.
> + */
> +static inline short consoles_suspended_srcu_read(void)
> +{
> +	WARN_ON_ONCE(!console_srcu_read_lock_is_held());
> +
> +	/*
> +	 * The READ_ONCE() matches the WRITE_ONCE() when
> "consoles_suspended"
> +	 * is modified with consoles_suspended_srcu_write().
> +	 */
> +	return data_race(READ_ONCE(consoles_suspended));
> +}
> +
> +/**
> + * consoles_suspended_srcu_write - Write the global flag for
> suspending
> + *			all consoles.
> + * @suspend:	new value to write
> + *
> + * The write must be done under the console_list_lock. The caller is
> responsible
> + * for calling synchronize_srcu() to make sure that all callers
> checking the
> + * usablility of registered consoles see the new state.
> + *
> + * Context: Any context.
> + */
> +static inline void consoles_suspended_srcu_write(bool suspend)
> +{
> +	lockdep_assert_console_list_lock_held();
> +
> +	/* This matches the READ_ONCE() in
> consoles_suspended_srcu_read(). */
> +	WRITE_ONCE(consoles_suspended, suspend);
> +}
> +
>  /* Variant of console_is_registered() when the console_list_lock is
> held. */
>  static inline bool console_is_registered_locked(const struct console
> *con)
>  {
> @@ -617,13 +659,15 @@ extern bool nbcon_kdb_try_acquire(struct
> console *con,
>  extern void nbcon_kdb_release(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.
> + * This variant might be called under console_list_lock where both
> + * @flags and @all_suspended flags can be read directly.
>   */
> -static inline bool console_is_usable(struct console *con, short
> flags, bool use_atomic)
> +static inline bool __console_is_usable(struct console *con, short
> flags,
> +				       bool all_suspended, bool
> use_atomic)
>  {
> +	if (all_suspended)
> +		return false;
> +
>  	if (!(flags & CON_ENABLED))
>  		return false;
>  
> @@ -666,6 +710,20 @@ static inline bool console_is_usable(struct
> console *con, short flags, bool use_
>  	return true;
>  }
>  
> +/*
> + * 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)
> +{
> +	bool all_suspended = consoles_suspended_srcu_read();
> +
> +	return __console_is_usable(con, flags, all_suspended,
> use_atomic);
> +}
> +
>  #else
>  static inline void nbcon_cpu_emergency_enter(void) { }
>  static inline void nbcon_cpu_emergency_exit(void) { }
> @@ -678,6 +736,8 @@ 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 nbcon_write_context
> *wctxt) { }
> +static inline bool __console_is_usable(struct console *con, short
> flags,
> +				       bool all_suspended, bool
> use_atomic) { return false; }
>  static inline bool console_is_usable(struct console *con, short
> flags,
>  				     bool use_atomic) { return
> false; }
>  #endif
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 23a14e8c7a49..12247df07420 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -104,6 +104,13 @@ DEFINE_STATIC_SRCU(console_srcu);
>   */
>  int __read_mostly suppress_printk;
>  
> +/*
> + * Global flag for calling down all consoles during suspend.
> + * There is also a per-console flag which is used when the related
> + * device HW gets disabled, see CON_SUSPEND.
> + */
> +bool consoles_suspended;
> +
>  #ifdef CONFIG_LOCKDEP
>  static struct lockdep_map console_lock_dep_map = {
>  	.name = "console_lock"
> @@ -2731,8 +2738,6 @@ MODULE_PARM_DESC(console_no_auto_verbose,
> "Disable console loglevel raise to hig
>   */
>  void console_suspend_all(void)
>  {
> -	struct console *con;
> -
>  	if (console_suspend_enabled)
>  		pr_info("Suspending console(s) (use
> no_console_suspend to debug)\n");
>  
> @@ -2749,8 +2754,7 @@ void console_suspend_all(void)
>  		return;
>  
>  	console_list_lock();
> -	for_each_console(con)
> -		console_srcu_write_flags(con, con->flags |
> CON_SUSPENDED);
> +	consoles_suspended_srcu_write(true);
>  	console_list_unlock();
>  
>  	/*
> @@ -2765,7 +2769,6 @@ void console_suspend_all(void)
>  void console_resume_all(void)
>  {
>  	struct console_flush_type ft;
> -	struct console *con;
>  
>  	/*
>  	 * Allow queueing irq_work. After restoring console state,
> deferred
> @@ -2776,8 +2779,7 @@ void console_resume_all(void)
>  
>  	if (console_suspend_enabled) {
>  		console_list_lock();
> -		for_each_console(con)
> -			console_srcu_write_flags(con, con->flags &
> ~CON_SUSPENDED);
> +		consoles_suspended_srcu_write(false);
>  		console_list_unlock();
>  
>  		/*

You did all the work :) Let pick your changes and prepare the new
patchset using it. Thanks a lot!

> 
> Best Regards,
> Petr