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
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
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
© 2016 - 2026 Red Hat, Inc.