2013-03-21 22:02:56

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 05/34] idle: Implement generic idle function

All idle functions in arch/* are more or less the same, plus minus a
few bugs and extra instrumentation, tickless support and other
optional items.

Implement a generic idle function which resembles the functionality
found in arch/. Provide weak arch_cpu_idle_* functions which can be
overridden by the architecture code if needed.

Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/Kconfig | 3 +
include/linux/cpu.h | 8 +++
kernel/cpu/idle.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+)

Index: linux-2.6/arch/Kconfig
===================================================================
--- linux-2.6.orig/arch/Kconfig
+++ linux-2.6/arch/Kconfig
@@ -216,6 +216,9 @@ config USE_GENERIC_SMP_HELPERS
config GENERIC_SMP_IDLE_THREAD
bool

+config GENERIC_IDLE_LOOP
+ bool
+
# Select if arch init_task initializer is different to init/init_task.c
config ARCH_INIT_TASK
bool
Index: linux-2.6/include/linux/cpu.h
===================================================================
--- linux-2.6.orig/include/linux/cpu.h
+++ linux-2.6/include/linux/cpu.h
@@ -220,4 +220,12 @@ enum cpuhp_state {
void cpu_startup_entry(enum cpuhp_state state);
void cpu_idle(void);

+void cpu_idle_poll_ctrl(bool enable);
+
+void arch_cpu_idle(void);
+void arch_cpu_idle_prepare(void);
+void arch_cpu_idle_enter(void);
+void arch_cpu_idle_exit(void);
+void arch_cpu_idle_dead(void);
+
#endif /* _LINUX_CPU_H_ */
Index: linux-2.6/kernel/cpu/idle.c
===================================================================
--- linux-2.6.orig/kernel/cpu/idle.c
+++ linux-2.6/kernel/cpu/idle.c
@@ -3,8 +3,113 @@
*/
#include <linux/sched.h>
#include <linux/cpu.h>
+#include <linux/tick.h>
+#include <linux/mm.h>

+#include <asm/tlb.h>
+
+#include <trace/events/power.h>
+
+#ifndef CONFIG_GENERIC_IDLE_LOOP
void cpu_startup_entry(enum cpuhp_state state)
{
cpu_idle();
}
+#else
+
+static int __read_mostly cpu_idle_force_poll;
+
+void cpu_idle_poll_ctrl(bool enable)
+{
+ if (enable) {
+ cpu_idle_force_poll++;
+ } else {
+ cpu_idle_force_poll--;
+ WARN_ON_ONCE(cpu_idle_force_poll < 0);
+ }
+}
+
+#ifdef CONFIG_GENERIC_IDLE_POLL_SETUP
+static int __init cpu_idle_poll_setup(char *__unused)
+{
+ cpu_idle_force_poll = 1;
+ return 1;
+}
+__setup("nohlt", cpu_idle_poll_setup);
+
+static int __init cpu_idle_nopoll_setup(char *__unused)
+{
+ cpu_idle_force_poll = 0;
+ return 1;
+}
+__setup("hlt", cpu_idle_nopoll_setup);
+#endif
+
+static inline int cpu_idle_poll(void)
+{
+ trace_cpu_idle_rcuidle(0, smp_processor_id());
+ local_irq_enable();
+ while (!need_resched())
+ cpu_relax();
+ trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
+ return 1;
+}
+
+/* Weak implementations for optional arch specific functions */
+void __weak arch_cpu_idle_prepare(void) { }
+void __weak arch_cpu_idle_enter(void) { }
+void __weak arch_cpu_idle_exit(void) { }
+void __weak arch_cpu_idle_dead(void) { }
+void __weak arch_cpu_idle(void)
+{
+ cpu_idle_poll();
+}
+
+/*
+ * Generic idle loop implementation
+ */
+static void cpu_idle_loop(void)
+{
+ while (1) {
+ tick_nohz_idle_enter();
+
+ while (!need_resched()) {
+ check_pgt_cache();
+ rmb();
+
+ if (cpu_is_offline(smp_processor_id()))
+ arch_cpu_idle_dead();
+
+ local_irq_disable();
+ arch_cpu_idle_enter();
+
+ if (cpu_idle_force_poll) {
+ cpu_idle_poll();
+ } else {
+ current_clr_polling();
+ if (!need_resched()) {
+ stop_critical_timings();
+ rcu_idle_enter();
+ arch_cpu_idle();
+ WARN_ON_ONCE(!irqs_disabled());
+ rcu_idle_exit();
+ start_critical_timings();
+ } else {
+ local_irq_enable();
+ }
+ current_set_polling();
+ }
+ arch_cpu_idle_exit();
+ }
+ tick_nohz_idle_exit();
+ schedule_preempt_disabled();
+ }
+}
+
+void cpu_startup_entry(enum cpuhp_state state)
+{
+ current_set_polling();
+ arch_cpu_idle_prepare();
+ cpu_idle_loop();
+}
+#endif


2013-03-23 08:56:56

by Heiko Carstens

[permalink] [raw]
Subject: Re: [patch 05/34] idle: Implement generic idle function

On Thu, Mar 21, 2013 at 09:53:00PM -0000, Thomas Gleixner wrote:
> All idle functions in arch/* are more or less the same, plus minus a
> few bugs and extra instrumentation, tickless support and other
> optional items.
>
> Implement a generic idle function which resembles the functionality
> found in arch/. Provide weak arch_cpu_idle_* functions which can be
> overridden by the architecture code if needed.
>
> Signed-off-by: Thomas Gleixner <[email protected]>

[...]

> +static void cpu_idle_loop(void)
> +{
> + while (1) {
> + tick_nohz_idle_enter();
> +
> + while (!need_resched()) {
> + check_pgt_cache();
> + rmb();
> +
> + if (cpu_is_offline(smp_processor_id()))
> + arch_cpu_idle_dead();
> +
> + local_irq_disable();
> + arch_cpu_idle_enter();
> +
> + if (cpu_idle_force_poll) {
> + cpu_idle_poll();
> + } else {
> + current_clr_polling();
> + if (!need_resched()) {
> + stop_critical_timings();
> + rcu_idle_enter();
> + arch_cpu_idle();
> + WARN_ON_ONCE(!irqs_disabled());

This should be WARN_ON_ONCE(irqs_disabled()), no?

> + rcu_idle_exit();
> + start_critical_timings();
> + } else {
> + local_irq_enable();
> + }
> + current_set_polling();
> + }
> + arch_cpu_idle_exit();
> + }
> + tick_nohz_idle_exit();

I was wondering why the scheduler doesn't complain when being called with
irqs disabled. In fact tick_nohz_idle_exit() enables irqs unconditionally
iff CONFIG_NO_HZ is set.

> + schedule_preempt_disabled();
> + }
> +}
> +
> +void cpu_startup_entry(enum cpuhp_state state)
> +{
> + current_set_polling();
> + arch_cpu_idle_prepare();
> + cpu_idle_loop();
> +}
> +#endif

2013-03-25 10:39:29

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 05/34] idle: Implement generic idle function

On Sat, 23 Mar 2013, Heiko Carstens wrote:

> On Thu, Mar 21, 2013 at 09:53:00PM -0000, Thomas Gleixner wrote:
> > All idle functions in arch/* are more or less the same, plus minus a
> > few bugs and extra instrumentation, tickless support and other
> > optional items.
> >
> > Implement a generic idle function which resembles the functionality
> > found in arch/. Provide weak arch_cpu_idle_* functions which can be
> > overridden by the architecture code if needed.
> >
> > Signed-off-by: Thomas Gleixner <[email protected]>
>
> [...]
>
> > +static void cpu_idle_loop(void)
> > +{
> > + while (1) {
> > + tick_nohz_idle_enter();
> > +
> > + while (!need_resched()) {
> > + check_pgt_cache();
> > + rmb();
> > +
> > + if (cpu_is_offline(smp_processor_id()))
> > + arch_cpu_idle_dead();
> > +
> > + local_irq_disable();
> > + arch_cpu_idle_enter();
> > +
> > + if (cpu_idle_force_poll) {
> > + cpu_idle_poll();
> > + } else {
> > + current_clr_polling();
> > + if (!need_resched()) {
> > + stop_critical_timings();
> > + rcu_idle_enter();
> > + arch_cpu_idle();
> > + WARN_ON_ONCE(!irqs_disabled());
>
> This should be WARN_ON_ONCE(irqs_disabled()), no?

Gah, yes.

> > + rcu_idle_exit();
> > + start_critical_timings();
> > + } else {
> > + local_irq_enable();
> > + }
> > + current_set_polling();
> > + }
> > + arch_cpu_idle_exit();
> > + }
> > + tick_nohz_idle_exit();
>
> I was wondering why the scheduler doesn't complain when being called with
> irqs disabled. In fact tick_nohz_idle_exit() enables irqs unconditionally
> iff CONFIG_NO_HZ is set.

It should complain. I'll have a look again.

> > + schedule_preempt_disabled();
> > + }
> > +}
> > +
> > +void cpu_startup_entry(enum cpuhp_state state)
> > +{
> > + current_set_polling();
> > + arch_cpu_idle_prepare();
> > + cpu_idle_loop();
> > +}
> > +#endif
>
>

2013-03-28 15:42:12

by Srivatsa S. Bhat

[permalink] [raw]
Subject: Re: [patch 05/34] idle: Implement generic idle function

On 03/22/2013 03:23 AM, Thomas Gleixner wrote:
> All idle functions in arch/* are more or less the same, plus minus a
> few bugs and extra instrumentation, tickless support and other
> optional items.
>
> Implement a generic idle function which resembles the functionality
> found in arch/. Provide weak arch_cpu_idle_* functions which can be
> overridden by the architecture code if needed.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/Kconfig | 3 +
> include/linux/cpu.h | 8 +++
> kernel/cpu/idle.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 116 insertions(+)
[...]
> +
> +/*
> + * Generic idle loop implementation
> + */
> +static void cpu_idle_loop(void)
> +{
> + while (1) {
> + tick_nohz_idle_enter();
> +
> + while (!need_resched()) {
> + check_pgt_cache();
> + rmb();
> +
> + if (cpu_is_offline(smp_processor_id()))
> + arch_cpu_idle_dead();
> +
> + local_irq_disable();
> + arch_cpu_idle_enter();
> +
> + if (cpu_idle_force_poll) {
> + cpu_idle_poll();
> + } else {
> + current_clr_polling();
> + if (!need_resched()) {
> + stop_critical_timings();
> + rcu_idle_enter();
> + arch_cpu_idle();
> + WARN_ON_ONCE(!irqs_disabled());

I understand that you have changed it to WARN_ON_ONCE(irqs_disabled()) in
the v2 hosted in your git tree..

Sometime ago, Deepthi (in CC) had added such a check and Rafael had faced
some weird problems with that, and hence it was removed. Below are some
links to that discussion.

https://lkml.org/lkml/2012/6/22/48
https://lkml.org/lkml/2012/6/24/132
https://lkml.org/lkml/2012/6/25/402

Rafael, do you still see that problem after applying Thomas' patches?

> + rcu_idle_exit();
> + start_critical_timings();
> + } else {
> + local_irq_enable();
> + }
> + current_set_polling();
> + }
> + arch_cpu_idle_exit();
> + }
> + tick_nohz_idle_exit();
> + schedule_preempt_disabled();
> + }
> +}
> +
> +void cpu_startup_entry(enum cpuhp_state state)
> +{
> + current_set_polling();
> + arch_cpu_idle_prepare();
> + cpu_idle_loop();
> +}
> +#endif
>
>

Regards,
Srivatsa S. Bhat

Subject: [tip:smp/hotplug] idle: Implement generic idle function

Commit-ID: d166991234347215dc23fc9dc15a63a83a1a54e1
Gitweb: http://git.kernel.org/tip/d166991234347215dc23fc9dc15a63a83a1a54e1
Author: Thomas Gleixner <[email protected]>
AuthorDate: Thu, 21 Mar 2013 22:49:35 +0100
Committer: Thomas Gleixner <[email protected]>
CommitDate: Mon, 8 Apr 2013 17:39:23 +0200

idle: Implement generic idle function

All idle functions in arch/* are more or less the same, plus minus a
few bugs and extra instrumentation, tickless support and other
optional items.

Implement a generic idle function which resembles the functionality
found in arch/. Provide weak arch_cpu_idle_* functions which can be
overridden by the architecture code if needed.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Paul McKenney <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Reviewed-by: Cc: Srivatsa S. Bhat <[email protected]>
Cc: Magnus Damm <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/Kconfig | 3 ++
include/linux/cpu.h | 8 ++++
kernel/cpu/idle.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 1455579..a699f37 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -216,6 +216,9 @@ config USE_GENERIC_SMP_HELPERS
config GENERIC_SMP_IDLE_THREAD
bool

+config GENERIC_IDLE_LOOP
+ bool
+
# Select if arch init_task initializer is different to init/init_task.c
config ARCH_INIT_TASK
bool
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 7419e30..c6f6e08 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -220,4 +220,12 @@ enum cpuhp_state {
void cpu_startup_entry(enum cpuhp_state state);
void cpu_idle(void);

+void cpu_idle_poll_ctrl(bool enable);
+
+void arch_cpu_idle(void);
+void arch_cpu_idle_prepare(void);
+void arch_cpu_idle_enter(void);
+void arch_cpu_idle_exit(void);
+void arch_cpu_idle_dead(void);
+
#endif /* _LINUX_CPU_H_ */
diff --git a/kernel/cpu/idle.c b/kernel/cpu/idle.c
index 1908f00..54c3203 100644
--- a/kernel/cpu/idle.c
+++ b/kernel/cpu/idle.c
@@ -3,8 +3,113 @@
*/
#include <linux/sched.h>
#include <linux/cpu.h>
+#include <linux/tick.h>
+#include <linux/mm.h>

+#include <asm/tlb.h>
+
+#include <trace/events/power.h>
+
+#ifndef CONFIG_GENERIC_IDLE_LOOP
void cpu_startup_entry(enum cpuhp_state state)
{
cpu_idle();
}
+#else
+
+static int __read_mostly cpu_idle_force_poll;
+
+void cpu_idle_poll_ctrl(bool enable)
+{
+ if (enable) {
+ cpu_idle_force_poll++;
+ } else {
+ cpu_idle_force_poll--;
+ WARN_ON_ONCE(cpu_idle_force_poll < 0);
+ }
+}
+
+#ifdef CONFIG_GENERIC_IDLE_POLL_SETUP
+static int __init cpu_idle_poll_setup(char *__unused)
+{
+ cpu_idle_force_poll = 1;
+ return 1;
+}
+__setup("nohlt", cpu_idle_poll_setup);
+
+static int __init cpu_idle_nopoll_setup(char *__unused)
+{
+ cpu_idle_force_poll = 0;
+ return 1;
+}
+__setup("hlt", cpu_idle_nopoll_setup);
+#endif
+
+static inline int cpu_idle_poll(void)
+{
+ trace_cpu_idle_rcuidle(0, smp_processor_id());
+ local_irq_enable();
+ while (!need_resched())
+ cpu_relax();
+ trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
+ return 1;
+}
+
+/* Weak implementations for optional arch specific functions */
+void __weak arch_cpu_idle_prepare(void) { }
+void __weak arch_cpu_idle_enter(void) { }
+void __weak arch_cpu_idle_exit(void) { }
+void __weak arch_cpu_idle_dead(void) { }
+void __weak arch_cpu_idle(void)
+{
+ cpu_idle_force_poll = 1;
+}
+
+/*
+ * Generic idle loop implementation
+ */
+static void cpu_idle_loop(void)
+{
+ while (1) {
+ tick_nohz_idle_enter();
+
+ while (!need_resched()) {
+ check_pgt_cache();
+ rmb();
+
+ if (cpu_is_offline(smp_processor_id()))
+ arch_cpu_idle_dead();
+
+ local_irq_disable();
+ arch_cpu_idle_enter();
+
+ if (cpu_idle_force_poll) {
+ cpu_idle_poll();
+ } else {
+ current_clr_polling();
+ if (!need_resched()) {
+ stop_critical_timings();
+ rcu_idle_enter();
+ arch_cpu_idle();
+ WARN_ON_ONCE(irqs_disabled());
+ rcu_idle_exit();
+ start_critical_timings();
+ } else {
+ local_irq_enable();
+ }
+ current_set_polling();
+ }
+ arch_cpu_idle_exit();
+ }
+ tick_nohz_idle_exit();
+ schedule_preempt_disabled();
+ }
+}
+
+void cpu_startup_entry(enum cpuhp_state state)
+{
+ current_set_polling();
+ arch_cpu_idle_prepare();
+ cpu_idle_loop();
+}
+#endif

2013-04-15 22:25:32

by Tony Luck

[permalink] [raw]
Subject: Re: [tip:smp/hotplug] idle: Implement generic idle function

Built next-20130415 and got this on ia64 early in boot:

WARNING: at kernel/cpu/idle.c:94 cpu_idle_loop+0x360/0x380()
Hardware name: server rx2620
Modules linked in:

Call Trace:
[<a000000100014ac0>] show_stack+0x80/0xa0
sp=a000000101287c50 bsp=a000000101280e48
[<a000000100dabd40>] dump_stack+0x30/0x50
sp=a000000101287e20 bsp=a000000101280e30
[<a000000100073e40>] warn_slowpath_common+0xc0/0x100
sp=a000000101287e20 bsp=a000000101280de8
[<a000000100073ec0>] warn_slowpath_null+0x40/0x60
sp=a000000101287e20 bsp=a000000101280dc0
[<a0000001000fa7a0>] cpu_idle_loop+0x360/0x380
sp=a000000101287e20 bsp=a000000101280d80
[<a0000001000fa800>] cpu_startup_entry+0x40/0x60
sp=a000000101287e20 bsp=a000000101280d68
[<a000000100d9f460>] rest_init+0x100/0x120
sp=a000000101287e20 bsp=a000000101280d50
[<a00000010114d3b0>] start_kernel+0x770/0x890
sp=a000000101287e20 bsp=a000000101280cd0
[<a000000100d9f320>] start_ap+0x760/0x780
sp=a000000101287e30 bsp=a000000101280bc0

2013-04-16 13:28:15

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [tip:smp/hotplug] idle: Implement generic idle function

On Mon, 15 Apr 2013, Tony Luck wrote:

> Built next-20130415 and got this on ia64 early in boot:
>
> WARNING: at kernel/cpu/idle.c:94 cpu_idle_loop+0x360/0x380()

Hmm, is safe_halt() returning with interrupts disabled? If yes, it
lacks a local_irq_enable().

Thanks,

tglx

2013-04-16 18:25:52

by Tony Luck

[permalink] [raw]
Subject: Re: [tip:smp/hotplug] idle: Implement generic idle function

On Tue, Apr 16, 2013 at 6:28 AM, Thomas Gleixner <[email protected]> wrote:
> Hmm, is safe_halt() returning with interrupts disabled? If yes, it
> lacks a local_irq_enable().

Quite probably. Adding arch_local_irq_enable() to arch_safe_halt()
makes all the problems go away. I'll send you the one-line patch
from a system that won't mung it like gmail will.

-Tony

2013-04-16 18:36:11

by Luck, Tony

[permalink] [raw]
Subject: [PATCH] ia64: Make sure interrupts enabled when we "safe_halt()"

In commit d166991234347215dc23fc9dc15a63a83a1a54e1
idle: Implement generic idle function
Thomas Gleixner cleaned up many things but perturbed some
fragile code that was keeping ia64 alive. So we started
seeing:
WARNING: at kernel/cpu/idle.c:94 cpu_idle_loop+0x360/0x380()
and other unpleasantness like system hangs during boot.

We really shouldn't ever halt with interrupts disabled.

Signed-off-by: Tony Luck <[email protected]>

---

Please fold into the same branch as the generic idle changes.

diff --git a/arch/ia64/include/asm/irqflags.h b/arch/ia64/include/asm/irqflags.h
index 2b68d85..1bf2cf2 100644
--- a/arch/ia64/include/asm/irqflags.h
+++ b/arch/ia64/include/asm/irqflags.h
@@ -89,6 +89,7 @@ static inline bool arch_irqs_disabled(void)

static inline void arch_safe_halt(void)
{
+ arch_local_irq_enable();
ia64_pal_halt_light(); /* PAL_HALT_LIGHT */
}

Subject: [tip:smp/hotplug] ia64: Make sure interrupts enabled when we " safe_halt()"

Commit-ID: 2412aa1293a4159c610616305c17efd237c8208d
Gitweb: http://git.kernel.org/tip/2412aa1293a4159c610616305c17efd237c8208d
Author: Luck, Tony <[email protected]>
AuthorDate: Tue, 16 Apr 2013 11:35:56 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 17 Apr 2013 10:39:37 +0200

ia64: Make sure interrupts enabled when we "safe_halt()"

In commit d166991234347215dc23fc9dc15a63a83a1a54e1
idle: Implement generic idle function
Thomas Gleixner cleaned up many things but perturbed some
fragile code that was keeping ia64 alive. So we started
seeing:
WARNING: at kernel/cpu/idle.c:94 cpu_idle_loop+0x360/0x380()
and other unpleasantness like system hangs during boot.

We really shouldn't ever halt with interrupts disabled.

Signed-off-by: Tony Luck <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Rusty Russell <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srivatsa S. Bhat <[email protected]>
Cc: Paul McKenney <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>

---
arch/ia64/include/asm/irqflags.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/ia64/include/asm/irqflags.h b/arch/ia64/include/asm/irqflags.h
index 2b68d85..1bf2cf2 100644
--- a/arch/ia64/include/asm/irqflags.h
+++ b/arch/ia64/include/asm/irqflags.h
@@ -89,6 +89,7 @@ static inline bool arch_irqs_disabled(void)

static inline void arch_safe_halt(void)
{
+ arch_local_irq_enable();
ia64_pal_halt_light(); /* PAL_HALT_LIGHT */
}