[RFC PATCH 1/2] Idle notifier standardization (v2)

[RFC PATCH 1/2] Idle notifier standardization (v2)

[Mathieu Desnoyers]

Move idle notifiers into arch-agnostic code. Adapt x86 64 accordingly to call
the new architecture-agnostic notifiers rather than its own.

The architectures implementing the idle notifier define the config option:

CONFIG_HAVE_IDLE_NOTIFIER

Changelog since v1:
* Add CONFIG_HAVE_IDLE_NOTIFIER.


This is needed by the generic ring buffer. It needs to let the system sleep if
there is nothing going on other than tracing on a cpu, but for streaming it also
has to provide an upper bound on the delay before the information is sent out
(for merging across event streams coming from different CPUs). These notifiers
lets the ring buffer use deferrable timers to perform data delivery by forcing a
buffer flush before going to sleep.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 arch/x86/Kconfig             |    1 +
 arch/x86/include/asm/idle.h  |    7 -------
 arch/x86/kernel/process_64.c |   19 +++----------------
 drivers/idle/i7300_idle.c    |    5 +++--
 include/linux/idle.h         |   19 +++++++++++++++++++
 init/Kconfig                 |    3 +++
 kernel/notifier.c            |   25 +++++++++++++++++++++++++
 7 files changed, 54 insertions(+), 25 deletions(-)

Index: linux.trees.git/arch/x86/kernel/process_64.c
===================================================================
--- linux.trees.git.orig/arch/x86/kernel/process_64.c
+++ linux.trees.git/arch/x86/kernel/process_64.c
@@ -35,6 +35,7 @@
 #include <linux/tick.h>
 #include <linux/prctl.h>
 #include <linux/uaccess.h>
+#include <linux/idle.h>
 #include <linux/io.h>
 #include <linux/ftrace.h>
 
@@ -58,31 +59,17 @@ asmlinkage extern void ret_from_fork(voi
 DEFINE_PER_CPU(unsigned long, old_rsp);
 static DEFINE_PER_CPU(unsigned char, is_idle);
 
-static ATOMIC_NOTIFIER_HEAD(idle_notifier);
-
-void idle_notifier_register(struct notifier_block *n)
-{
-	atomic_notifier_chain_register(&idle_notifier, n);
-}
-EXPORT_SYMBOL_GPL(idle_notifier_register);
-
-void idle_notifier_unregister(struct notifier_block *n)
-{
-	atomic_notifier_chain_unregister(&idle_notifier, n);
-}
-EXPORT_SYMBOL_GPL(idle_notifier_unregister);
-
 void enter_idle(void)
 {
 	percpu_write(is_idle, 1);
-	atomic_notifier_call_chain(&idle_notifier, IDLE_START, NULL);
+	notify_idle(IDLE_START);
 }
 
 static void __exit_idle(void)
 {
 	if (x86_test_and_clear_bit_percpu(0, is_idle) == 0)
 		return;
-	atomic_notifier_call_chain(&idle_notifier, IDLE_END, NULL);
+	notify_idle(IDLE_END);
 }
 
 /* Called from interrupts to signify idle end */
Index: linux.trees.git/arch/x86/include/asm/idle.h
===================================================================
--- linux.trees.git.orig/arch/x86/include/asm/idle.h
+++ linux.trees.git/arch/x86/include/asm/idle.h
@@ -1,13 +1,6 @@
 #ifndef _ASM_X86_IDLE_H
 #define _ASM_X86_IDLE_H
 
-#define IDLE_START 1
-#define IDLE_END 2
-
-struct notifier_block;
-void idle_notifier_register(struct notifier_block *n);
-void idle_notifier_unregister(struct notifier_block *n);
-
 #ifdef CONFIG_X86_64
 void enter_idle(void);
 void exit_idle(void);
Index: linux.trees.git/include/linux/idle.h
===================================================================
--- /dev/null
+++ linux.trees.git/include/linux/idle.h
@@ -0,0 +1,19 @@
+/*
+ * include/linux/idle.h - generic idle definition
+ *
+ */
+#ifndef _LINUX_IDLE_H_
+#define _LINUX_IDLE_H_
+
+#include <linux/notifier.h>
+
+enum idle_val {
+	IDLE_START = 1,
+	IDLE_END = 2,
+};
+
+int notify_idle(enum idle_val val);
+void register_idle_notifier(struct notifier_block *n);
+void unregister_idle_notifier(struct notifier_block *n);
+
+#endif /* _LINUX_IDLE_H_ */
Index: linux.trees.git/kernel/notifier.c
===================================================================
--- linux.trees.git.orig/kernel/notifier.c
+++ linux.trees.git/kernel/notifier.c
@@ -5,6 +5,7 @@
 #include <linux/rcupdate.h>
 #include <linux/vmalloc.h>
 #include <linux/reboot.h>
+#include <linux/idle.h>
 
 /*
  *	Notifier list for kernel code which wants to be called
@@ -584,3 +585,27 @@ int unregister_die_notifier(struct notif
 	return atomic_notifier_chain_unregister(&die_chain, nb);
 }
 EXPORT_SYMBOL_GPL(unregister_die_notifier);
+
+static ATOMIC_NOTIFIER_HEAD(idle_notifier);
+
+/*
+ * Trace last event before calling notifiers. Notifiers flush data from buffers
+ * before going to idle.
+ */
+int notrace notify_idle(enum idle_val val)
+{
+	return atomic_notifier_call_chain(&idle_notifier, val, NULL);
+}
+EXPORT_SYMBOL_GPL(notify_idle);
+
+void register_idle_notifier(struct notifier_block *n)
+{
+	atomic_notifier_chain_register(&idle_notifier, n);
+}
+EXPORT_SYMBOL_GPL(register_idle_notifier);
+
+void unregister_idle_notifier(struct notifier_block *n)
+{
+	atomic_notifier_chain_unregister(&idle_notifier, n);
+}
+EXPORT_SYMBOL_GPL(unregister_idle_notifier);
Index: linux.trees.git/drivers/idle/i7300_idle.c
===================================================================
--- linux.trees.git.orig/drivers/idle/i7300_idle.c
+++ linux.trees.git/drivers/idle/i7300_idle.c
@@ -27,6 +27,7 @@
 #include <linux/debugfs.h>
 #include <linux/stop_machine.h>
 #include <linux/i7300_idle.h>
+#include <linux/idle.h>
 
 #include <asm/idle.h>
 
@@ -583,7 +584,7 @@ static int __init i7300_idle_init(void)
 		}
 	}
 
-	idle_notifier_register(&i7300_idle_nb);
+	register_idle_notifier(&i7300_idle_nb);
 
 	printk(KERN_INFO "i7300_idle: loaded v%s\n", I7300_IDLE_DRIVER_VERSION);
 	return 0;
@@ -591,7 +592,7 @@ static int __init i7300_idle_init(void)
 
 static void __exit i7300_idle_exit(void)
 {
-	idle_notifier_unregister(&i7300_idle_nb);
+	unregister_idle_notifier(&i7300_idle_nb);
 	free_cpumask_var(idle_cpumask);
 
 	if (debugfs_dir) {
Index: linux.trees.git/arch/x86/Kconfig
===================================================================
--- linux.trees.git.orig/arch/x86/Kconfig
+++ linux.trees.git/arch/x86/Kconfig
@@ -45,6 +45,7 @@ config X86
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_GENERIC_DMA_COHERENT if X86_32
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
+	select HAVE_IDLE_NOTIFIER if X86_64
 	select USER_STACKTRACE_SUPPORT
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_DMA_API_DEBUG
Index: linux.trees.git/init/Kconfig
===================================================================
--- linux.trees.git.orig/init/Kconfig
+++ linux.trees.git/init/Kconfig
@@ -1149,6 +1149,9 @@ source "arch/Kconfig"
 
 endmenu		# General setup
 
+config HAVE_IDLE_NOTIFIER
+	bool
+
 config HAVE_GENERIC_DMA_COHERENT
 	bool
 	default n
-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[RFC PATCH 2/2] Idle notifier standardization x86_32 (v2)

[Mathieu Desnoyers]

Add idle notifier callback to x86_32.

Changelog since v1:
* Add CONFIG_HAVE_IDLE_NOTIFIER.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 arch/x86/Kconfig             |    2 +-
 arch/x86/include/asm/idle.h  |    5 -----
 arch/x86/kernel/apm_32.c     |    6 ++++++
 arch/x86/kernel/process_32.c |   33 +++++++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 6 deletions(-)

Index: linux.trees.git/arch/x86/kernel/process_32.c
===================================================================
--- linux.trees.git.orig/arch/x86/kernel/process_32.c
+++ linux.trees.git/arch/x86/kernel/process_32.c
@@ -38,6 +38,8 @@
 #include <linux/uaccess.h>
 #include <linux/io.h>
 #include <linux/kdebug.h>
+#include <linux/notifier.h>
+#include <linux/idle.h>
 
 #include <asm/pgtable.h>
 #include <asm/system.h>
@@ -61,6 +63,30 @@
 
 asmlinkage void ret_from_fork(void) __asm__("ret_from_fork");
 
+static DEFINE_PER_CPU(unsigned char, is_idle);
+
+void enter_idle(void)
+{
+	percpu_write(is_idle, 1);
+	notify_idle(IDLE_START);
+}
+
+static void __exit_idle(void)
+{
+	if (x86_test_and_clear_bit_percpu(0, is_idle) == 0)
+		return;
+	notify_idle(IDLE_END);
+}
+
+/* Called from interrupts to signify idle end */
+void exit_idle(void)
+{
+	/* idle loop has pid 0 */
+	if (current->pid)
+		return;
+	__exit_idle();
+}
+
 /*
  * Return saved PC of a blocked thread.
  */
@@ -109,10 +135,17 @@ void cpu_idle(void)
 				play_dead();
 
 			local_irq_disable();
+			enter_idle();
 			/* Don't trace irqs off for idle */
 			stop_critical_timings();
 			pm_idle();
 			start_critical_timings();
+			/*
+			 * In many cases the interrupt that ended idle
+			 * has already called exit_idle. But some idle loops can
+			 * be woken up without interrupt.
+			 */
+			__exit_idle();
 
 			trace_power_end(smp_processor_id());
 		}
Index: linux.trees.git/arch/x86/include/asm/idle.h
===================================================================
--- linux.trees.git.orig/arch/x86/include/asm/idle.h
+++ linux.trees.git/arch/x86/include/asm/idle.h
@@ -1,13 +1,8 @@
 #ifndef _ASM_X86_IDLE_H
 #define _ASM_X86_IDLE_H
 
-#ifdef CONFIG_X86_64
 void enter_idle(void);
 void exit_idle(void);
-#else /* !CONFIG_X86_64 */
-static inline void enter_idle(void) { }
-static inline void exit_idle(void) { }
-#endif /* CONFIG_X86_64 */
 
 void c1e_remove_cpu(int cpu);
 
Index: linux.trees.git/arch/x86/kernel/apm_32.c
===================================================================
--- linux.trees.git.orig/arch/x86/kernel/apm_32.c
+++ linux.trees.git/arch/x86/kernel/apm_32.c
@@ -227,6 +227,7 @@
 #include <linux/suspend.h>
 #include <linux/kthread.h>
 #include <linux/jiffies.h>
+#include <linux/idle.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -947,10 +948,15 @@ recalc:
 				break;
 			}
 		}
+		enter_idle();
 		if (original_pm_idle)
 			original_pm_idle();
 		else
 			default_idle();
+		/* In many cases the interrupt that ended idle
+		   has already called exit_idle. But some idle
+		   loops can be woken up without interrupt. */
+		__exit_idle();
 		local_irq_disable();
 		jiffies_since_last_check = jiffies - last_jiffies;
 		if (jiffies_since_last_check > idle_period)
Index: linux.trees.git/arch/x86/Kconfig
===================================================================
--- linux.trees.git.orig/arch/x86/Kconfig
+++ linux.trees.git/arch/x86/Kconfig
@@ -45,7 +45,7 @@ config X86
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_GENERIC_DMA_COHERENT if X86_32
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
-	select HAVE_IDLE_NOTIFIER if X86_64
+	select HAVE_IDLE_NOTIFIER
 	select USER_STACKTRACE_SUPPORT
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_DMA_API_DEBUG
-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [RFC PATCH 1/2] Idle notifier standardization (v2)

[Thomas Gleixner]

On Wed, 8 Sep 2010, Mathieu Desnoyers wrote:

> Move idle notifiers into arch-agnostic code. Adapt x86 64 accordingly to call
> the new architecture-agnostic notifiers rather than its own.
> 
> The architectures implementing the idle notifier define the config option:
> 
> CONFIG_HAVE_IDLE_NOTIFIER
> 
> Changelog since v1:
> * Add CONFIG_HAVE_IDLE_NOTIFIER.
> 
> 
> This is needed by the generic ring buffer. It needs to let the system sleep if
> there is nothing going on other than tracing on a cpu, but for streaming it also
> has to provide an upper bound on the delay before the information is sent out
> (for merging across event streams coming from different CPUs). These notifiers
> lets the ring buffer use deferrable timers to perform data delivery by forcing a
> buffer flush before going to sleep.

I really have a hard time to understand how this is related to
deferrable timers. The whole point of deferrable timers is that they
do not fire when the machine is idle. 

I understand that you want to not care about the timer, but at the
same time you want to flush the buffer when going idle. 

So why do you keep the timer armed ? Just that it fires when the CPU
comes out of a long idle sleep and you flush the buffer again? So why
not cancel the timer on idle enter and rearm it when the machine
starts again?

So really, the reason why you want those notifiers is to flush the
buffer and _not_ to allow you the usage of deferrable timers.

Aside of that I really hate it to sprinkle the same notifier crap into
all arch idle functions - you even blindly copied the 64 bit
implementation to 32bit instead of moving it into the shared process.c
file.

The whole point of your exercise seems to be power saving related, so
why don't you hook that tracer flush stuff into
tick_nohz_stop_sched_tick() and tick_nohz_restart_sched_tick()
instead?  Those are called on idle enter and exit from all archs which
use NOHZ, so you should be all set. No need for adding that notifier
horror to every arch, really.

Thanks,

	tglx

Re: [RFC PATCH 1/2] Idle notifier standardization (v2)

[Mathieu Desnoyers]

* Thomas Gleixner (tglx@linutronix.de) wrote:
> On Wed, 8 Sep 2010, Mathieu Desnoyers wrote:
> 
> > Move idle notifiers into arch-agnostic code. Adapt x86 64 accordingly to call
> > the new architecture-agnostic notifiers rather than its own.
> > 
> > The architectures implementing the idle notifier define the config option:
> > 
> > CONFIG_HAVE_IDLE_NOTIFIER
> > 
> > Changelog since v1:
> > * Add CONFIG_HAVE_IDLE_NOTIFIER.
> > 
> > 
> > This is needed by the generic ring buffer. It needs to let the system sleep if
> > there is nothing going on other than tracing on a cpu, but for streaming it also
> > has to provide an upper bound on the delay before the information is sent out
> > (for merging across event streams coming from different CPUs). These notifiers
> > lets the ring buffer use deferrable timers to perform data delivery by forcing a
> > buffer flush before going to sleep.
> 
> I really have a hard time to understand how this is related to
> deferrable timers. The whole point of deferrable timers is that they
> do not fire when the machine is idle. 
> 
> I understand that you want to not care about the timer, but at the
> same time you want to flush the buffer when going idle. 
> 
> So why do you keep the timer armed ? Just that it fires when the CPU
> comes out of a long idle sleep and you flush the buffer again? So why
> not cancel the timer on idle enter and rearm it when the machine
> starts again?

That sounds exactly like what I am trying to achieve. Letting the timer fire
upon exit from idle was a side-effect I could really do without.

> 
> So really, the reason why you want those notifiers is to flush the
> buffer and _not_ to allow you the usage of deferrable timers.

Yep.

> 
> Aside of that I really hate it to sprinkle the same notifier crap into
> all arch idle functions - you even blindly copied the 64 bit
> implementation to 32bit instead of moving it into the shared process.c
> file.

Yep, I would have moved it to process.c, but I guess I'll hook on nohz instead.

> 
> The whole point of your exercise seems to be power saving related, so
> why don't you hook that tracer flush stuff into
> tick_nohz_stop_sched_tick() and tick_nohz_restart_sched_tick()
> instead?  Those are called on idle enter and exit from all archs which
> use NOHZ, so you should be all set. No need for adding that notifier
> horror to every arch, really.

Yep. I'll do that. Thanks a ton for looking into this.

Mathieu

> 
> Thanks,
> 
> 	tglx

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com