All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add generic idle notifiers
@ 2011-06-28  2:46 ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King
  Cc: linux-kernel, linux-arm-kernel, Todd Poynor

Add notifiers for idle entry and exit, called with IDLE_START when a
CPU goes idle and IDLE_END when it goes out of idle, based on the
existing idle notifiers for the x86_64 arch.

Convert x86_64 to use these notifiers, and call the notifiers on ARM.

Convert the ARM LEDs events for idle start/end to these notifiers.

 arch/arm/kernel/leds.c       |   27 ++++++++++++++++++++++++++-
 arch/arm/kernel/process.c    |    5 ++---
 arch/x86/include/asm/idle.h  |    7 -------
 arch/x86/kernel/process_64.c |   18 ++----------------
 include/linux/cpu.h          |    7 +++++++
 kernel/cpu.c                 |   20 ++++++++++++++++++++
 6 files changed, 57 insertions(+), 27 deletions(-)

-- 
1.7.3.1

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-06-28  2:46 ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: linux-arm-kernel

Add notifiers for idle entry and exit, called with IDLE_START when a
CPU goes idle and IDLE_END when it goes out of idle, based on the
existing idle notifiers for the x86_64 arch.

Convert x86_64 to use these notifiers, and call the notifiers on ARM.

Convert the ARM LEDs events for idle start/end to these notifiers.

 arch/arm/kernel/leds.c       |   27 ++++++++++++++++++++++++++-
 arch/arm/kernel/process.c    |    5 ++---
 arch/x86/include/asm/idle.h  |    7 -------
 arch/x86/kernel/process_64.c |   18 ++----------------
 include/linux/cpu.h          |    7 +++++++
 kernel/cpu.c                 |   20 ++++++++++++++++++++
 6 files changed, 57 insertions(+), 27 deletions(-)

-- 
1.7.3.1

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/3] Move x86_64 idle notifiers to generic
  2011-06-28  2:46 ` Todd Poynor
@ 2011-06-28  2:46   ` Todd Poynor
  -1 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King
  Cc: linux-kernel, linux-arm-kernel, Todd Poynor

Move the x86_64 idle notifiers originally by Andi Kleen and Venkatesh
Pallipadi to generic.

Change-Id: Idf29cda15be151f494ff245933c12462643388d5
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 arch/x86/include/asm/idle.h  |    7 -------
 arch/x86/kernel/process_64.c |   18 ++----------------
 include/linux/cpu.h          |    7 +++++++
 kernel/cpu.c                 |   20 ++++++++++++++++++++
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/idle.h b/arch/x86/include/asm/idle.h
index f49253d7..f1e4268 100644
--- a/arch/x86/include/asm/idle.h
+++ b/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);
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index ca6f7ab..63c8aed 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -56,31 +56,17 @@ asmlinkage extern void ret_from_fork(void);
 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);
+	idle_notifier_call_chain(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);
+	idle_notifier_call_chain(IDLE_END);
 }
 
 /* Called from interrupts to signify idle end */
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 5f09323..97f1ca7 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -174,4 +174,11 @@ static inline int disable_nonboot_cpus(void) { return 0; }
 static inline void enable_nonboot_cpus(void) {}
 #endif /* !CONFIG_PM_SLEEP_SMP */
 
+#define IDLE_START 1
+#define IDLE_END 2
+
+void idle_notifier_register(struct notifier_block *n);
+void idle_notifier_unregister(struct notifier_block *n);
+void idle_notifier_call_chain(unsigned long val);
+
 #endif /* _LINUX_CPU_H_ */
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 12b7458..4047707 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -594,3 +594,23 @@ void init_cpu_online(const struct cpumask *src)
 {
 	cpumask_copy(to_cpumask(cpu_online_bits), src);
 }
+
+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 idle_notifier_call_chain(unsigned long val)
+{
+	atomic_notifier_call_chain(&idle_notifier, val, NULL);
+}
+EXPORT_SYMBOL_GPL(idle_notifier_call_chain);
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 1/3] Move x86_64 idle notifiers to generic
@ 2011-06-28  2:46   ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: linux-arm-kernel

Move the x86_64 idle notifiers originally by Andi Kleen and Venkatesh
Pallipadi to generic.

Change-Id: Idf29cda15be151f494ff245933c12462643388d5
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 arch/x86/include/asm/idle.h  |    7 -------
 arch/x86/kernel/process_64.c |   18 ++----------------
 include/linux/cpu.h          |    7 +++++++
 kernel/cpu.c                 |   20 ++++++++++++++++++++
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/idle.h b/arch/x86/include/asm/idle.h
index f49253d7..f1e4268 100644
--- a/arch/x86/include/asm/idle.h
+++ b/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);
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index ca6f7ab..63c8aed 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -56,31 +56,17 @@ asmlinkage extern void ret_from_fork(void);
 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);
+	idle_notifier_call_chain(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);
+	idle_notifier_call_chain(IDLE_END);
 }
 
 /* Called from interrupts to signify idle end */
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 5f09323..97f1ca7 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -174,4 +174,11 @@ static inline int disable_nonboot_cpus(void) { return 0; }
 static inline void enable_nonboot_cpus(void) {}
 #endif /* !CONFIG_PM_SLEEP_SMP */
 
+#define IDLE_START 1
+#define IDLE_END 2
+
+void idle_notifier_register(struct notifier_block *n);
+void idle_notifier_unregister(struct notifier_block *n);
+void idle_notifier_call_chain(unsigned long val);
+
 #endif /* _LINUX_CPU_H_ */
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 12b7458..4047707 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -594,3 +594,23 @@ void init_cpu_online(const struct cpumask *src)
 {
 	cpumask_copy(to_cpumask(cpu_online_bits), src);
 }
+
+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 idle_notifier_call_chain(unsigned long val)
+{
+	atomic_notifier_call_chain(&idle_notifier, val, NULL);
+}
+EXPORT_SYMBOL_GPL(idle_notifier_call_chain);
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] ARM: Call idle notifiers
  2011-06-28  2:46 ` Todd Poynor
@ 2011-06-28  2:46   ` Todd Poynor
  -1 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King
  Cc: linux-kernel, linux-arm-kernel, Todd Poynor

Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 arch/arm/kernel/process.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 5e1e541..1b9101e 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -184,6 +184,7 @@ void cpu_idle(void)
 	while (1) {
 		tick_nohz_stop_sched_tick(1);
 		leds_event(led_idle_start);
+		idle_notifier_call_chain(IDLE_START);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU
 			if (cpu_is_offline(smp_processor_id()))
@@ -208,6 +209,7 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
+		idle_notifier_call_chain(IDLE_END);
 		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();
 		schedule();
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] ARM: Call idle notifiers
@ 2011-06-28  2:46   ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: linux-arm-kernel

Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 arch/arm/kernel/process.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 5e1e541..1b9101e 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -184,6 +184,7 @@ void cpu_idle(void)
 	while (1) {
 		tick_nohz_stop_sched_tick(1);
 		leds_event(led_idle_start);
+		idle_notifier_call_chain(IDLE_START);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU
 			if (cpu_is_offline(smp_processor_id()))
@@ -208,6 +209,7 @@ void cpu_idle(void)
 			}
 		}
 		leds_event(led_idle_end);
+		idle_notifier_call_chain(IDLE_END);
 		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();
 		schedule();
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] ARM: Move leds idle start/stop calls to idle notifiers
  2011-06-28  2:46 ` Todd Poynor
@ 2011-06-28  2:46   ` Todd Poynor
  -1 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King
  Cc: linux-kernel, linux-arm-kernel, Todd Poynor

Change-Id: I5d8e4e85b17bbab7992ecb477f0bdb5e4138b166
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 arch/arm/kernel/leds.c    |   27 ++++++++++++++++++++++++++-
 arch/arm/kernel/process.c |    3 ---
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/leds.c b/arch/arm/kernel/leds.c
index 0f107dc..136e837 100644
--- a/arch/arm/kernel/leds.c
+++ b/arch/arm/kernel/leds.c
@@ -9,6 +9,8 @@
  */
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/notifier.h>
+#include <linux/cpu.h>
 #include <linux/sysdev.h>
 #include <linux/syscore_ops.h>
 
@@ -101,6 +103,25 @@ static struct syscore_ops leds_syscore_ops = {
 	.resume		= leds_resume,
 };
 
+static int leds_idle_notifier(struct notifier_block *nb, unsigned long val,
+                                void *data)
+{
+	switch (val) {
+	case IDLE_START:
+		leds_event(led_idle_start);
+		break;
+	case IDLE_END:
+		leds_event(led_idle_end);
+		break;
+	}
+
+	return 0;
+}
+
+static struct notifier_block leds_idle_nb = {
+	.notifier_call = leds_idle_notifier,
+};
+
 static int __init leds_init(void)
 {
 	int ret;
@@ -109,8 +130,12 @@ static int __init leds_init(void)
 		ret = sysdev_register(&leds_device);
 	if (ret == 0)
 		ret = sysdev_create_file(&leds_device, &attr_event);
-	if (ret == 0)
+
+	if (ret == 0) {
 		register_syscore_ops(&leds_syscore_ops);
+		idle_notifier_register(&leds_idle_nb);
+	}
+
 	return ret;
 }
 
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 1b9101e..6adf53f 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -32,7 +32,6 @@
 #include <linux/hw_breakpoint.h>
 
 #include <asm/cacheflush.h>
-#include <asm/leds.h>
 #include <asm/processor.h>
 #include <asm/system.h>
 #include <asm/thread_notify.h>
@@ -183,7 +182,6 @@ void cpu_idle(void)
 	/* endless idle loop with no priority at all */
 	while (1) {
 		tick_nohz_stop_sched_tick(1);
-		leds_event(led_idle_start);
 		idle_notifier_call_chain(IDLE_START);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU
@@ -208,7 +206,6 @@ void cpu_idle(void)
 				local_irq_enable();
 			}
 		}
-		leds_event(led_idle_end);
 		idle_notifier_call_chain(IDLE_END);
 		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();
-- 
1.7.3.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] ARM: Move leds idle start/stop calls to idle notifiers
@ 2011-06-28  2:46   ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-06-28  2:46 UTC (permalink / raw)
  To: linux-arm-kernel

Change-Id: I5d8e4e85b17bbab7992ecb477f0bdb5e4138b166
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 arch/arm/kernel/leds.c    |   27 ++++++++++++++++++++++++++-
 arch/arm/kernel/process.c |    3 ---
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/leds.c b/arch/arm/kernel/leds.c
index 0f107dc..136e837 100644
--- a/arch/arm/kernel/leds.c
+++ b/arch/arm/kernel/leds.c
@@ -9,6 +9,8 @@
  */
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/notifier.h>
+#include <linux/cpu.h>
 #include <linux/sysdev.h>
 #include <linux/syscore_ops.h>
 
@@ -101,6 +103,25 @@ static struct syscore_ops leds_syscore_ops = {
 	.resume		= leds_resume,
 };
 
+static int leds_idle_notifier(struct notifier_block *nb, unsigned long val,
+                                void *data)
+{
+	switch (val) {
+	case IDLE_START:
+		leds_event(led_idle_start);
+		break;
+	case IDLE_END:
+		leds_event(led_idle_end);
+		break;
+	}
+
+	return 0;
+}
+
+static struct notifier_block leds_idle_nb = {
+	.notifier_call = leds_idle_notifier,
+};
+
 static int __init leds_init(void)
 {
 	int ret;
@@ -109,8 +130,12 @@ static int __init leds_init(void)
 		ret = sysdev_register(&leds_device);
 	if (ret == 0)
 		ret = sysdev_create_file(&leds_device, &attr_event);
-	if (ret == 0)
+
+	if (ret == 0) {
 		register_syscore_ops(&leds_syscore_ops);
+		idle_notifier_register(&leds_idle_nb);
+	}
+
 	return ret;
 }
 
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 1b9101e..6adf53f 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -32,7 +32,6 @@
 #include <linux/hw_breakpoint.h>
 
 #include <asm/cacheflush.h>
-#include <asm/leds.h>
 #include <asm/processor.h>
 #include <asm/system.h>
 #include <asm/thread_notify.h>
@@ -183,7 +182,6 @@ void cpu_idle(void)
 	/* endless idle loop with no priority at all */
 	while (1) {
 		tick_nohz_stop_sched_tick(1);
-		leds_event(led_idle_start);
 		idle_notifier_call_chain(IDLE_START);
 		while (!need_resched()) {
 #ifdef CONFIG_HOTPLUG_CPU
@@ -208,7 +206,6 @@ void cpu_idle(void)
 				local_irq_enable();
 			}
 		}
-		leds_event(led_idle_end);
 		idle_notifier_call_chain(IDLE_END);
 		tick_nohz_restart_sched_tick();
 		preempt_enable_no_resched();
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-06-28  2:46 ` Todd Poynor
@ 2011-06-28  9:02   ` Bryan Wu
  -1 siblings, 0 replies; 30+ messages in thread
From: Bryan Wu @ 2011-06-28  9:02 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel

On Tue, Jun 28, 2011 at 10:46 AM, Todd Poynor <toddpoynor@google.com> wrote:
> Add notifiers for idle entry and exit, called with IDLE_START when a
> CPU goes idle and IDLE_END when it goes out of idle, based on the
> existing idle notifiers for the x86_64 arch.
>
> Convert x86_64 to use these notifiers, and call the notifiers on ARM.
>
> Convert the ARM LEDs events for idle start/end to these notifiers.
>

LinusW and I is working on consolidation LED events interface in ARM.
Please take a look at https://patchwork.kernel.org/patch/918792/

I do like to add notifiers into ledtrig-cpu driver, which can be also
shared by x86

Thanks,
-Bryan

>  arch/arm/kernel/leds.c       |   27 ++++++++++++++++++++++++++-
>  arch/arm/kernel/process.c    |    5 ++---
>  arch/x86/include/asm/idle.h  |    7 -------
>  arch/x86/kernel/process_64.c |   18 ++----------------
>  include/linux/cpu.h          |    7 +++++++
>  kernel/cpu.c                 |   20 ++++++++++++++++++++
>  6 files changed, 57 insertions(+), 27 deletions(-)
>
> --
> 1.7.3.1
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
Bryan Wu <bryan.wu@canonical.com>
Kernel Developer    +86.138-1617-6545 Mobile
Ubuntu Kernel Team
Canonical Ltd.      www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-06-28  9:02   ` Bryan Wu
  0 siblings, 0 replies; 30+ messages in thread
From: Bryan Wu @ 2011-06-28  9:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jun 28, 2011 at 10:46 AM, Todd Poynor <toddpoynor@google.com> wrote:
> Add notifiers for idle entry and exit, called with IDLE_START when a
> CPU goes idle and IDLE_END when it goes out of idle, based on the
> existing idle notifiers for the x86_64 arch.
>
> Convert x86_64 to use these notifiers, and call the notifiers on ARM.
>
> Convert the ARM LEDs events for idle start/end to these notifiers.
>

LinusW and I is working on consolidation LED events interface in ARM.
Please take a look at https://patchwork.kernel.org/patch/918792/

I do like to add notifiers into ledtrig-cpu driver, which can be also
shared by x86

Thanks,
-Bryan

> ?arch/arm/kernel/leds.c ? ? ? | ? 27 ++++++++++++++++++++++++++-
> ?arch/arm/kernel/process.c ? ?| ? ?5 ++---
> ?arch/x86/include/asm/idle.h ?| ? ?7 -------
> ?arch/x86/kernel/process_64.c | ? 18 ++----------------
> ?include/linux/cpu.h ? ? ? ? ?| ? ?7 +++++++
> ?kernel/cpu.c ? ? ? ? ? ? ? ? | ? 20 ++++++++++++++++++++
> ?6 files changed, 57 insertions(+), 27 deletions(-)
>
> --
> 1.7.3.1
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
Bryan Wu <bryan.wu@canonical.com>
Kernel Developer ? ?+86.138-1617-6545 Mobile
Ubuntu Kernel Team
Canonical Ltd. ? ? ?www.canonical.com
Ubuntu - Linux for human beings | www.ubuntu.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-06-28  2:46 ` Todd Poynor
@ 2011-06-28 18:25   ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-06-28 18:25 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel

On Mon, 27 Jun 2011, Todd Poynor wrote:

> Add notifiers for idle entry and exit, called with IDLE_START when a
> CPU goes idle and IDLE_END when it goes out of idle, based on the
> existing idle notifiers for the x86_64 arch.
> 
> Convert x86_64 to use these notifiers, and call the notifiers on ARM.
> 
> Convert the ARM LEDs events for idle start/end to these notifiers.

This is nice.

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

for all 3 patches.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-06-28 18:25   ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-06-28 18:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 27 Jun 2011, Todd Poynor wrote:

> Add notifiers for idle entry and exit, called with IDLE_START when a
> CPU goes idle and IDLE_END when it goes out of idle, based on the
> existing idle notifiers for the x86_64 arch.
> 
> Convert x86_64 to use these notifiers, and call the notifiers on ARM.
> 
> Convert the ARM LEDs events for idle start/end to these notifiers.

This is nice.

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

for all 3 patches.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-06-28  9:02   ` Bryan Wu
@ 2011-06-28 18:28     ` Nicolas Pitre
  -1 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-06-28 18:28 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Todd Poynor, Russell King, linux-kernel, Ingo Molnar,
	H. Peter Anvin, Thomas Gleixner, linux-arm-kernel

On Tue, 28 Jun 2011, Bryan Wu wrote:

> On Tue, Jun 28, 2011 at 10:46 AM, Todd Poynor <toddpoynor@google.com> wrote:
> > Add notifiers for idle entry and exit, called with IDLE_START when a
> > CPU goes idle and IDLE_END when it goes out of idle, based on the
> > existing idle notifiers for the x86_64 arch.
> >
> > Convert x86_64 to use these notifiers, and call the notifiers on ARM.
> >
> > Convert the ARM LEDs events for idle start/end to these notifiers.
> >
> 
> LinusW and I is working on consolidation LED events interface in ARM.
> Please take a look at https://patchwork.kernel.org/patch/918792/

There is no conflict between those two series as one deals with the way 
the LED events are generated and the other with the way those events are 
acted upon.  The LED event mechanism in the middle is still untouched 
(could be deprecated eventually when all its users have migrated to the 
common LED API but not yet).


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-06-28 18:28     ` Nicolas Pitre
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2011-06-28 18:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 28 Jun 2011, Bryan Wu wrote:

> On Tue, Jun 28, 2011 at 10:46 AM, Todd Poynor <toddpoynor@google.com> wrote:
> > Add notifiers for idle entry and exit, called with IDLE_START when a
> > CPU goes idle and IDLE_END when it goes out of idle, based on the
> > existing idle notifiers for the x86_64 arch.
> >
> > Convert x86_64 to use these notifiers, and call the notifiers on ARM.
> >
> > Convert the ARM LEDs events for idle start/end to these notifiers.
> >
> 
> LinusW and I is working on consolidation LED events interface in ARM.
> Please take a look at https://patchwork.kernel.org/patch/918792/

There is no conflict between those two series as one deals with the way 
the LED events are generated and the other with the way those events are 
acted upon.  The LED event mechanism in the middle is still untouched 
(could be deprecated eventually when all its users have migrated to the 
common LED API but not yet).


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-06-28  2:46 ` Todd Poynor
@ 2011-07-07 17:05   ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-07-07 17:05 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel, Colin Cross

Todd Poynor <toddpoynor@google.com> writes:

> Add notifiers for idle entry and exit, called with IDLE_START when a
> CPU goes idle and IDLE_END when it goes out of idle, based on the
> existing idle notifiers for the x86_64 arch.
>
> Convert x86_64 to use these notifiers, and call the notifiers on ARM.
>
> Convert the ARM LEDs events for idle start/end to these notifiers.

Is this intended to replace the more general CPU PM notifiers proposed
by Colin:

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052827.html

Kevin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-07-07 17:05   ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-07-07 17:05 UTC (permalink / raw)
  To: linux-arm-kernel

Todd Poynor <toddpoynor@google.com> writes:

> Add notifiers for idle entry and exit, called with IDLE_START when a
> CPU goes idle and IDLE_END when it goes out of idle, based on the
> existing idle notifiers for the x86_64 arch.
>
> Convert x86_64 to use these notifiers, and call the notifiers on ARM.
>
> Convert the ARM LEDs events for idle start/end to these notifiers.

Is this intended to replace the more general CPU PM notifiers proposed
by Colin:

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052827.html

Kevin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] ARM: Call idle notifiers
  2011-06-28  2:46   ` Todd Poynor
@ 2011-07-07 17:08     ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-07-07 17:08 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel

Todd Poynor <toddpoynor@google.com> writes:

> Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
> Signed-off-by: Todd Poynor <toddpoynor@google.com>

I don't think the notifiers should be called in ARM-generic code.
As discussed w/Colin in his proposal for the CPU PM notifiers, the
platform-specific code should decide when to run notifier chain.

To give an example, on OMAP we wouldn't want the notifier chain to be
run until the OMAP PM core has programmed the next states of the various
power domains.  That way the notifier functions could check the next
state to determine if their powerdomain is going to retention or off and
decide whether or not a context save/restore will be needed.

Kevin


> ---
>  arch/arm/kernel/process.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 5e1e541..1b9101e 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -184,6 +184,7 @@ void cpu_idle(void)
>  	while (1) {
>  		tick_nohz_stop_sched_tick(1);
>  		leds_event(led_idle_start);
> +		idle_notifier_call_chain(IDLE_START);
>  		while (!need_resched()) {
>  #ifdef CONFIG_HOTPLUG_CPU
>  			if (cpu_is_offline(smp_processor_id()))
> @@ -208,6 +209,7 @@ void cpu_idle(void)
>  			}
>  		}
>  		leds_event(led_idle_end);
> +		idle_notifier_call_chain(IDLE_END);
>  		tick_nohz_restart_sched_tick();
>  		preempt_enable_no_resched();
>  		schedule();

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/3] ARM: Call idle notifiers
@ 2011-07-07 17:08     ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-07-07 17:08 UTC (permalink / raw)
  To: linux-arm-kernel

Todd Poynor <toddpoynor@google.com> writes:

> Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
> Signed-off-by: Todd Poynor <toddpoynor@google.com>

I don't think the notifiers should be called in ARM-generic code.
As discussed w/Colin in his proposal for the CPU PM notifiers, the
platform-specific code should decide when to run notifier chain.

To give an example, on OMAP we wouldn't want the notifier chain to be
run until the OMAP PM core has programmed the next states of the various
power domains.  That way the notifier functions could check the next
state to determine if their powerdomain is going to retention or off and
decide whether or not a context save/restore will be needed.

Kevin


> ---
>  arch/arm/kernel/process.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 5e1e541..1b9101e 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -184,6 +184,7 @@ void cpu_idle(void)
>  	while (1) {
>  		tick_nohz_stop_sched_tick(1);
>  		leds_event(led_idle_start);
> +		idle_notifier_call_chain(IDLE_START);
>  		while (!need_resched()) {
>  #ifdef CONFIG_HOTPLUG_CPU
>  			if (cpu_is_offline(smp_processor_id()))
> @@ -208,6 +209,7 @@ void cpu_idle(void)
>  			}
>  		}
>  		leds_event(led_idle_end);
> +		idle_notifier_call_chain(IDLE_END);
>  		tick_nohz_restart_sched_tick();
>  		preempt_enable_no_resched();
>  		schedule();

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-07-07 17:05   ` Kevin Hilman
@ 2011-07-07 20:17     ` Todd Poynor
  -1 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-07-07 20:17 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel, Colin Cross

On Thu, Jul 07, 2011 at 10:05:10AM -0700, Kevin Hilman wrote:
> Todd Poynor <toddpoynor@google.com> writes:
> 
> > Add notifiers for idle entry and exit, called with IDLE_START when a
> > CPU goes idle and IDLE_END when it goes out of idle, based on the
> > existing idle notifiers for the x86_64 arch.
> >
> > Convert x86_64 to use these notifiers, and call the notifiers on ARM.
> >
> > Convert the ARM LEDs events for idle start/end to these notifiers.
> 
> Is this intended to replace the more general CPU PM notifiers proposed
> by Colin:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052827.html

This is intended to coexist with the CPU PM notifiers.  The idle
notifiers proposed here are notifying of a change in kernel scheduler
state: the scheduler has no task to run and is entering its "idle
loop", or now has something to run and is exiting idle state.
Things that can use this include power management hints such as the
existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
and there's probably other existing potential uses in other arch'es that
I should go track down.

The CPU PM notifiers notify of changes in hardware power state that
affect the CPU and closely associated IP blocks (such as an OMAP
power state transition that causes one or more CPU power domains to
hit OFF).  These changes may be due to cpuidle power state management
when the system is idle, or may be due to suspending or resuming the
system (such as a suspend-to-RAM), or may be due to a CPU hotplug event.
In the case of an idle loop entry, if cpuidle decides it is not
appropriate to change CPU power state (as when insufficient time
remains until next timer expiry to enter such a power state due to
entry and exit latency), there may be no CPU PM notification
generated.  The callbacks for CPU PM notifiers on ARM do things such
as save/restore GIC interrupt controller state and VFP floating-point
coprocessor state.

That's the intent, anyhow, but ideas are welcome, thanks,


Todd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-07-07 20:17     ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-07-07 20:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 07, 2011 at 10:05:10AM -0700, Kevin Hilman wrote:
> Todd Poynor <toddpoynor@google.com> writes:
> 
> > Add notifiers for idle entry and exit, called with IDLE_START when a
> > CPU goes idle and IDLE_END when it goes out of idle, based on the
> > existing idle notifiers for the x86_64 arch.
> >
> > Convert x86_64 to use these notifiers, and call the notifiers on ARM.
> >
> > Convert the ARM LEDs events for idle start/end to these notifiers.
> 
> Is this intended to replace the more general CPU PM notifiers proposed
> by Colin:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052827.html

This is intended to coexist with the CPU PM notifiers.  The idle
notifiers proposed here are notifying of a change in kernel scheduler
state: the scheduler has no task to run and is entering its "idle
loop", or now has something to run and is exiting idle state.
Things that can use this include power management hints such as the
existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
and there's probably other existing potential uses in other arch'es that
I should go track down.

The CPU PM notifiers notify of changes in hardware power state that
affect the CPU and closely associated IP blocks (such as an OMAP
power state transition that causes one or more CPU power domains to
hit OFF).  These changes may be due to cpuidle power state management
when the system is idle, or may be due to suspending or resuming the
system (such as a suspend-to-RAM), or may be due to a CPU hotplug event.
In the case of an idle loop entry, if cpuidle decides it is not
appropriate to change CPU power state (as when insufficient time
remains until next timer expiry to enter such a power state due to
entry and exit latency), there may be no CPU PM notification
generated.  The callbacks for CPU PM notifiers on ARM do things such
as save/restore GIC interrupt controller state and VFP floating-point
coprocessor state.

That's the intent, anyhow, but ideas are welcome, thanks,


Todd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-07-07 20:17     ` Todd Poynor
@ 2011-07-07 22:34       ` Kevin Hilman
  -1 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-07-07 22:34 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel, Colin Cross

Todd Poynor <toddpoynor@google.com> writes:

> On Thu, Jul 07, 2011 at 10:05:10AM -0700, Kevin Hilman wrote:
>> Todd Poynor <toddpoynor@google.com> writes:
>> 
>> > Add notifiers for idle entry and exit, called with IDLE_START when a
>> > CPU goes idle and IDLE_END when it goes out of idle, based on the
>> > existing idle notifiers for the x86_64 arch.
>> >
>> > Convert x86_64 to use these notifiers, and call the notifiers on ARM.
>> >
>> > Convert the ARM LEDs events for idle start/end to these notifiers.
>> 
>> Is this intended to replace the more general CPU PM notifiers proposed
>> by Colin:
>> 
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052827.html
>
> This is intended to coexist with the CPU PM notifiers.  The idle
> notifiers proposed here are notifying of a change in kernel scheduler
> state: the scheduler has no task to run and is entering its "idle
> loop", or now has something to run and is exiting idle state.
> Things that can use this include power management hints such as the
> existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
> and there's probably other existing potential uses in other arch'es that
> I should go track down.
>
> The CPU PM notifiers notify of changes in hardware power state that
> affect the CPU and closely associated IP blocks (such as an OMAP
> power state transition that causes one or more CPU power domains to
> hit OFF).  These changes may be due to cpuidle power state management
> when the system is idle, or may be due to suspending or resuming the
> system (such as a suspend-to-RAM), or may be due to a CPU hotplug event.
> In the case of an idle loop entry, if cpuidle decides it is not
> appropriate to change CPU power state (as when insufficient time
> remains until next timer expiry to enter such a power state due to
> entry and exit latency), there may be no CPU PM notification
> generated.  The callbacks for CPU PM notifiers on ARM do things such
> as save/restore GIC interrupt controller state and VFP floating-point
> coprocessor state.
>
> That's the intent, anyhow, but ideas are welcome, thanks,

OK, thanks for the clarification.

Kevin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-07-07 22:34       ` Kevin Hilman
  0 siblings, 0 replies; 30+ messages in thread
From: Kevin Hilman @ 2011-07-07 22:34 UTC (permalink / raw)
  To: linux-arm-kernel

Todd Poynor <toddpoynor@google.com> writes:

> On Thu, Jul 07, 2011 at 10:05:10AM -0700, Kevin Hilman wrote:
>> Todd Poynor <toddpoynor@google.com> writes:
>> 
>> > Add notifiers for idle entry and exit, called with IDLE_START when a
>> > CPU goes idle and IDLE_END when it goes out of idle, based on the
>> > existing idle notifiers for the x86_64 arch.
>> >
>> > Convert x86_64 to use these notifiers, and call the notifiers on ARM.
>> >
>> > Convert the ARM LEDs events for idle start/end to these notifiers.
>> 
>> Is this intended to replace the more general CPU PM notifiers proposed
>> by Colin:
>> 
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052827.html
>
> This is intended to coexist with the CPU PM notifiers.  The idle
> notifiers proposed here are notifying of a change in kernel scheduler
> state: the scheduler has no task to run and is entering its "idle
> loop", or now has something to run and is exiting idle state.
> Things that can use this include power management hints such as the
> existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
> and there's probably other existing potential uses in other arch'es that
> I should go track down.
>
> The CPU PM notifiers notify of changes in hardware power state that
> affect the CPU and closely associated IP blocks (such as an OMAP
> power state transition that causes one or more CPU power domains to
> hit OFF).  These changes may be due to cpuidle power state management
> when the system is idle, or may be due to suspending or resuming the
> system (such as a suspend-to-RAM), or may be due to a CPU hotplug event.
> In the case of an idle loop entry, if cpuidle decides it is not
> appropriate to change CPU power state (as when insufficient time
> remains until next timer expiry to enter such a power state due to
> entry and exit latency), there may be no CPU PM notification
> generated.  The callbacks for CPU PM notifiers on ARM do things such
> as save/restore GIC interrupt controller state and VFP floating-point
> coprocessor state.
>
> That's the intent, anyhow, but ideas are welcome, thanks,

OK, thanks for the clarification.

Kevin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-07-07 20:17     ` Todd Poynor
@ 2011-07-08 11:13       ` Russell King - ARM Linux
  -1 siblings, 0 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2011-07-08 11:13 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Kevin Hilman, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	linux-kernel, linux-arm-kernel, Colin Cross

On Thu, Jul 07, 2011 at 01:17:48PM -0700, Todd Poynor wrote:
> This is intended to coexist with the CPU PM notifiers.  The idle
> notifiers proposed here are notifying of a change in kernel scheduler
> state: the scheduler has no task to run and is entering its "idle
> loop", or now has something to run and is exiting idle state.
> Things that can use this include power management hints such as the
> existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
> and there's probably other existing potential uses in other arch'es that
> I should go track down.

Isn't drivers/idle yet another cpuidle like implementation, but done
differently?  Is there any reason why the drivers/idle stuff can't
hook in like cpuidle or vice versa?

Isn't it possible for cpuidle to gain the same information via the idle
callback which these notifiers are providing drivers/idle with?

One of the problems I see with these notifiers is that it's not really
possible to keep an eye on the number of things hooked into them - and
in this path adding latency to the idle stuff could affect responsiveness.

Also bear in mind that at the point where idle starts, the decision about
what power state to idle in hasn't been decided, so nothing actually knows
whether it will be powered down, so all in all, I'm not sure what you'd
use these notifiers for.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-07-08 11:13       ` Russell King - ARM Linux
  0 siblings, 0 replies; 30+ messages in thread
From: Russell King - ARM Linux @ 2011-07-08 11:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 07, 2011 at 01:17:48PM -0700, Todd Poynor wrote:
> This is intended to coexist with the CPU PM notifiers.  The idle
> notifiers proposed here are notifying of a change in kernel scheduler
> state: the scheduler has no task to run and is entering its "idle
> loop", or now has something to run and is exiting idle state.
> Things that can use this include power management hints such as the
> existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
> and there's probably other existing potential uses in other arch'es that
> I should go track down.

Isn't drivers/idle yet another cpuidle like implementation, but done
differently?  Is there any reason why the drivers/idle stuff can't
hook in like cpuidle or vice versa?

Isn't it possible for cpuidle to gain the same information via the idle
callback which these notifiers are providing drivers/idle with?

One of the problems I see with these notifiers is that it's not really
possible to keep an eye on the number of things hooked into them - and
in this path adding latency to the idle stuff could affect responsiveness.

Also bear in mind that at the point where idle starts, the decision about
what power state to idle in hasn't been decided, so nothing actually knows
whether it will be powered down, so all in all, I'm not sure what you'd
use these notifiers for.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] ARM: Call idle notifiers
  2011-06-28  2:46   ` Todd Poynor
@ 2011-07-11 19:50     ` Frederic Weisbecker
  -1 siblings, 0 replies; 30+ messages in thread
From: Frederic Weisbecker @ 2011-07-11 19:50 UTC (permalink / raw)
  To: Todd Poynor
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel

On Mon, Jun 27, 2011 at 07:46:29PM -0700, Todd Poynor wrote:
> Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> ---
>  arch/arm/kernel/process.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 5e1e541..1b9101e 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -184,6 +184,7 @@ void cpu_idle(void)
>  	while (1) {
>  		tick_nohz_stop_sched_tick(1);
>  		leds_event(led_idle_start);
> +		idle_notifier_call_chain(IDLE_START);
>  		while (!need_resched()) {
>  #ifdef CONFIG_HOTPLUG_CPU
>  			if (cpu_is_offline(smp_processor_id()))
> @@ -208,6 +209,7 @@ void cpu_idle(void)
>  			}
>  		}
>  		leds_event(led_idle_end);
> +		idle_notifier_call_chain(IDLE_END);
>  		tick_nohz_restart_sched_tick();
>  		preempt_enable_no_resched();
>  		schedule();

You seem to use this notifier with different semantics than x86.
x86 notifies idle state when it knows it goes to sleep and exit it any
time it gets interrupted. And it does that every time in the need_resched()
loop.

But here in ARM you enter idle only once before the loop (and you don't even
know if you will enter the loop). And you don't notify idle exit state on interrupts.

So if in the end this idle notifier is something that is really wanted, it needs
to have a consistant behaviour across archs.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/3] ARM: Call idle notifiers
@ 2011-07-11 19:50     ` Frederic Weisbecker
  0 siblings, 0 replies; 30+ messages in thread
From: Frederic Weisbecker @ 2011-07-11 19:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 27, 2011 at 07:46:29PM -0700, Todd Poynor wrote:
> Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
> Signed-off-by: Todd Poynor <toddpoynor@google.com>
> ---
>  arch/arm/kernel/process.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 5e1e541..1b9101e 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -184,6 +184,7 @@ void cpu_idle(void)
>  	while (1) {
>  		tick_nohz_stop_sched_tick(1);
>  		leds_event(led_idle_start);
> +		idle_notifier_call_chain(IDLE_START);
>  		while (!need_resched()) {
>  #ifdef CONFIG_HOTPLUG_CPU
>  			if (cpu_is_offline(smp_processor_id()))
> @@ -208,6 +209,7 @@ void cpu_idle(void)
>  			}
>  		}
>  		leds_event(led_idle_end);
> +		idle_notifier_call_chain(IDLE_END);
>  		tick_nohz_restart_sched_tick();
>  		preempt_enable_no_resched();
>  		schedule();

You seem to use this notifier with different semantics than x86.
x86 notifies idle state when it knows it goes to sleep and exit it any
time it gets interrupted. And it does that every time in the need_resched()
loop.

But here in ARM you enter idle only once before the loop (and you don't even
know if you will enter the loop). And you don't notify idle exit state on interrupts.

So if in the end this idle notifier is something that is really wanted, it needs
to have a consistant behaviour across archs.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0/3] Add generic idle notifiers
  2011-07-08 11:13       ` Russell King - ARM Linux
@ 2011-07-11 22:10         ` Todd Poynor
  -1 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-07-11 22:10 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kevin Hilman, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	linux-kernel, linux-arm-kernel, Colin Cross

On Fri, Jul 08, 2011 at 12:13:09PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 07, 2011 at 01:17:48PM -0700, Todd Poynor wrote:
> > This is intended to coexist with the CPU PM notifiers.  The idle
> > notifiers proposed here are notifying of a change in kernel scheduler
> > state: the scheduler has no task to run and is entering its "idle
> > loop", or now has something to run and is exiting idle state.
> > Things that can use this include power management hints such as the
> > existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
> > and there's probably other existing potential uses in other arch'es that
> > I should go track down.
> 
> Isn't drivers/idle yet another cpuidle like implementation, but done
> differently?  Is there any reason why the drivers/idle stuff can't
> hook in like cpuidle or vice versa?
> 
> Isn't it possible for cpuidle to gain the same information via the idle
> callback which these notifiers are providing drivers/idle with?
> 
> One of the problems I see with these notifiers is that it's not really
> possible to keep an eye on the number of things hooked into them - and
> in this path adding latency to the idle stuff could affect responsiveness.
> 
> Also bear in mind that at the point where idle starts, the decision about
> what power state to idle in hasn't been decided, so nothing actually knows
> whether it will be powered down, so all in all, I'm not sure what you'd
> use these notifiers for.

Originally I was only targetting a couple of existing uses of callback
on each idle entry/exit, so no new functionality has been proposed
thus far.  I haven't looked at drivers/idle, but have also had the same
thought that some of the power management uses I started to envision could
instead be hooked up through cpuidle or the PM notifiers also under
discussion.

The other existing thing converted to the notifiers here is the ARM idle
LED triggers -- if these are to live on then it still seems nicer to me
to have calls to general notifiers in the ARM idle loop than direct
calls to the LED code.  (And the point about needing to be careful
about adding too much code to this performance-sensitive path is
well-taken.)

A potential use of OS idle entry/exit notifiers, which motivated these
patches, is something that wants to only run while the OS is running
something else, and wants to get out of the way when the OS is idle.  
(It's an ondemand-style cpufreq governor that only wants to sample load
when there's other work to be done, and wants to start sampling as soon
as there's work to do, and wants to cancel its timer when there's no
other work to be done).  As always, alternate suggestions for how to
go about that are welcome as well.



Todd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 0/3] Add generic idle notifiers
@ 2011-07-11 22:10         ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-07-11 22:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 08, 2011 at 12:13:09PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 07, 2011 at 01:17:48PM -0700, Todd Poynor wrote:
> > This is intended to coexist with the CPU PM notifiers.  The idle
> > notifiers proposed here are notifying of a change in kernel scheduler
> > state: the scheduler has no task to run and is entering its "idle
> > loop", or now has something to run and is exiting idle state.
> > Things that can use this include power management hints such as the
> > existing drivers/idle/i7300_idle.c usage, the ARM LEDs idle triggers,
> > and there's probably other existing potential uses in other arch'es that
> > I should go track down.
> 
> Isn't drivers/idle yet another cpuidle like implementation, but done
> differently?  Is there any reason why the drivers/idle stuff can't
> hook in like cpuidle or vice versa?
> 
> Isn't it possible for cpuidle to gain the same information via the idle
> callback which these notifiers are providing drivers/idle with?
> 
> One of the problems I see with these notifiers is that it's not really
> possible to keep an eye on the number of things hooked into them - and
> in this path adding latency to the idle stuff could affect responsiveness.
> 
> Also bear in mind that at the point where idle starts, the decision about
> what power state to idle in hasn't been decided, so nothing actually knows
> whether it will be powered down, so all in all, I'm not sure what you'd
> use these notifiers for.

Originally I was only targetting a couple of existing uses of callback
on each idle entry/exit, so no new functionality has been proposed
thus far.  I haven't looked at drivers/idle, but have also had the same
thought that some of the power management uses I started to envision could
instead be hooked up through cpuidle or the PM notifiers also under
discussion.

The other existing thing converted to the notifiers here is the ARM idle
LED triggers -- if these are to live on then it still seems nicer to me
to have calls to general notifiers in the ARM idle loop than direct
calls to the LED code.  (And the point about needing to be careful
about adding too much code to this performance-sensitive path is
well-taken.)

A potential use of OS idle entry/exit notifiers, which motivated these
patches, is something that wants to only run while the OS is running
something else, and wants to get out of the way when the OS is idle.  
(It's an ondemand-style cpufreq governor that only wants to sample load
when there's other work to be done, and wants to start sampling as soon
as there's work to do, and wants to cancel its timer when there's no
other work to be done).  As always, alternate suggestions for how to
go about that are welcome as well.



Todd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/3] ARM: Call idle notifiers
  2011-07-11 19:50     ` Frederic Weisbecker
@ 2011-07-13 22:53       ` Todd Poynor
  -1 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-07-13 22:53 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Russell King,
	linux-kernel, linux-arm-kernel

On Mon, Jul 11, 2011 at 09:50:04PM +0200, Frederic Weisbecker wrote:
> On Mon, Jun 27, 2011 at 07:46:29PM -0700, Todd Poynor wrote:
> > Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
> > Signed-off-by: Todd Poynor <toddpoynor@google.com>
> > ---
> >  arch/arm/kernel/process.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > index 5e1e541..1b9101e 100644
> > --- a/arch/arm/kernel/process.c
> > +++ b/arch/arm/kernel/process.c
> > @@ -184,6 +184,7 @@ void cpu_idle(void)
> >  	while (1) {
> >  		tick_nohz_stop_sched_tick(1);
> >  		leds_event(led_idle_start);
> > +		idle_notifier_call_chain(IDLE_START);
> >  		while (!need_resched()) {
> >  #ifdef CONFIG_HOTPLUG_CPU
> >  			if (cpu_is_offline(smp_processor_id()))
> > @@ -208,6 +209,7 @@ void cpu_idle(void)
> >  			}
> >  		}
> >  		leds_event(led_idle_end);
> > +		idle_notifier_call_chain(IDLE_END);
> >  		tick_nohz_restart_sched_tick();
> >  		preempt_enable_no_resched();
> >  		schedule();
> 
> You seem to use this notifier with different semantics than x86.
> x86 notifies idle state when it knows it goes to sleep and exit it any
> time it gets interrupted. And it does that every time in the need_resched()
> loop.
> 
> But here in ARM you enter idle only once before the loop (and you don't even
> know if you will enter the loop). And you don't notify idle exit state on interrupts.
> 
> So if in the end this idle notifier is something that is really wanted, it needs
> to have a consistant behaviour across archs.

Yes, I didn't want to change the behavior of the existing notifiers
being converted in these patches, but eventually one would want
cross-arch idle callbacks with consistent behavior, and the
existing arch-specific notifications have different semantics.
My goal is to notify when the OS scheduler enters and exits its idle
loop, so the existing ARM semantics are what I'm aiming for; the x86_64
behavior is probably further evidence that it is really a
cpuidle-style operation being performed.  If the idle notifiers do
find any traction then it sounds like moving the notification to the
common schedule code would be the right thing to do.


Thanks -- Todd

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2/3] ARM: Call idle notifiers
@ 2011-07-13 22:53       ` Todd Poynor
  0 siblings, 0 replies; 30+ messages in thread
From: Todd Poynor @ 2011-07-13 22:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 11, 2011 at 09:50:04PM +0200, Frederic Weisbecker wrote:
> On Mon, Jun 27, 2011 at 07:46:29PM -0700, Todd Poynor wrote:
> > Change-Id: Id833e61c13baa1783705ac9e9046d1f0cc90c95e
> > Signed-off-by: Todd Poynor <toddpoynor@google.com>
> > ---
> >  arch/arm/kernel/process.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > index 5e1e541..1b9101e 100644
> > --- a/arch/arm/kernel/process.c
> > +++ b/arch/arm/kernel/process.c
> > @@ -184,6 +184,7 @@ void cpu_idle(void)
> >  	while (1) {
> >  		tick_nohz_stop_sched_tick(1);
> >  		leds_event(led_idle_start);
> > +		idle_notifier_call_chain(IDLE_START);
> >  		while (!need_resched()) {
> >  #ifdef CONFIG_HOTPLUG_CPU
> >  			if (cpu_is_offline(smp_processor_id()))
> > @@ -208,6 +209,7 @@ void cpu_idle(void)
> >  			}
> >  		}
> >  		leds_event(led_idle_end);
> > +		idle_notifier_call_chain(IDLE_END);
> >  		tick_nohz_restart_sched_tick();
> >  		preempt_enable_no_resched();
> >  		schedule();
> 
> You seem to use this notifier with different semantics than x86.
> x86 notifies idle state when it knows it goes to sleep and exit it any
> time it gets interrupted. And it does that every time in the need_resched()
> loop.
> 
> But here in ARM you enter idle only once before the loop (and you don't even
> know if you will enter the loop). And you don't notify idle exit state on interrupts.
> 
> So if in the end this idle notifier is something that is really wanted, it needs
> to have a consistant behaviour across archs.

Yes, I didn't want to change the behavior of the existing notifiers
being converted in these patches, but eventually one would want
cross-arch idle callbacks with consistent behavior, and the
existing arch-specific notifications have different semantics.
My goal is to notify when the OS scheduler enters and exits its idle
loop, so the existing ARM semantics are what I'm aiming for; the x86_64
behavior is probably further evidence that it is really a
cpuidle-style operation being performed.  If the idle notifiers do
find any traction then it sounds like moving the notification to the
common schedule code would be the right thing to do.


Thanks -- Todd

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2011-07-13 22:54 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-28  2:46 [PATCH 0/3] Add generic idle notifiers Todd Poynor
2011-06-28  2:46 ` Todd Poynor
2011-06-28  2:46 ` [PATCH 1/3] Move x86_64 idle notifiers to generic Todd Poynor
2011-06-28  2:46   ` Todd Poynor
2011-06-28  2:46 ` [PATCH 2/3] ARM: Call idle notifiers Todd Poynor
2011-06-28  2:46   ` Todd Poynor
2011-07-07 17:08   ` Kevin Hilman
2011-07-07 17:08     ` Kevin Hilman
2011-07-11 19:50   ` Frederic Weisbecker
2011-07-11 19:50     ` Frederic Weisbecker
2011-07-13 22:53     ` Todd Poynor
2011-07-13 22:53       ` Todd Poynor
2011-06-28  2:46 ` [PATCH 3/3] ARM: Move leds idle start/stop calls to " Todd Poynor
2011-06-28  2:46   ` Todd Poynor
2011-06-28  9:02 ` [PATCH 0/3] Add generic " Bryan Wu
2011-06-28  9:02   ` Bryan Wu
2011-06-28 18:28   ` Nicolas Pitre
2011-06-28 18:28     ` Nicolas Pitre
2011-06-28 18:25 ` Nicolas Pitre
2011-06-28 18:25   ` Nicolas Pitre
2011-07-07 17:05 ` Kevin Hilman
2011-07-07 17:05   ` Kevin Hilman
2011-07-07 20:17   ` Todd Poynor
2011-07-07 20:17     ` Todd Poynor
2011-07-07 22:34     ` Kevin Hilman
2011-07-07 22:34       ` Kevin Hilman
2011-07-08 11:13     ` Russell King - ARM Linux
2011-07-08 11:13       ` Russell King - ARM Linux
2011-07-11 22:10       ` Todd Poynor
2011-07-11 22:10         ` Todd Poynor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.