[PATCH] printk: Use two irq_works instead per-CPU

[PATCH] printk: Use two irq_works instead per-CPU

[Sebastian Andrzej Siewior]

In commit b845b517b5e37 ("printk: robustify printk") initially got its
delayed wake up which is today implemented via the irq_work. Back then
it was a variable which was assigned to 1 if a pending wake up was
waiting. This variable was checked on each timer tick. Therefore it
makes sense to assign a per-CPU variable with this task to avoid cache
line bouncing when each CPU reads the variable on each timer tick
simultaneously.

Today we have a irq_work which is enqueued onto a list and handled if it
has work items assigned. This list is already per-CPU.

We could replace the per-CPU irq_work and the per-CPU printk_pending with
two irq_work structs: One for PRINTK_PENDING_WAKEUP and the other for
PRINTK_PENDING_OUTPUT.
While unrolling the conditions in __wake_up_klogd() it becomes obvious
that PRINTK_PENDING_WAKEUP is always passed as an argument but never
checked. The return value wq_has_sleeper() of does not really matter
it's sole purpose is to enforce the needed memory barrier.

Redo the irq_work by:
- use one irq_work (pending_wakeup_work) for klogd wakes. Schedule it
  only if there is a sleeper waiting.
- use one irq_work (&pending_output_work) for output printing. Schedule
  it only if PRINTK_PENDING_OUTPUT passed.
- Drop unused PRINTK_PENDING_WAKEUP.
- Remove the printk_percpu_data_ready() check because per-CPU data is no
  longer used by __wake_up_klogd().

Worst case is defer_console_output() where two irq_work are scheduled but
those get processed one after the other during the HZ tick so it
shouldn't get worse.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/printk/printk.c | 55 ++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 3fcdf4b4e2e53..7c5a220e84323 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4593,37 +4593,29 @@ bool pr_flush(int timeout_ms, bool reset_on_progress)
 /*
  * Delayed printk version, for scheduler-internal messages:
  */
-#define PRINTK_PENDING_WAKEUP	0x01
-#define PRINTK_PENDING_OUTPUT	0x02
+#define PRINTK_PENDING_OUTPUT	0x01
 
-static DEFINE_PER_CPU(int, printk_pending);
-
-static void wake_up_klogd_work_func(struct irq_work *irq_work)
+static void pending_wake_fn(struct irq_work *irq_work)
 {
-	int pending = this_cpu_xchg(printk_pending, 0);
-
-	if (pending & PRINTK_PENDING_OUTPUT) {
-		if (force_legacy_kthread()) {
-			if (printk_legacy_kthread)
-				wake_up_interruptible(&legacy_wait);
-		} else {
-			if (console_trylock())
-				console_unlock();
-		}
-	}
-
-	if (pending & PRINTK_PENDING_WAKEUP)
-		wake_up_interruptible(&log_wait);
+	wake_up_interruptible(&log_wait);
 }
 
-static DEFINE_PER_CPU(struct irq_work, wake_up_klogd_work) =
-	IRQ_WORK_INIT_LAZY(wake_up_klogd_work_func);
+static void pending_output_fn(struct irq_work *irq_work)
+{
+	if (force_legacy_kthread()) {
+		if (printk_legacy_kthread)
+			wake_up_interruptible(&legacy_wait);
+	} else {
+		if (console_trylock())
+			console_unlock();
+	}
+}
+
+struct irq_work pending_wakeup_work = IRQ_WORK_INIT_LAZY(pending_wake_fn);
+struct irq_work pending_output_work = IRQ_WORK_INIT_LAZY(pending_output_fn);
 
 static void __wake_up_klogd(int val)
 {
-	if (!printk_percpu_data_ready())
-		return;
-
 	/*
 	 * It is not allowed to call this function when console irq_work
 	 * is blocked.
@@ -4643,11 +4635,12 @@ static void __wake_up_klogd(int val)
 	 *
 	 * This pairs with devkmsg_read:A and syslog_print:A.
 	 */
-	if (wq_has_sleeper(&log_wait) || /* LMM(__wake_up_klogd:A) */
-	    (val & PRINTK_PENDING_OUTPUT)) {
-		this_cpu_or(printk_pending, val);
-		irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
-	}
+	if (wq_has_sleeper(&log_wait)) /* LMM(__wake_up_klogd:A) */
+		irq_work_queue(&pending_wakeup_work);
+
+	if (val & PRINTK_PENDING_OUTPUT)
+		irq_work_queue(&pending_output_work);
+
 	preempt_enable();
 }
 
@@ -4663,7 +4656,7 @@ static void __wake_up_klogd(int val)
  */
 void wake_up_klogd(void)
 {
-	__wake_up_klogd(PRINTK_PENDING_WAKEUP);
+	__wake_up_klogd(0);
 }
 
 /**
@@ -4684,7 +4677,7 @@ void defer_console_output(void)
 	 * New messages may have been added directly to the ringbuffer
 	 * using vprintk_store(), so wake any waiters as well.
 	 */
-	__wake_up_klogd(PRINTK_PENDING_WAKEUP | PRINTK_PENDING_OUTPUT);
+	__wake_up_klogd(PRINTK_PENDING_OUTPUT);
 }
 
 /**
-- 
2.55.0

Re: [PATCH] printk: Use two irq_works instead per-CPU

[kernel test robot]

Hi Sebastian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v7.3-rc2 next-20260909]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sebastian-Andrzej-Siewior/printk-Use-two-irq_works-instead-per-CPU/20260911-123832
base:   linus/master
patch link:    https://lore.kernel.org/r/20260911103832.w6C8cT4L%40linutronix.de
patch subject: [PATCH] printk: Use two irq_works instead per-CPU
config: m68k-randconfig-r131-20260911 (https://download.01.org/0day-ci/archive/20260912/202609120349.yDbrP6Tf-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.5.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260912/202609120349.yDbrP6Tf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609120349.yDbrP6Tf-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/printk/printk.c:4614:17: sparse: sparse: symbol 'pending_wakeup_work' was not declared. Should it be static?
>> kernel/printk/printk.c:4615:17: sparse: sparse: symbol 'pending_output_work' was not declared. Should it be static?

vim +/pending_wakeup_work +4614 kernel/printk/printk.c

  4613	
> 4614	struct irq_work pending_wakeup_work = IRQ_WORK_INIT_LAZY(pending_wake_fn);
> 4615	struct irq_work pending_output_work = IRQ_WORK_INIT_LAZY(pending_output_fn);
  4616	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki