[RFC PATCH 0/5] printk: Make it usable on nohz CPUs

[RFC PATCH 0/5] printk: Make it usable on nohz CPUs

[Frederic Weisbecker]

Hi,

So here is a proposition on what we can do to make printk
correctly working on a tickless CPU.

Although it's targeted to be part of the adaptive tickmess
implemetation, it's pretty standalone and generic and also
works for printk() calls in idle.

It is based on latest linus tree.

Waiting for your comments.

Thanks.

PS: only built-tested for now.

Frederic Weisbecker (5):
  irq_work: Move irq_work_raise() declaration/default definition to
    arch headers
  irq_work: Only run irq_work from tick if arch needs it
  x86: Implement arch_irq_work_use_tick
  nohz: Add API to check tick state
  printk: Wake up klogd with irq_work on nohz CPU

 arch/alpha/include/asm/irq_work.h    |    9 +++++++
 arch/alpha/kernel/time.c             |    2 +-
 arch/arm/include/asm/irq_work.h      |    1 +
 arch/arm64/include/asm/irq_work.h    |    1 +
 arch/blackfin/include/asm/irq_work.h |    1 +
 arch/frv/include/asm/irq_work.h      |    1 +
 arch/hexagon/include/asm/irq_work.h  |    1 +
 arch/mips/include/asm/irq_work.h     |    1 +
 arch/parisc/include/asm/irq_work.h   |    1 +
 arch/powerpc/include/asm/irq_work.h  |    8 ++++++
 arch/powerpc/kernel/time.c           |    2 +-
 arch/s390/include/asm/irq_work.h     |    1 +
 arch/sh/include/asm/irq_work.h       |    1 +
 arch/sparc/include/asm/irq_work.h    |    8 ++++++
 arch/sparc/kernel/pcr.c              |    2 +-
 arch/x86/include/asm/irq_work.h      |   15 ++++++++++++
 arch/x86/kernel/irq_work.c           |    6 ++--
 include/asm-generic/irq_work.h       |   22 +++++++++++++++++
 include/linux/irq_work.h             |    1 +
 include/linux/tick.h                 |   16 ++++++++++++-
 kernel/irq_work.c                    |    7 -----
 kernel/printk.c                      |   42 ++++++++++++++++++++++++++++++++++
 kernel/time/tick-sched.c             |    2 +-
 kernel/timer.c                       |    2 +-
 24 files changed, 137 insertions(+), 16 deletions(-)
 create mode 100644 arch/alpha/include/asm/irq_work.h
 create mode 100644 arch/arm/include/asm/irq_work.h
 create mode 100644 arch/arm64/include/asm/irq_work.h
 create mode 100644 arch/blackfin/include/asm/irq_work.h
 create mode 100644 arch/frv/include/asm/irq_work.h
 create mode 100644 arch/hexagon/include/asm/irq_work.h
 create mode 100644 arch/mips/include/asm/irq_work.h
 create mode 100644 arch/parisc/include/asm/irq_work.h
 create mode 100644 arch/powerpc/include/asm/irq_work.h
 create mode 100644 arch/s390/include/asm/irq_work.h
 create mode 100644 arch/sh/include/asm/irq_work.h
 create mode 100644 arch/sparc/include/asm/irq_work.h
 create mode 100644 arch/x86/include/asm/irq_work.h
 create mode 100644 include/asm-generic/irq_work.h

-- 
1.7.5.4

[RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Frederic Weisbecker]

This optimization doesn't matter much. But this prepares the
arch headers that we need to add a new API in order to detect
when the arch hooks on the tick to implement the irq work.

This is necessary later to make printk working in nohz CPUs.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/alpha/include/asm/irq_work.h    |    6 ++++++
 arch/arm/include/asm/irq_work.h      |    1 +
 arch/arm64/include/asm/irq_work.h    |    1 +
 arch/blackfin/include/asm/irq_work.h |    1 +
 arch/frv/include/asm/irq_work.h      |    1 +
 arch/hexagon/include/asm/irq_work.h  |    1 +
 arch/mips/include/asm/irq_work.h     |    1 +
 arch/parisc/include/asm/irq_work.h   |    1 +
 arch/powerpc/include/asm/irq_work.h  |    6 ++++++
 arch/s390/include/asm/irq_work.h     |    1 +
 arch/sh/include/asm/irq_work.h       |    1 +
 arch/sparc/include/asm/irq_work.h    |    6 ++++++
 arch/x86/include/asm/irq_work.h      |   10 ++++++++++
 arch/x86/kernel/irq_work.c           |    4 ++--
 include/asm-generic/irq_work.h       |    9 +++++++++
 include/linux/irq_work.h             |    1 +
 kernel/irq_work.c                    |    7 -------
 17 files changed, 49 insertions(+), 9 deletions(-)
 create mode 100644 arch/alpha/include/asm/irq_work.h
 create mode 100644 arch/arm/include/asm/irq_work.h
 create mode 100644 arch/arm64/include/asm/irq_work.h
 create mode 100644 arch/blackfin/include/asm/irq_work.h
 create mode 100644 arch/frv/include/asm/irq_work.h
 create mode 100644 arch/hexagon/include/asm/irq_work.h
 create mode 100644 arch/mips/include/asm/irq_work.h
 create mode 100644 arch/parisc/include/asm/irq_work.h
 create mode 100644 arch/powerpc/include/asm/irq_work.h
 create mode 100644 arch/s390/include/asm/irq_work.h
 create mode 100644 arch/sh/include/asm/irq_work.h
 create mode 100644 arch/sparc/include/asm/irq_work.h
 create mode 100644 arch/x86/include/asm/irq_work.h
 create mode 100644 include/asm-generic/irq_work.h

diff --git a/arch/alpha/include/asm/irq_work.h b/arch/alpha/include/asm/irq_work.h
new file mode 100644
index 0000000..814ff3d
--- /dev/null
+++ b/arch/alpha/include/asm/irq_work.h
@@ -0,0 +1,6 @@
+#ifndef _ALPHA_IRQ_WORK_H
+#define _ALPHA_IRQ_WORK_H
+
+extern void arch_irq_work_raise(void);
+
+#endif
diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/arm/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/arm64/include/asm/irq_work.h b/arch/arm64/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/arm64/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/blackfin/include/asm/irq_work.h b/arch/blackfin/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/blackfin/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/frv/include/asm/irq_work.h b/arch/frv/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/frv/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/hexagon/include/asm/irq_work.h b/arch/hexagon/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/hexagon/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/mips/include/asm/irq_work.h b/arch/mips/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/mips/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/parisc/include/asm/irq_work.h b/arch/parisc/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/parisc/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/powerpc/include/asm/irq_work.h b/arch/powerpc/include/asm/irq_work.h
new file mode 100644
index 0000000..8b9927f
--- /dev/null
+++ b/arch/powerpc/include/asm/irq_work.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_POWERPC_IRQ_WORK_H
+#define _ASM_POWERPC_IRQ_WORK_H
+
+extern void arch_irq_work_raise(void);
+
+#endif
diff --git a/arch/s390/include/asm/irq_work.h b/arch/s390/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/s390/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/sh/include/asm/irq_work.h b/arch/sh/include/asm/irq_work.h
new file mode 100644
index 0000000..f1bffa2
--- /dev/null
+++ b/arch/sh/include/asm/irq_work.h
@@ -0,0 +1 @@
+#include <asm-generic/irq_work.h>
diff --git a/arch/sparc/include/asm/irq_work.h b/arch/sparc/include/asm/irq_work.h
new file mode 100644
index 0000000..1d062a6
--- /dev/null
+++ b/arch/sparc/include/asm/irq_work.h
@@ -0,0 +1,6 @@
+#ifndef ___ASM_SPARC_IRQ_H
+#define ___ASM_SPARC_IRQ_H
+
+extern void arch_irq_work_raise(void);
+
+#endif
diff --git a/arch/x86/include/asm/irq_work.h b/arch/x86/include/asm/irq_work.h
new file mode 100644
index 0000000..38eed96
--- /dev/null
+++ b/arch/x86/include/asm/irq_work.h
@@ -0,0 +1,10 @@
+#ifndef _ASM_X86_IRQ_WORK_H
+#define _ASM_X86_IRQ_WORK_H
+
+#ifndef CONFIG_X86_LOCAL_APIC
+#include <asm-generic/irq_work.h>
+# else
+extern void arch_irq_work_raise(void);
+#endif
+
+#endif
diff --git a/arch/x86/kernel/irq_work.c b/arch/x86/kernel/irq_work.c
index ca8f703..95f5d4e 100644
--- a/arch/x86/kernel/irq_work.c
+++ b/arch/x86/kernel/irq_work.c
@@ -18,13 +18,13 @@ void smp_irq_work_interrupt(struct pt_regs *regs)
 	irq_exit();
 }
 
+#ifdef CONFIG_X86_LOCAL_APIC
 void arch_irq_work_raise(void)
 {
-#ifdef CONFIG_X86_LOCAL_APIC
 	if (!cpu_has_apic)
 		return;
 
 	apic->send_IPI_self(IRQ_WORK_VECTOR);
 	apic_wait_icr_idle();
-#endif
 }
+#endif
diff --git a/include/asm-generic/irq_work.h b/include/asm-generic/irq_work.h
new file mode 100644
index 0000000..a2d4108
--- /dev/null
+++ b/include/asm-generic/irq_work.h
@@ -0,0 +1,9 @@
+#ifndef __ASM_GENERIC_ATOMIC_H
+#define __ASM_GENERIC_ATOMIC_H
+
+/*
+ * Lame architectures will get the timer tick callback
+ */
+static inline void arch_irq_work_raise(void) { }
+
+#endif
diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 6a9e8f5..b39ea0b 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -2,6 +2,7 @@
 #define _LINUX_IRQ_WORK_H
 
 #include <linux/llist.h>
+#include <asm/irq_work.h>
 
 struct irq_work {
 	unsigned long flags;
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..44a5b19 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -49,13 +49,6 @@ static bool irq_work_claim(struct irq_work *work)
 	return true;
 }
 
-void __weak arch_irq_work_raise(void)
-{
-	/*
-	 * Lame architectures will get the timer tick callback
-	 */
-}
-
 /*
  * Queue the entry and raise the IPI if needed.
  */
-- 
1.7.5.4

[RFC PATCH 2/5] irq_work: Only run irq_work from tick if arch needs it

[Frederic Weisbecker]

It may optimize a bit the tick path for archs that have their
own way to run irq work.

This may be further optimized using static keys.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/alpha/include/asm/irq_work.h   |    5 ++++-
 arch/alpha/kernel/time.c            |    2 +-
 arch/powerpc/include/asm/irq_work.h |    4 +++-
 arch/powerpc/kernel/time.c          |    2 +-
 arch/sparc/include/asm/irq_work.h   |    4 +++-
 arch/sparc/kernel/pcr.c             |    2 +-
 arch/x86/include/asm/irq_work.h     |    9 +++++----
 arch/x86/kernel/irq_work.c          |    2 +-
 include/asm-generic/irq_work.h      |   13 +++++++++++++
 kernel/timer.c                      |    2 +-
 10 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/arch/alpha/include/asm/irq_work.h b/arch/alpha/include/asm/irq_work.h
index 814ff3d..3d32132 100644
--- a/arch/alpha/include/asm/irq_work.h
+++ b/arch/alpha/include/asm/irq_work.h
@@ -1,6 +1,9 @@
 #ifndef _ALPHA_IRQ_WORK_H
 #define _ALPHA_IRQ_WORK_H
 
-extern void arch_irq_work_raise(void);
+extern void __arch_irq_work_raise(void);
+#define arch_irq_work_raise __arch_irq_work_raise
+
+#include <asm-generic/irq_work.h>
 
 #endif
diff --git a/arch/alpha/kernel/time.c b/arch/alpha/kernel/time.c
index e336694..91c5eec 100644
--- a/arch/alpha/kernel/time.c
+++ b/arch/alpha/kernel/time.c
@@ -90,7 +90,7 @@ DEFINE_PER_CPU(u8, irq_work_pending);
 #define test_irq_work_pending()      __get_cpu_var(irq_work_pending)
 #define clear_irq_work_pending()     __get_cpu_var(irq_work_pending) = 0
 
-void arch_irq_work_raise(void)
+void __arch_irq_work_raise(void)
 {
 	set_irq_work_pending_flag();
 }
diff --git a/arch/powerpc/include/asm/irq_work.h b/arch/powerpc/include/asm/irq_work.h
index 8b9927f..8aa36aa 100644
--- a/arch/powerpc/include/asm/irq_work.h
+++ b/arch/powerpc/include/asm/irq_work.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_POWERPC_IRQ_WORK_H
 #define _ASM_POWERPC_IRQ_WORK_H
 
-extern void arch_irq_work_raise(void);
+extern void __arch_irq_work_raise(void);
+#define arch_irq_work_raise __arch_irq_work_raise
 
+#include <asm-generic/irq_work.h>
 #endif
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index c9986fd..31565ac 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -466,7 +466,7 @@ DEFINE_PER_CPU(u8, irq_work_pending);
 
 #endif /* 32 vs 64 bit */
 
-void arch_irq_work_raise(void)
+void __arch_irq_work_raise(void)
 {
 	preempt_disable();
 	set_irq_work_pending_flag();
diff --git a/arch/sparc/include/asm/irq_work.h b/arch/sparc/include/asm/irq_work.h
index 1d062a6..383772d 100644
--- a/arch/sparc/include/asm/irq_work.h
+++ b/arch/sparc/include/asm/irq_work.h
@@ -1,6 +1,8 @@
 #ifndef ___ASM_SPARC_IRQ_H
 #define ___ASM_SPARC_IRQ_H
 
-extern void arch_irq_work_raise(void);
+extern void __arch_irq_work_raise(void);
+#define arch_irq_work_raise __arch_irq_work_raise
 
+#include <asm-generic/irq_work.h>
 #endif
diff --git a/arch/sparc/kernel/pcr.c b/arch/sparc/kernel/pcr.c
index 269af58..d1e1ecf 100644
--- a/arch/sparc/kernel/pcr.c
+++ b/arch/sparc/kernel/pcr.c
@@ -43,7 +43,7 @@ void __irq_entry deferred_pcr_work_irq(int irq, struct pt_regs *regs)
 	set_irq_regs(old_regs);
 }
 
-void arch_irq_work_raise(void)
+void __arch_irq_work_raise(void)
 {
 	set_softint(1 << PIL_DEFERRED_PCR_WORK);
 }
diff --git a/arch/x86/include/asm/irq_work.h b/arch/x86/include/asm/irq_work.h
index 38eed96..dad8266 100644
--- a/arch/x86/include/asm/irq_work.h
+++ b/arch/x86/include/asm/irq_work.h
@@ -1,10 +1,11 @@
 #ifndef _ASM_X86_IRQ_WORK_H
 #define _ASM_X86_IRQ_WORK_H
 
-#ifndef CONFIG_X86_LOCAL_APIC
-#include <asm-generic/irq_work.h>
-# else
-extern void arch_irq_work_raise(void);
+#ifdef CONFIG_X86_LOCAL_APIC
+extern void __arch_irq_work_raise(void);
+#define arch_irq_work_raise __arch_irq_work_raise
 #endif
 
+#include <asm-generic/irq_work.h>
+
 #endif
diff --git a/arch/x86/kernel/irq_work.c b/arch/x86/kernel/irq_work.c
index 95f5d4e..7389d5e 100644
--- a/arch/x86/kernel/irq_work.c
+++ b/arch/x86/kernel/irq_work.c
@@ -19,7 +19,7 @@ void smp_irq_work_interrupt(struct pt_regs *regs)
 }
 
 #ifdef CONFIG_X86_LOCAL_APIC
-void arch_irq_work_raise(void)
+void __arch_irq_work_raise(void)
 {
 	if (!cpu_has_apic)
 		return;
diff --git a/include/asm-generic/irq_work.h b/include/asm-generic/irq_work.h
index a2d4108..b172da0 100644
--- a/include/asm-generic/irq_work.h
+++ b/include/asm-generic/irq_work.h
@@ -4,6 +4,19 @@
 /*
  * Lame architectures will get the timer tick callback
  */
+#ifndef arch_irq_work_raise
 static inline void arch_irq_work_raise(void) { }
+#endif
+
+/*
+ * Unless told otherwise, consider the arch implements irq work
+ * through a hook to the timer tick.
+ */
+#ifndef arch_irq_work_use_tick
+static inline bool arch_irq_work_use_tick(void)
+{
+	return true;
+}
+#endif
 
 #endif
diff --git a/kernel/timer.c b/kernel/timer.c
index d5de1b2..6a6d1b1 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1351,7 +1351,7 @@ void update_process_times(int user_tick)
 	rcu_check_callbacks(cpu, user_tick);
 	printk_tick();
 #ifdef CONFIG_IRQ_WORK
-	if (in_irq())
+	if (arch_irq_work_use_tick() && in_irq())
 		irq_work_run();
 #endif
 	scheduler_tick();
-- 
1.7.5.4

[RFC PATCH 3/5] x86: Implement arch_irq_work_use_tick

[Frederic Weisbecker]

Most of the time, x86 can trigger self-IPIs. Tell
irq work subsystem about it.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/include/asm/irq_work.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/irq_work.h b/arch/x86/include/asm/irq_work.h
index dad8266..fc62b60 100644
--- a/arch/x86/include/asm/irq_work.h
+++ b/arch/x86/include/asm/irq_work.h
@@ -2,8 +2,12 @@
 #define _ASM_X86_IRQ_WORK_H
 
 #ifdef CONFIG_X86_LOCAL_APIC
+#include <asm/cpufeature.h>
+
 extern void __arch_irq_work_raise(void);
 #define arch_irq_work_raise __arch_irq_work_raise
+
+#define arch_irq_work_use_tick() (!cpu_has_apic)
 #endif
 
 #include <asm-generic/irq_work.h>
-- 
1.7.5.4

[RFC PATCH 4/5] nohz: Add API to check tick state

[Frederic Weisbecker]

We need some quick way to check if the CPU has stopped
its tick. This will be useful for printk when it wants
to wake up klogd on nohz CPU.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/tick.h     |   16 +++++++++++++++-
 kernel/time/tick-sched.c |    2 +-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/linux/tick.h b/include/linux/tick.h
index f37fceb..05d1919 100644
--- a/include/linux/tick.h
+++ b/include/linux/tick.h
@@ -8,6 +8,7 @@
 
 #include <linux/clockchips.h>
 #include <linux/irqflags.h>
+#include <linux/percpu.h>
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS
 
@@ -122,13 +123,26 @@ static inline int tick_oneshot_mode_active(void) { return 0; }
 #endif /* !CONFIG_GENERIC_CLOCKEVENTS */
 
 # ifdef CONFIG_NO_HZ
+DECLARE_PER_CPU(struct tick_sched, tick_cpu_sched);
+
+static inline int tick_nohz_tick_stopped(void)
+{
+	return __this_cpu_read(tick_cpu_sched.tick_stopped);
+}
+
 extern void tick_nohz_idle_enter(void);
 extern void tick_nohz_idle_exit(void);
 extern void tick_nohz_irq_exit(void);
 extern ktime_t tick_nohz_get_sleep_length(void);
 extern u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time);
 extern u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time);
-# else
+
+# else /* !CONFIG_NO_HZ */
+static inline int tick_nohz_tick_stopped(void)
+{
+	return 0;
+}
+
 static inline void tick_nohz_idle_enter(void) { }
 static inline void tick_nohz_idle_exit(void) { }
 
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index f423bdd..ccc1971 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -28,7 +28,7 @@
 /*
  * Per cpu nohz control structure
  */
-static DEFINE_PER_CPU(struct tick_sched, tick_cpu_sched);
+DEFINE_PER_CPU(struct tick_sched, tick_cpu_sched);
 
 /*
  * The time, when the last jiffy update happened. Protected by xtime_lock.
-- 
1.7.5.4

[RFC PATCH 5/5] printk: Wake up klogd with irq_work on nohz CPU

[Frederic Weisbecker]

klogd is woken up asynchronously from the tick in order
to do it safely.

However if printk is called when the tick is stopped, the reader
won't be woken up until the next interrupt, which might not fire
before a while. As a result, the user may miss some message.

To fix this we try to schedule the wake up into an irq work
when the tick is stopped and irq work is not implemented on top
of the tick.

Ideally we could always rely on irq work for this to simplify the
code. But this may result in too much interrupts in case we have
a lot of printk calls in a short period of time. So we do this when
the tick is stopped only.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/printk.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 66a2ea3..c8ab918 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -42,6 +42,8 @@
 #include <linux/notifier.h>
 #include <linux/rculist.h>
 #include <linux/poll.h>
+#include <linux/tick.h>
+#include <linux/irq_work.h>
 
 #include <asm/uaccess.h>
 
@@ -1976,10 +1978,50 @@ int printk_needs_cpu(int cpu)
 	return __this_cpu_read(printk_pending);
 }
 
+#ifdef CONFIG_IRQ_WORK
+static void wake_klogd_irq_work(struct irq_work *irq_work)
+{
+	printk_tick();
+}
+#endif
+
+/*
+ * When the tick is stopped, we need another way to wake up
+ * klogd safely.
+ */
+static void wake_up_klogd_nohz(void)
+{
+	/*
+	 * If irq work is not itself implemented using the tick
+	 * it's a safe and fast way to wake up the reader.
+	 */
+#ifdef CONFIG_IRQ_WORK
+	if (!arch_irq_work_use_tick()) {
+		static struct irq_work klogd_irq_work = {
+			.func = wake_klogd_irq_work
+		};
+
+		irq_work_queue(&klogd_irq_work);
+		return;
+	}
+#endif
+	/*
+	 * Our last resort in the case of idle is to bet
+	 * on the fact we haven't yet reached the last need_resched()
+	 * check before the CPU goes to halt. This way we go through
+	 * another idle loop to recheck printk_needs_cpu().
+	 */
+	if (is_idle_task(current))
+		set_need_resched();
+}
+
 void wake_up_klogd(void)
 {
 	if (waitqueue_active(&log_wait))
 		this_cpu_or(printk_pending, PRINTK_PENDING_WAKEUP);
+
+	if (tick_nohz_tick_stopped())
+		wake_up_klogd_nohz();
 }
 
 static void console_cont_flush(char *text, size_t size)
-- 
1.7.5.4

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Catalin Marinas]

On 12 October 2012 19:09, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
> new file mode 100644
> index 0000000..f1bffa2
> --- /dev/null
> +++ b/arch/arm/include/asm/irq_work.h
> @@ -0,0 +1 @@
> +#include <asm-generic/irq_work.h>
> diff --git a/arch/arm64/include/asm/irq_work.h b/arch/arm64/include/asm/irq_work.h
> new file mode 100644
> index 0000000..f1bffa2
> --- /dev/null
> +++ b/arch/arm64/include/asm/irq_work.h
> @@ -0,0 +1 @@
> +#include <asm-generic/irq_work.h>

Just add generic-y += irq_work.h to arch/*/include/asm/Kbuild to avoid
creating new files.

-- 
Catalin

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Steven Rostedt]

On Mon, 2012-10-15 at 17:11 +0100, Catalin Marinas wrote:
> On 12 October 2012 19:09, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
> > new file mode 100644
> > index 0000000..f1bffa2
> > --- /dev/null
> > +++ b/arch/arm/include/asm/irq_work.h
> > @@ -0,0 +1 @@
> > +#include <asm-generic/irq_work.h>
> > diff --git a/arch/arm64/include/asm/irq_work.h b/arch/arm64/include/asm/irq_work.h
> > new file mode 100644
> > index 0000000..f1bffa2
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/irq_work.h
> > @@ -0,0 +1 @@
> > +#include <asm-generic/irq_work.h>
> 
> Just add generic-y += irq_work.h to arch/*/include/asm/Kbuild to avoid
> creating new files.
> 

BTW, is there any rational reason that the include path lookup doesn't
just check for the files in include/asm-generic after looking in
arch/*/include/asm? 

Really, the best way would be just to add the default asm files into
include/asm-generic and be done with it. I hate the fact that we need to
touch every arch for every generic default file.

I'd be willing to take a look at fixing this too, when I get a chance.

-- Steve

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Frederic Weisbecker]

2012/10/15 Steven Rostedt <rostedt@goodmis.org>:
> On Mon, 2012-10-15 at 17:11 +0100, Catalin Marinas wrote:
>> On 12 October 2012 19:09, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> > diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
>> > new file mode 100644
>> > index 0000000..f1bffa2
>> > --- /dev/null
>> > +++ b/arch/arm/include/asm/irq_work.h
>> > @@ -0,0 +1 @@
>> > +#include <asm-generic/irq_work.h>
>> > diff --git a/arch/arm64/include/asm/irq_work.h b/arch/arm64/include/asm/irq_work.h
>> > new file mode 100644
>> > index 0000000..f1bffa2
>> > --- /dev/null
>> > +++ b/arch/arm64/include/asm/irq_work.h
>> > @@ -0,0 +1 @@
>> > +#include <asm-generic/irq_work.h>
>>
>> Just add generic-y += irq_work.h to arch/*/include/asm/Kbuild to avoid
>> creating new files.
>>
>
> BTW, is there any rational reason that the include path lookup doesn't
> just check for the files in include/asm-generic after looking in
> arch/*/include/asm?
>
> Really, the best way would be just to add the default asm files into
> include/asm-generic and be done with it. I hate the fact that we need to
> touch every arch for every generic default file.

Agreed. I'm including Arnd in the conversation.

>
> I'd be willing to take a look at fixing this too, when I get a chance.
>
> -- Steve
>
>

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Steven Rostedt]

On Mon, 2012-10-15 at 22:23 +0200, Frederic Weisbecker wrote:
> 2012/10/15 Steven Rostedt <rostedt@goodmis.org>:
> > On Mon, 2012-10-15 at 17:11 +0100, Catalin Marinas wrote:
> >> On 12 October 2012 19:09, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> > diff --git a/arch/arm/include/asm/irq_work.h b/arch/arm/include/asm/irq_work.h
> >> > new file mode 100644
> >> > index 0000000..f1bffa2
> >> > --- /dev/null
> >> > +++ b/arch/arm/include/asm/irq_work.h
> >> > @@ -0,0 +1 @@
> >> > +#include <asm-generic/irq_work.h>
> >> > diff --git a/arch/arm64/include/asm/irq_work.h b/arch/arm64/include/asm/irq_work.h
> >> > new file mode 100644
> >> > index 0000000..f1bffa2
> >> > --- /dev/null
> >> > +++ b/arch/arm64/include/asm/irq_work.h
> >> > @@ -0,0 +1 @@
> >> > +#include <asm-generic/irq_work.h>
> >>
> >> Just add generic-y += irq_work.h to arch/*/include/asm/Kbuild to avoid
> >> creating new files.
> >>
> >
> > BTW, is there any rational reason that the include path lookup doesn't
> > just check for the files in include/asm-generic after looking in
> > arch/*/include/asm?
> >
> > Really, the best way would be just to add the default asm files into
> > include/asm-generic and be done with it. I hate the fact that we need to
> > touch every arch for every generic default file.
> 
> Agreed. I'm including Arnd in the conversation.

As David Howells is doing user space header work, I'll include him too.
Maybe someone can shed some light onto this.

-- Steve

> 
> >
> > I'd be willing to take a look at fixing this too, when I get a chance.
> >
> > -- Steve
> >
> >

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Arnd Bergmann]

On Monday 15 October 2012, Steven Rostedt wrote:
> On Mon, 2012-10-15 at 22:23 +0200, Frederic Weisbecker wrote:
> > 2012/10/15 Steven Rostedt <rostedt@goodmis.org>:
> > > On Mon, 2012-10-15 at 17:11 +0100, Catalin Marinas wrote:
> > > BTW, is there any rational reason that the include path lookup doesn't
> > > just check for the files in include/asm-generic after looking in
> > > arch/*/include/asm?
> > >
> > > Really, the best way would be just to add the default asm files into
> > > include/asm-generic and be done with it. I hate the fact that we need to
> > > touch every arch for every generic default file.
> > 
> > Agreed. I'm including Arnd in the conversation.
> 
> As David Howells is doing user space header work, I'll include him too.
> Maybe someone can shed some light onto this.
> 

A number of people have expressed the wish to do this through Makefile magic, but
so far nobody has been able to come up with the right incantation.

I've spent a day trying to figure it out, and I think Mark Brown tried some of
the same things. It's probably not all that hard for someone who is more familiar
with the Kbuild internals.

	Arnd

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Frederic Weisbecker]

2012/10/15 Arnd Bergmann <arnd@arndb.de>:
> On Monday 15 October 2012, Steven Rostedt wrote:
>> On Mon, 2012-10-15 at 22:23 +0200, Frederic Weisbecker wrote:
>> > 2012/10/15 Steven Rostedt <rostedt@goodmis.org>:
>> > > On Mon, 2012-10-15 at 17:11 +0100, Catalin Marinas wrote:
>> > > BTW, is there any rational reason that the include path lookup doesn't
>> > > just check for the files in include/asm-generic after looking in
>> > > arch/*/include/asm?
>> > >
>> > > Really, the best way would be just to add the default asm files into
>> > > include/asm-generic and be done with it. I hate the fact that we need to
>> > > touch every arch for every generic default file.
>> >
>> > Agreed. I'm including Arnd in the conversation.
>>
>> As David Howells is doing user space header work, I'll include him too.
>> Maybe someone can shed some light onto this.
>>
>
> A number of people have expressed the wish to do this through Makefile magic, but
> so far nobody has been able to come up with the right incantation.
>
> I've spent a day trying to figure it out, and I think Mark Brown tried some of
> the same things. It's probably not all that hard for someone who is more familiar
> with the Kbuild internals.

This seems to do the trick:

(It's the diff result of ln -s asm-generic include/asm)

diff --git a/include/asm b/include/asm
new file mode 120000
index 0000000..86e8787
--- /dev/null
+++ b/include/asm
@@ -0,0 +1 @@
+asm-generic/

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Mark Brown]

On Tue, Oct 16, 2012 at 12:18:05AM +0200, Frederic Weisbecker wrote:
> 2012/10/15 Arnd Bergmann <arnd@arndb.de>:
> > On Monday 15 October 2012, Steven Rostedt wrote:
> >> On Mon, 2012-10-15 at 22:23 +0200, Frederic Weisbecker wrote:
> >> > 2012/10/15 Steven Rostedt <rostedt@goodmis.org>:
> >> > > On Mon, 2012-10-15 at 17:11 +0100, Catalin Marinas wrote:

> >> > > BTW, is there any rational reason that the include path lookup doesn't
> >> > > just check for the files in include/asm-generic after looking in
> >> > > arch/*/include/asm?

> >> > > Really, the best way would be just to add the default asm files into
> >> > > include/asm-generic and be done with it. I hate the fact that we need to
> >> > > touch every arch for every generic default file.

> >> > Agreed. I'm including Arnd in the conversation.

> >> As David Howells is doing user space header work, I'll include him too.
> >> Maybe someone can shed some light onto this.

I'll just add my vote there, I've *no* idea why asm-generic isn't in the
include path by default, I could never figure out what that was for.

> > A number of people have expressed the wish to do this through Makefile magic, but
> > so far nobody has been able to come up with the right incantation.
> >
> > I've spent a day trying to figure it out, and I think Mark Brown tried some of
> > the same things. It's probably not all that hard for someone who is more familiar
> > with the Kbuild internals.

I came up with stuff for it, though it needed prettyfying.

> This seems to do the trick:

> (It's the diff result of ln -s asm-generic include/asm)

That'd work, but I assume there is some reason why we've got this system
of explicitly adding each file.  It's not like cpp can test for the
presence of include files.  If we can't figure out why we're not doing
this I'd propose we start.

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Arnd Bergmann]

On Tuesday 16 October 2012, Mark Brown wrote:
> On Tue, Oct 16, 2012 at 12:18:05AM +0200, Frederic Weisbecker wrote:
> 
> I came up with stuff for it, though it needed prettyfying.
> 
> > This seems to do the trick:
> 
> > (It's the diff result of ln -s asm-generic include/asm)

Unfortunately, this is not enough, it breaks "make headers_install",
which now needs to decide whether to install a header that is present
in the architecture directory, or to copy one from the asm-generic
directory otherwise.

On the plus side, the symlink breaks checking into CVS and cross-building
from operating systems that don't support posix file systems, so we
might want to do this anyway. ;-)

> That'd work, but I assume there is some reason why we've got this system
> of explicitly adding each file.  It's not like cpp can test for the
> presence of include files.  If we can't figure out why we're not doing
> this I'd propose we start.

We discussed renaming asm-generic to asm before, but some people objected
to the use of #include_next. There is a smaller problem with opening the
asm/*.h namespace to header files that are not relevant for architctures,
so I'd prefer to have a well-defined list of headers that are implicitly
shared, but it's not a technical argument.

	Arnd

Re: [RFC PATCH 1/5] irq_work: Move irq_work_raise() declaration/default definition to arch headers

[Mark Brown]

On Tue, Oct 16, 2012 at 09:25:11AM +0000, Arnd Bergmann wrote:
> On Tuesday 16 October 2012, Mark Brown wrote:

> > That'd work, but I assume there is some reason why we've got this system
> > of explicitly adding each file.  It's not like cpp can test for the
> > presence of include files.  If we can't figure out why we're not doing
> > this I'd propose we start.

> We discussed renaming asm-generic to asm before, but some people objected
> to the use of #include_next. There is a smaller problem with opening the
> asm/*.h namespace to header files that are not relevant for architctures,
> so I'd prefer to have a well-defined list of headers that are implicitly
> shared, but it's not a technical argument.

Perhaps what we need here is a more elegantly named
asm-default-implementation which is for the headers which almost every
architecture should be using (like clk and gpio) since the APIs should
be at least stubbed in order to avoid making the architecture terminally
annoying.

Alternatively we go to the gpiolib approach where I just made
architectures that want to do fun stuff select a Kconfig symbol and
otherwise the default is directly in the linux/ header.  This was
massively easier to deploy all round.

The case where the current situation is really annoying is the case
where we want to allow some sort of performance optimisation in
something that's normally there and probably doesn't need it; right now
the cost is on the API users to convince every single architecture
maintainer to adopt the API.

Re: [RFC PATCH 0/5] printk: Make it usable on nohz CPUs

[Frederic Weisbecker]

2012/10/12 Frederic Weisbecker <fweisbec@gmail.com>:
> Hi,
>
> So here is a proposition on what we can do to make printk
> correctly working on a tickless CPU.
>
> Although it's targeted to be part of the adaptive tickmess
> implemetation, it's pretty standalone and generic and also
> works for printk() calls in idle.
>
> It is based on latest linus tree.
>
> Waiting for your comments.

I've been thinking about this more and I now think we may want to
actually keep the irq_work_run() call in the sched tick but then
always implement the printk_tick() through an irq_work. This means:

- we can remove the direct call to printk_tick() in the sched tick. So
we have a single consolidated way to call that printk tick.
- we only send an IPI if the tick is stopped. Otherwise we wait for
the next tick to do the job (so we avoid IPI storms in case of
frequent printk calls)
- we remove printk_needs_cpu() and introduce irq_work_needs_cpu()
instead, in case we have pending irq works (not triggered with
self-IPIs) before we stop the tick.

I'm going to do this in the next round of this patchset.

>
> Thanks.
>
> PS: only built-tested for now.
>
> Frederic Weisbecker (5):
>   irq_work: Move irq_work_raise() declaration/default definition to
>     arch headers
>   irq_work: Only run irq_work from tick if arch needs it
>   x86: Implement arch_irq_work_use_tick
>   nohz: Add API to check tick state
>   printk: Wake up klogd with irq_work on nohz CPU
>
>  arch/alpha/include/asm/irq_work.h    |    9 +++++++
>  arch/alpha/kernel/time.c             |    2 +-
>  arch/arm/include/asm/irq_work.h      |    1 +
>  arch/arm64/include/asm/irq_work.h    |    1 +
>  arch/blackfin/include/asm/irq_work.h |    1 +
>  arch/frv/include/asm/irq_work.h      |    1 +
>  arch/hexagon/include/asm/irq_work.h  |    1 +
>  arch/mips/include/asm/irq_work.h     |    1 +
>  arch/parisc/include/asm/irq_work.h   |    1 +
>  arch/powerpc/include/asm/irq_work.h  |    8 ++++++
>  arch/powerpc/kernel/time.c           |    2 +-
>  arch/s390/include/asm/irq_work.h     |    1 +
>  arch/sh/include/asm/irq_work.h       |    1 +
>  arch/sparc/include/asm/irq_work.h    |    8 ++++++
>  arch/sparc/kernel/pcr.c              |    2 +-
>  arch/x86/include/asm/irq_work.h      |   15 ++++++++++++
>  arch/x86/kernel/irq_work.c           |    6 ++--
>  include/asm-generic/irq_work.h       |   22 +++++++++++++++++
>  include/linux/irq_work.h             |    1 +
>  include/linux/tick.h                 |   16 ++++++++++++-
>  kernel/irq_work.c                    |    7 -----
>  kernel/printk.c                      |   42 ++++++++++++++++++++++++++++++++++
>  kernel/time/tick-sched.c             |    2 +-
>  kernel/timer.c                       |    2 +-
>  24 files changed, 137 insertions(+), 16 deletions(-)
>  create mode 100644 arch/alpha/include/asm/irq_work.h
>  create mode 100644 arch/arm/include/asm/irq_work.h
>  create mode 100644 arch/arm64/include/asm/irq_work.h
>  create mode 100644 arch/blackfin/include/asm/irq_work.h
>  create mode 100644 arch/frv/include/asm/irq_work.h
>  create mode 100644 arch/hexagon/include/asm/irq_work.h
>  create mode 100644 arch/mips/include/asm/irq_work.h
>  create mode 100644 arch/parisc/include/asm/irq_work.h
>  create mode 100644 arch/powerpc/include/asm/irq_work.h
>  create mode 100644 arch/s390/include/asm/irq_work.h
>  create mode 100644 arch/sh/include/asm/irq_work.h
>  create mode 100644 arch/sparc/include/asm/irq_work.h
>  create mode 100644 arch/x86/include/asm/irq_work.h
>  create mode 100644 include/asm-generic/irq_work.h
>
> --
> 1.7.5.4
>