From: Thomas Gleixner <tglx@linutronix.de>

Add a function to wakeup the printer threads. The printer threads
are woken when:

  - a record is added to the printk ringbuffer
  - consoles are resumed
  - triggered via printk_trigger_flush()
  - consoles should be replayed via sysrq

The actual waking is performed via irq_work so that the function
can be called from any context.

Co-developed-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Signed-off-by: Thomas Gleixner (Intel) <tglx@linutronix.de>
---
 include/linux/console.h  |  3 +++
 kernel/printk/internal.h |  1 +
 kernel/printk/nbcon.c    | 56 ++++++++++++++++++++++++++++++++++++++++
 kernel/printk/printk.c   |  8 ++++++
 4 files changed, 68 insertions(+)

diff --git a/include/linux/console.h b/include/linux/console.h
index 4de42ec1527c..adcc1a74aeee 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -16,6 +16,7 @@
 
 #include <linux/atomic.h>
 #include <linux/bits.h>
+#include <linux/irq_work.h>
 #include <linux/rculist.h>
 #include <linux/rcuwait.h>
 #include <linux/types.h>
@@ -327,6 +328,7 @@ struct nbcon_write_context {
  * @pbufs:		Pointer to nbcon private buffer
  * @kthread:		Printer kthread for this console
  * @rcuwait:		RCU-safe wait object for @kthread waking
+ * @irq_work:		Defer @kthread waking to IRQ work context
  */
 struct console {
 	char			name[16];
@@ -446,6 +448,7 @@ struct console {
 	struct printk_buffers	*pbufs;
 	struct task_struct	*kthread;
 	struct rcuwait		rcuwait;
+	struct irq_work		irq_work;
 };
 
 #ifdef CONFIG_LOCKDEP
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 243d3d3bc889..b2b5c10a4321 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -93,6 +93,7 @@ void nbcon_atomic_flush_pending(void);
 bool nbcon_legacy_emit_next_record(struct console *con, bool *handover,
 				   int cookie);
 void nbcon_kthread_create(struct console *con);
+void nbcon_wake_threads(void);
 
 /*
  * Check if the given console is currently capable and allowed to print
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 4200953d368e..4ab47cc014a3 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1058,6 +1058,61 @@ static int nbcon_kthread_func(void *__console)
 	goto wait_for_event;
 }
 
+/**
+ * nbcon_irq_work - irq work to wake printk thread
+ * @irq_work:	The irq work to operate on
+ */
+static void nbcon_irq_work(struct irq_work *irq_work)
+{
+	struct console *con = container_of(irq_work, struct console, irq_work);
+
+	nbcon_kthread_wake(con);
+}
+
+static inline bool rcuwait_has_sleeper(struct rcuwait *w)
+{
+	bool has_sleeper;
+
+	rcu_read_lock();
+	/*
+	 * Guarantee any new records can be seen by tasks preparing to wait
+	 * before this context checks if the rcuwait is empty.
+	 *
+	 * This full memory barrier pairs with the full memory barrier within
+	 * set_current_state() of ___rcuwait_wait_event(), which is called
+	 * after prepare_to_rcuwait() adds the waiter but before it has
+	 * checked the wait condition.
+	 *
+	 * This pairs with nbcon_kthread_func:A.
+	 */
+	smp_mb(); /* LMM(rcuwait_has_sleeper:A) */
+	has_sleeper = !!rcu_dereference(w->task);
+	rcu_read_unlock();
+
+	return has_sleeper;
+}
+
+/**
+ * nbcon_wake_threads - Wake up printing threads using irq_work
+ */
+void nbcon_wake_threads(void)
+{
+	struct console *con;
+	int cookie;
+
+	cookie = console_srcu_read_lock();
+	for_each_console_srcu(con) {
+		/*
+		 * Only schedule irq_work if the printing thread is
+		 * actively waiting. If not waiting, the thread will
+		 * notice by itself that it has work to do.
+		 */
+		if (con->kthread && rcuwait_has_sleeper(&con->rcuwait))
+			irq_work_queue(&con->irq_work);
+	}
+	console_srcu_read_unlock(cookie);
+}
+
 /* Track the nbcon emergency nesting per CPU. */
 static DEFINE_PER_CPU(unsigned int, nbcon_pcpu_emergency_nesting);
 static unsigned int early_nbcon_pcpu_emergency_nesting __initdata;
@@ -1558,6 +1613,7 @@ void nbcon_init(struct console *con, u64 init_seq)
 	BUG_ON(!con->pbufs);
 
 	rcuwait_init(&con->rcuwait);
+	init_irq_work(&con->irq_work, nbcon_irq_work);
 	nbcon_seq_force(con, init_seq);
 	nbcon_state_set(con, &state);
 }
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a2c4fe3d21cc..457c01311a95 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2402,6 +2402,8 @@ asmlinkage int vprintk_emit(int facility, int level,
 		}
 	}
 
+	nbcon_wake_threads();
+
 	if (do_trylock_unlock) {
 		/*
 		 * The caller may be holding system-critical or
@@ -2708,6 +2710,10 @@ void resume_console(void)
 	 */
 	synchronize_srcu(&console_srcu);
 
+	/*
+	 * Since this runs in task context, wake the threaded printers
+	 * directly rather than scheduling irq_work to do it.
+	 */
 	cookie = console_srcu_read_lock();
 	for_each_console_srcu(con) {
 		flags = console_srcu_read_flags(con);
@@ -4178,6 +4184,7 @@ void defer_console_output(void)
 
 void printk_trigger_flush(void)
 {
+	nbcon_wake_threads();
 	defer_console_output();
 }
 
@@ -4513,6 +4520,7 @@ void console_try_replay_all(void)
 {
 	if (console_trylock()) {
 		__console_rewind_all();
+		nbcon_wake_threads();
 		/* Consoles are flushed as part of console_unlock(). */
 		console_unlock();
 	}
-- 
2.39.2

On Tue 2024-06-04 01:30:42, John Ogness wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add a function to wakeup the printer threads. The printer threads
> are woken when:
> 
>   - a record is added to the printk ringbuffer
>   - consoles are resumed
>   - triggered via printk_trigger_flush()
>   - consoles should be replayed via sysrq
> 
> The actual waking is performed via irq_work so that the function
> can be called from any context.
> 
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1058,6 +1058,61 @@ static int nbcon_kthread_func(void *__console)
>  	goto wait_for_event;
>  }
>  
> +/**
> + * nbcon_irq_work - irq work to wake printk thread
> + * @irq_work:	The irq work to operate on
> + */
> +static void nbcon_irq_work(struct irq_work *irq_work)
> +{
> +	struct console *con = container_of(irq_work, struct console, irq_work);
> +
> +	nbcon_kthread_wake(con);
> +}
> +
> +static inline bool rcuwait_has_sleeper(struct rcuwait *w)
> +{
> +	bool has_sleeper;
> +
> +	rcu_read_lock();
> +	/*
> +	 * Guarantee any new records can be seen by tasks preparing to wait
> +	 * before this context checks if the rcuwait is empty.
> +	 *
> +	 * This full memory barrier pairs with the full memory barrier within
> +	 * set_current_state() of ___rcuwait_wait_event(), which is called
> +	 * after prepare_to_rcuwait() adds the waiter but before it has
> +	 * checked the wait condition.
> +	 *
> +	 * This pairs with nbcon_kthread_func:A.
> +	 */
> +	smp_mb(); /* LMM(rcuwait_has_sleeper:A) */
> +	has_sleeper = !!rcu_dereference(w->task);

We should use the existing API rcuwait_active().

> +	rcu_read_unlock();
> +
> +	return has_sleeper;
> +}
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2402,6 +2402,8 @@ asmlinkage int vprintk_emit(int facility, int level,
>  		}
>  	}
>  
> +	nbcon_wake_threads();

This need to be called only when there is a nbcon console
and no boot console and it has not been flushed explicitly.

I think that we could move this slightly above:

	if (have_nbcon_console && !have_boot_console) {
[...]
		if (is_panic_context ||
		    !printk_threads_enabled ||
		    (system_state > SYSTEM_RUNNING)) {
			nbcon_atomic_flush_pending();
		} else {
			nbcon_wake_threads();
		}
	}

> +
>  	if (do_trylock_unlock) {
>  		/*
>  		 * The caller may be holding system-critical or
> @@ -2708,6 +2710,10 @@ void resume_console(void)
>  	 */
>  	synchronize_srcu(&console_srcu);
>  
> +	/*
> +	 * Since this runs in task context, wake the threaded printers
> +	 * directly rather than scheduling irq_work to do it.
> +	 */
>  	cookie = console_srcu_read_lock();
>  	for_each_console_srcu(con) {
>  		flags = console_srcu_read_flags(con);

The wake up call has already been added in 4th patch of this patchset.

I would slightly prefer to move it from the 4th patch to this one.
Same with start_console(). But it is not super important.


> @@ -4178,6 +4184,7 @@ void defer_console_output(void)
>  
>  void printk_trigger_flush(void)
>  {
> +	nbcon_wake_threads();

IMHO, this is not needed. vprintk_emit() always either flushes nbcon
consoles directly or wakes them.

In each case, it is not needed when printk_trigger_flush() is called
from nbcon_cpu_emergency_exit().

Hmm, I am not sure about the situation in nmi_trigger_cpumask_backtrace().
printk_trigger_flush() is called there to queue the IRQ work yet
another CPU to be on the safe side. But the irq_work used by
nbcon is per-console (not per-CPU). I guess that an attempt to
queue it on 2nd CPU would be a NOP.

>  	defer_console_output();
>  }
>  
> @@ -4513,6 +4520,7 @@ void console_try_replay_all(void)
>  {
>  	if (console_trylock()) {
>  		__console_rewind_all();
> +		nbcon_wake_threads();
>  		/* Consoles are flushed as part of console_unlock(). */
>  		console_unlock();
>  	}

Just an idea. We probably could do better for nbcon consoles. Like try
to flush them directly with emergency prio. But it can be done
in a separate patch later.

Best Regards,
Petr