Once the kthread is running and available
(i.e. @printk_kthreads_running is set), the kthread becomes
responsible for flushing any pending messages which are added
in NBCON_PRIO_NORMAL context. Namely the legacy
console_flush_all() and device_release() no longer flush the
console. And nbcon_atomic_flush_pending() used by
nbcon_cpu_emergency_exit() no longer flushes messages added
after the emergency messages.
The console context is safe when used by the kthread only when
one of the following conditions are true:
1. Other caller acquires the console context with
NBCON_PRIO_NORMAL with preemption disabled. It will
release the context before rescheduling.
2. Other caller acquires the console context with
NBCON_PRIO_NORMAL under the device_lock.
3. The kthread is the only context which acquires the console
with NBCON_PRIO_NORMAL.
This is satisfied for all atomic printing call sites:
nbcon_legacy_emit_next_record() (#1)
nbcon_atomic_flush_pending_con() (#1)
nbcon_device_release() (#2)
It is even double guaranteed when @printk_kthreads_running
is set because then _only_ the kthread will print for
NBCON_PRIO_NORMAL. (#3)
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
kernel/printk/internal.h | 26 ++++++++++++++++++++++
kernel/printk/nbcon.c | 17 ++++++++++-----
kernel/printk/printk.c | 47 +++++++++++++++++++++++++++++++++++++++-
3 files changed, 83 insertions(+), 7 deletions(-)
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index a96d4114a1db..8166e24f8780 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -113,6 +113,13 @@ static inline bool console_is_usable(struct console *con, short flags, bool use_
/* 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;
@@ -176,6 +183,7 @@ static inline void nbcon_atomic_flush_pending(void) { }
static inline bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
int cookie, bool use_atomic) { return false; }
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; }
@@ -190,6 +198,7 @@ extern bool legacy_allow_panic_sync;
/**
* struct console_flush_type - Define available console flush methods
* @nbcon_atomic: Flush directly using nbcon_atomic() callback
+ * @nbcon_offload: Offload flush to printer thread
* @legacy_direct: Call the legacy loop in this context
* @legacy_offload: Offload the legacy loop into IRQ
*
@@ -197,6 +206,7 @@ extern bool legacy_allow_panic_sync;
*/
struct console_flush_type {
bool nbcon_atomic;
+ bool nbcon_offload;
bool legacy_direct;
bool legacy_offload;
};
@@ -211,6 +221,22 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft)
switch (nbcon_get_default_prio()) {
case NBCON_PRIO_NORMAL:
+ if (have_nbcon_console && !have_boot_console) {
+ if (printk_kthreads_running)
+ ft->nbcon_offload = true;
+ else
+ ft->nbcon_atomic = true;
+ }
+
+ /* Legacy consoles are flushed directly when possible. */
+ if (have_legacy_console || have_boot_console) {
+ if (!is_printk_legacy_deferred())
+ ft->legacy_direct = true;
+ else
+ ft->legacy_offload = true;
+ }
+ break;
+
case NBCON_PRIO_EMERGENCY:
if (have_nbcon_console && !have_boot_console)
ft->nbcon_atomic = true;
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 8745fffbfbb0..cebdb9936609 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1492,6 +1492,7 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
static void nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
bool allow_unsafe_takeover)
{
+ struct console_flush_type ft;
unsigned long flags;
int err;
@@ -1521,10 +1522,12 @@ static void nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
/*
* If flushing was successful but more records are available, this
- * context must flush those remaining records because there is no
- * other context that will do it.
+ * context must flush those remaining records if the printer thread
+ * is not available do it.
*/
- if (prb_read_valid(prb, nbcon_seq_read(con), NULL)) {
+ printk_get_console_flush_type(&ft);
+ if (!ft.nbcon_offload &&
+ prb_read_valid(prb, nbcon_seq_read(con), NULL)) {
stop_seq = prb_next_reserve_seq(prb);
goto again;
}
@@ -1752,17 +1755,19 @@ void nbcon_device_release(struct console *con)
/*
* This context must flush any new records added while the console
- * was locked. The console_srcu_read_lock must be taken to ensure
- * the console is usable throughout flushing.
+ * was locked if the printer thread is not available to do it. The
+ * console_srcu_read_lock must be taken to ensure the console is
+ * usable throughout flushing.
*/
cookie = console_srcu_read_lock();
+ printk_get_console_flush_type(&ft);
if (console_is_usable(con, console_srcu_read_flags(con), true) &&
+ !ft.nbcon_offload &&
prb_read_valid(prb, nbcon_seq_read(con), NULL)) {
/*
* If nbcon_atomic flushing is not available, fallback to
* using the legacy loop.
*/
- printk_get_console_flush_type(&ft);
if (ft.nbcon_atomic) {
__nbcon_atomic_flush_pending_con(con, prb_next_reserve_seq(prb), false);
} else if (ft.legacy_direct) {
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 55d75db00042..b9378636188e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2384,6 +2384,9 @@ asmlinkage int vprintk_emit(int facility, int level,
if (ft.nbcon_atomic)
nbcon_atomic_flush_pending();
+ if (ft.nbcon_offload)
+ nbcon_kthreads_wake();
+
if (ft.legacy_direct) {
/*
* The caller may be holding system-critical or
@@ -2732,6 +2735,7 @@ void suspend_console(void)
void resume_console(void)
{
+ struct console_flush_type ft;
struct console *con;
if (!console_suspend_enabled)
@@ -2749,6 +2753,10 @@ void resume_console(void)
*/
synchronize_srcu(&console_srcu);
+ printk_get_console_flush_type(&ft);
+ if (ft.nbcon_offload)
+ nbcon_kthreads_wake();
+
pr_flush(1000, true);
}
@@ -3060,6 +3068,7 @@ static inline void printk_kthreads_check_locked(void) { }
*/
static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handover)
{
+ struct console_flush_type ft;
bool any_usable = false;
struct console *con;
bool any_progress;
@@ -3071,12 +3080,21 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove
do {
any_progress = false;
+ printk_get_console_flush_type(&ft);
+
cookie = console_srcu_read_lock();
for_each_console_srcu(con) {
short flags = console_srcu_read_flags(con);
u64 printk_seq;
bool progress;
+ /*
+ * console_flush_all() is only for legacy consoles when
+ * the nbcon consoles have their printer threads.
+ */
+ if ((flags & CON_NBCON) && ft.nbcon_offload)
+ continue;
+
if (!console_is_usable(con, flags, !do_cond_resched))
continue;
any_usable = true;
@@ -3387,9 +3405,25 @@ EXPORT_SYMBOL(console_stop);
void console_start(struct console *console)
{
+ struct console_flush_type ft;
+ bool is_nbcon;
+
console_list_lock();
console_srcu_write_flags(console, console->flags | CON_ENABLED);
+ is_nbcon = console->flags & CON_NBCON;
console_list_unlock();
+
+ /*
+ * Ensure that all SRCU list walks have completed. The related
+ * printing context must be able to see it is enabled so that
+ * it is guaranteed to wake up and resume printing.
+ */
+ synchronize_srcu(&console_srcu);
+
+ printk_get_console_flush_type(&ft);
+ if (is_nbcon && ft.nbcon_offload)
+ nbcon_kthread_wake(console);
+
__pr_flush(console, 1000, true);
}
EXPORT_SYMBOL(console_start);
@@ -4115,6 +4149,8 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
/* Flush the consoles so that records up to @seq are printed. */
printk_get_console_flush_type(&ft);
+ if (ft.nbcon_atomic)
+ nbcon_atomic_flush_pending();
if (ft.legacy_direct) {
console_lock();
console_unlock();
@@ -4152,8 +4188,10 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
* that they make forward progress, so only increment
* @diff for usable consoles.
*/
- if (!console_is_usable(c, flags, true))
+ if (!console_is_usable(c, flags, true) &&
+ !console_is_usable(c, flags, false)) {
continue;
+ }
if (flags & CON_NBCON) {
printk_seq = nbcon_seq_read(c);
@@ -4629,8 +4667,15 @@ EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
*/
void console_try_replay_all(void)
{
+ struct console_flush_type ft;
+
+ printk_get_console_flush_type(&ft);
if (console_trylock()) {
__console_rewind_all();
+ if (ft.nbcon_atomic)
+ nbcon_atomic_flush_pending();
+ if (ft.nbcon_offload)
+ nbcon_kthreads_wake();
/* Consoles are flushed as part of console_unlock(). */
console_unlock();
}
--
2.39.2On Fri 2024-08-30 17:35:08, John Ogness wrote:
> Once the kthread is running and available
> (i.e. @printk_kthreads_running is set), the kthread becomes
> responsible for flushing any pending messages which are added
> in NBCON_PRIO_NORMAL context. Namely the legacy
> console_flush_all() and device_release() no longer flush the
> console. And nbcon_atomic_flush_pending() used by
> nbcon_cpu_emergency_exit() no longer flushes messages added
> after the emergency messages.
>
> The console context is safe when used by the kthread only when
> one of the following conditions are true:
>
> 1. Other caller acquires the console context with
> NBCON_PRIO_NORMAL with preemption disabled. It will
> release the context before rescheduling.
>
> 2. Other caller acquires the console context with
> NBCON_PRIO_NORMAL under the device_lock.
>
> 3. The kthread is the only context which acquires the console
> with NBCON_PRIO_NORMAL.
>
> This is satisfied for all atomic printing call sites:
>
> nbcon_legacy_emit_next_record() (#1)
>
> nbcon_atomic_flush_pending_con() (#1)
>
> nbcon_device_release() (#2)
>
> It is even double guaranteed when @printk_kthreads_running
> is set because then _only_ the kthread will print for
> NBCON_PRIO_NORMAL. (#3)
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> kernel/printk/internal.h | 26 ++++++++++++++++++++++
> kernel/printk/nbcon.c | 17 ++++++++++-----
> kernel/printk/printk.c | 47 +++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
> index a96d4114a1db..8166e24f8780 100644
> --- a/kernel/printk/internal.h
> +++ b/kernel/printk/internal.h
> @@ -113,6 +113,13 @@ static inline bool console_is_usable(struct console *con, short flags, bool use_
> /* 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;
> @@ -176,6 +183,7 @@ static inline void nbcon_atomic_flush_pending(void) { }
> static inline bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
> int cookie, bool use_atomic) { return false; }
> 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; }
> @@ -190,6 +198,7 @@ extern bool legacy_allow_panic_sync;
> /**
> * struct console_flush_type - Define available console flush methods
> * @nbcon_atomic: Flush directly using nbcon_atomic() callback
> + * @nbcon_offload: Offload flush to printer thread
> * @legacy_direct: Call the legacy loop in this context
> * @legacy_offload: Offload the legacy loop into IRQ
> *
> @@ -197,6 +206,7 @@ extern bool legacy_allow_panic_sync;
> */
> struct console_flush_type {
> bool nbcon_atomic;
> + bool nbcon_offload;
> bool legacy_direct;
> bool legacy_offload;
> };
> @@ -211,6 +221,22 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft)
>
> switch (nbcon_get_default_prio()) {
> case NBCON_PRIO_NORMAL:
> + if (have_nbcon_console && !have_boot_console) {
> + if (printk_kthreads_running)
> + ft->nbcon_offload = true;
> + else
> + ft->nbcon_atomic = true;
> + }
> +
> + /* Legacy consoles are flushed directly when possible. */
> + if (have_legacy_console || have_boot_console) {
> + if (!is_printk_legacy_deferred())
> + ft->legacy_direct = true;
> + else
> + ft->legacy_offload = true;
> + }
> + break;
> +
> case NBCON_PRIO_EMERGENCY:
> if (have_nbcon_console && !have_boot_console)
> ft->nbcon_atomic = true;
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 8745fffbfbb0..cebdb9936609 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1492,6 +1492,7 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
> static void nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
> bool allow_unsafe_takeover)
> {
> + struct console_flush_type ft;
> unsigned long flags;
> int err;
>
> @@ -1521,10 +1522,12 @@ static void nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
>
> /*
> * If flushing was successful but more records are available, this
> - * context must flush those remaining records because there is no
> - * other context that will do it.
> + * context must flush those remaining records if the printer thread
> + * is not available do it.
> */
> - if (prb_read_valid(prb, nbcon_seq_read(con), NULL)) {
> + printk_get_console_flush_type(&ft);
> + if (!ft.nbcon_offload &&
> + prb_read_valid(prb, nbcon_seq_read(con), NULL)) {
> stop_seq = prb_next_reserve_seq(prb);
> goto again;
> }
> @@ -1752,17 +1755,19 @@ void nbcon_device_release(struct console *con)
>
> /*
> * This context must flush any new records added while the console
> - * was locked. The console_srcu_read_lock must be taken to ensure
> - * the console is usable throughout flushing.
> + * was locked if the printer thread is not available to do it. The
> + * console_srcu_read_lock must be taken to ensure the console is
> + * usable throughout flushing.
> */
> cookie = console_srcu_read_lock();
> + printk_get_console_flush_type(&ft);
> if (console_is_usable(con, console_srcu_read_flags(con), true) &&
> + !ft.nbcon_offload &&
> prb_read_valid(prb, nbcon_seq_read(con), NULL)) {
> /*
> * If nbcon_atomic flushing is not available, fallback to
> * using the legacy loop.
> */
> - printk_get_console_flush_type(&ft);
> if (ft.nbcon_atomic) {
> __nbcon_atomic_flush_pending_con(con, prb_next_reserve_seq(prb), false);
> } else if (ft.legacy_direct) {
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 55d75db00042..b9378636188e 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2384,6 +2384,9 @@ asmlinkage int vprintk_emit(int facility, int level,
> if (ft.nbcon_atomic)
> nbcon_atomic_flush_pending();
>
> + if (ft.nbcon_offload)
> + nbcon_kthreads_wake();
> +
> if (ft.legacy_direct) {
> /*
> * The caller may be holding system-critical or
> @@ -2732,6 +2735,7 @@ void suspend_console(void)
>
> void resume_console(void)
> {
> + struct console_flush_type ft;
> struct console *con;
>
> if (!console_suspend_enabled)
> @@ -2749,6 +2753,10 @@ void resume_console(void)
> */
> synchronize_srcu(&console_srcu);
>
> + printk_get_console_flush_type(&ft);
> + if (ft.nbcon_offload)
> + nbcon_kthreads_wake();
> +
> pr_flush(1000, true);
> }
>
> @@ -3060,6 +3068,7 @@ static inline void printk_kthreads_check_locked(void) { }
> */
> static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handover)
> {
> + struct console_flush_type ft;
> bool any_usable = false;
> struct console *con;
> bool any_progress;
> @@ -3071,12 +3080,21 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove
> do {
> any_progress = false;
>
> + printk_get_console_flush_type(&ft);
> +
> cookie = console_srcu_read_lock();
> for_each_console_srcu(con) {
> short flags = console_srcu_read_flags(con);
> u64 printk_seq;
> bool progress;
>
> + /*
> + * console_flush_all() is only for legacy consoles when
> + * the nbcon consoles have their printer threads.
> + */
> + if ((flags & CON_NBCON) && ft.nbcon_offload)
> + continue;
If I get it correctly then we could skip nbcon consoles here also when
ft.nbcon_atomic == true.
In this case, the messages are flushed directly from vprintk_emit() by
nbcon_atomic_flush_pending(). It goes down to
nbcon_atomic_flush_pending_con() which takes care also about parallel
printk() calls.
A question is whether we want this.
On one side, we want to separate the legacy code as much as possible.
And it should be needed only when there is either a boot or legacy
console.
On the other side, the legacy loop has the console_trylock_spinning()
which allows to pass the owner. While nbcon_atomic_flush_pending_con()
leaves the responsibility on the current owner.
Well, the printk kthreads should be started early enough to prevent
softlockups. I hope that the owner steeling trick won't be needed
for nbcon consoles.
In addition, nbcon_atomic_flush_pending_con() allows to flush
the messages directly even in NMI.
So, I think that we go in the right direction. I mean that we really
should handle nbcon consoles in the legacy loop only when there
is a boot console (both ft.nbcon_* == false).
> +
> if (!console_is_usable(con, flags, !do_cond_resched))
> continue;
> any_usable = true;
Best Regards,
Petr
On 2024-09-03, Petr Mladek <pmladek@suse.com> wrote:
>> @@ -3071,12 +3080,21 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove
>> do {
>> any_progress = false;
>>
>> + printk_get_console_flush_type(&ft);
>> +
>> cookie = console_srcu_read_lock();
>> for_each_console_srcu(con) {
>> short flags = console_srcu_read_flags(con);
>> u64 printk_seq;
>> bool progress;
>>
>> + /*
>> + * console_flush_all() is only for legacy consoles when
>> + * the nbcon consoles have their printer threads.
>> + */
>> + if ((flags & CON_NBCON) && ft.nbcon_offload)
>> + continue;
>
> I mean that we really
> should handle nbcon consoles in the legacy loop only when there
> is a boot console (both ft.nbcon_* == false).
Agreed. I will change it to:
/*
* console_flush_all() is only responsible for nbcon
* consoles when the nbcon consoles cannot print via
* their atomic or threaded flushing.
*/
if ((flags & CON_NBCON) && (ft.nbcon_atomic || ft.nbcon_offload))
continue;
Also note that patch 15/17 ("printk: Implement legacy printer kthread
for PREEMPT_RT") has the same check in legacy_kthread_should_wakeup(),
which I will also update.
John
© 2016 - 2025 Red Hat, Inc.