2013-03-21 22:01:52

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 08/34] arm: Use generic idle loop

Use the generic idle loop and replace enable/disable_hlt with the
respective core functions.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
---
arch/arm/Kconfig | 2
arch/arm/include/asm/system_misc.h | 3 -
arch/arm/kernel/process.c | 96 ++++++++++---------------------------
arch/arm/kernel/smp.c | 2
arch/arm/mach-gemini/idle.c | 4 +
arch/arm/mach-gemini/irq.c | 2
arch/arm/mach-ixp4xx/common.c | 2
arch/arm/mach-omap1/pm.c | 5 -
arch/arm/mach-omap2/omap_hwmod.c | 6 +-
arch/arm/mach-omap2/pm.c | 5 -
arch/arm/mach-orion5x/board-dt.c | 2
arch/arm/mach-orion5x/common.c | 2
arch/arm/mach-shark/core.c | 2
arch/arm/mach-shmobile/suspend.c | 4 -
arch/arm/mach-w90x900/dev.c | 2
15 files changed, 47 insertions(+), 92 deletions(-)

Index: linux-2.6/arch/arm/Kconfig
===================================================================
--- linux-2.6.orig/arch/arm/Kconfig
+++ linux-2.6/arch/arm/Kconfig
@@ -15,6 +15,8 @@ config ARM
select GENERIC_IRQ_SHOW
select GENERIC_PCI_IOMAP
select GENERIC_SMP_IDLE_THREAD
+ select GENERIC_IDLE_LOOP
+ select GENERIC_IDLE_POLL_SETUP
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select HARDIRQS_SW_RESEND
Index: linux-2.6/arch/arm/include/asm/system_misc.h
===================================================================
--- linux-2.6.orig/arch/arm/include/asm/system_misc.h
+++ linux-2.6/arch/arm/include/asm/system_misc.h
@@ -21,9 +21,6 @@ extern void (*arm_pm_idle)(void);

extern unsigned int user_debug;

-extern void disable_hlt(void);
-extern void enable_hlt(void);
-
#endif /* !__ASSEMBLY__ */

#endif /* __ASM_ARM_SYSTEM_MISC_H */
Index: linux-2.6/arch/arm/kernel/process.c
===================================================================
--- linux-2.6.orig/arch/arm/kernel/process.c
+++ linux-2.6/arch/arm/kernel/process.c
@@ -57,34 +57,6 @@ static const char *isa_modes[] = {
"ARM" , "Thumb" , "Jazelle", "ThumbEE"
};

-static volatile int hlt_counter;
-
-void disable_hlt(void)
-{
- hlt_counter++;
-}
-
-void enable_hlt(void)
-{
- hlt_counter--;
- BUG_ON(hlt_counter < 0);
-}
-
-static int __init nohlt_setup(char *__unused)
-{
- hlt_counter = 1;
- return 1;
-}
-
-static int __init hlt_setup(char *__unused)
-{
- hlt_counter = 0;
- return 1;
-}
-
-__setup("nohlt", nohlt_setup);
-__setup("hlt", hlt_setup);
-
extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
typedef void (*phys_reset_t)(unsigned long);

@@ -168,54 +140,38 @@ static void default_idle(void)
local_irq_enable();
}

-/*
- * The idle thread.
- * We always respect 'hlt_counter' to prevent low power idle.
- */
-void cpu_idle(void)
+void arch_cpu_idle_prepare(void)
{
local_fiq_enable();
+}

- /* endless idle loop with no priority at all */
- while (1) {
- tick_nohz_idle_enter();
- rcu_idle_enter();
- ledtrig_cpu(CPU_LED_IDLE_START);
- while (!need_resched()) {
-#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(smp_processor_id()))
- cpu_die();
+void arch_cpu_idle_enter(void)
+{
+ ledtrig_cpu(CPU_LED_IDLE_START);
+#ifdef CONFIG_PL310_ERRATA_769419
+ wmb();
#endif
+}

- /*
- * We need to disable interrupts here
- * to ensure we don't miss a wakeup call.
- */
- local_irq_disable();
-#ifdef CONFIG_PL310_ERRATA_769419
- wmb();
+void arch_cpu_idle_exit(void)
+{
+ ledtrig_cpu(CPU_LED_IDLE_END);
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+void arch_cpu_idle_dead(void)
+{
+ cpu_die();
+}
#endif
- if (hlt_counter) {
- local_irq_enable();
- cpu_relax();
- } else if (!need_resched()) {
- stop_critical_timings();
- if (cpuidle_idle_call())
- default_idle();
- start_critical_timings();
- /*
- * default_idle functions must always
- * return with IRQs enabled.
- */
- WARN_ON(irqs_disabled());
- } else
- local_irq_enable();
- }
- ledtrig_cpu(CPU_LED_IDLE_END);
- rcu_idle_exit();
- tick_nohz_idle_exit();
- schedule_preempt_disabled();
- }
+
+/*
+ * Called from the core idle loop.
+ */
+void arch_cpu_idle(void)
+{
+ if (cpuidle_idle_call())
+ default_idle();
}

static char reboot_mode = 'h';
Index: linux-2.6/arch/arm/kernel/smp.c
===================================================================
--- linux-2.6.orig/arch/arm/kernel/smp.c
+++ linux-2.6/arch/arm/kernel/smp.c
@@ -336,7 +336,7 @@ asmlinkage void __cpuinit secondary_star
/*
* OK, it's off to the idle thread for us
*/
- cpu_idle();
+ cpu_startup_entry(CPUHP_ONLINE);
}

void __init smp_cpus_done(unsigned int max_cpus)
Index: linux-2.6/arch/arm/mach-gemini/idle.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-gemini/idle.c
+++ linux-2.6/arch/arm/mach-gemini/idle.c
@@ -13,9 +13,11 @@ static void gemini_idle(void)
* will never wakeup... Acctualy it is not very good to enable
* interrupts first since scheduler can miss a tick, but there is
* no other way around this. Platforms that needs it for power saving
- * should call enable_hlt() in init code, since by default it is
+ * should enable it in init code, since by default it is
* disabled.
*/
+
+ /* FIXME: Enabling interrupts here is racy! */
local_irq_enable();
cpu_do_idle();
}
Index: linux-2.6/arch/arm/mach-gemini/irq.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-gemini/irq.c
+++ linux-2.6/arch/arm/mach-gemini/irq.c
@@ -77,7 +77,7 @@ void __init gemini_init_irq(void)
* Disable the idle handler by default since it is buggy
* For more info see arch/arm/mach-gemini/idle.c
*/
- disable_hlt();
+ cpu_idle_poll_ctrl(true);

request_resource(&iomem_resource, &irq_resource);

Index: linux-2.6/arch/arm/mach-ixp4xx/common.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-ixp4xx/common.c
+++ linux-2.6/arch/arm/mach-ixp4xx/common.c
@@ -239,7 +239,7 @@ void __init ixp4xx_init_irq(void)
* ixp4xx does not implement the XScale PWRMODE register
* so it must not call cpu_do_idle().
*/
- disable_hlt();
+ cpu_idle_poll_ctrl(true);

/* Route all sources to IRQ instead of FIQ */
*IXP4XX_ICLR = 0x0;
Index: linux-2.6/arch/arm/mach-omap1/pm.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-omap1/pm.c
+++ linux-2.6/arch/arm/mach-omap1/pm.c
@@ -584,8 +584,7 @@ static void omap_pm_init_proc(void)
static int omap_pm_prepare(void)
{
/* We cannot sleep in idle until we have resumed */
- disable_hlt();
-
+ cpu_idle_poll_ctrl(true);
return 0;
}

@@ -621,7 +620,7 @@ static int omap_pm_enter(suspend_state_t

static void omap_pm_finish(void)
{
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
}


Index: linux-2.6/arch/arm/mach-omap2/omap_hwmod.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-omap2/omap_hwmod.c
+++ linux-2.6/arch/arm/mach-omap2/omap_hwmod.c
@@ -2154,7 +2154,7 @@ static int _enable(struct omap_hwmod *oh
if (soc_ops.enable_module)
soc_ops.enable_module(oh);
if (oh->flags & HWMOD_BLOCK_WFI)
- disable_hlt();
+ cpu_idle_poll_ctrl(true);

if (soc_ops.update_context_lost)
soc_ops.update_context_lost(oh);
@@ -2218,7 +2218,7 @@ static int _idle(struct omap_hwmod *oh)
_del_initiator_dep(oh, mpu_oh);

if (oh->flags & HWMOD_BLOCK_WFI)
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
if (soc_ops.disable_module)
soc_ops.disable_module(oh);

@@ -2328,7 +2328,7 @@ static int _shutdown(struct omap_hwmod *
_del_initiator_dep(oh, mpu_oh);
/* XXX what about the other system initiators here? dma, dsp */
if (oh->flags & HWMOD_BLOCK_WFI)
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
if (soc_ops.disable_module)
soc_ops.disable_module(oh);
_disable_clocks(oh);
Index: linux-2.6/arch/arm/mach-omap2/pm.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-omap2/pm.c
+++ linux-2.6/arch/arm/mach-omap2/pm.c
@@ -218,7 +218,7 @@ static int omap_pm_enter(suspend_state_t

static int omap_pm_begin(suspend_state_t state)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
if (cpu_is_omap34xx())
omap_prcm_irq_prepare();
return 0;
@@ -226,8 +226,7 @@ static int omap_pm_begin(suspend_state_t

static void omap_pm_end(void)
{
- enable_hlt();
- return;
+ cpu_idle_poll_ctrl(false);
}

static void omap_pm_finish(void)
Index: linux-2.6/arch/arm/mach-orion5x/board-dt.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-orion5x/board-dt.c
+++ linux-2.6/arch/arm/mach-orion5x/board-dt.c
@@ -52,7 +52,7 @@ static void __init orion5x_dt_init(void)
*/
if (dev == MV88F5281_DEV_ID && rev == MV88F5281_REV_D0) {
printk(KERN_INFO "Orion: Applying 5281 D0 WFI workaround.\n");
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
}

if (of_machine_is_compatible("lacie,ethernet-disk-mini-v2"))
Index: linux-2.6/arch/arm/mach-orion5x/common.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-orion5x/common.c
+++ linux-2.6/arch/arm/mach-orion5x/common.c
@@ -293,7 +293,7 @@ void __init orion5x_init(void)
*/
if (dev == MV88F5281_DEV_ID && rev == MV88F5281_REV_D0) {
printk(KERN_INFO "Orion: Applying 5281 D0 WFI workaround.\n");
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
}

/*
Index: linux-2.6/arch/arm/mach-shark/core.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shark/core.c
+++ linux-2.6/arch/arm/mach-shark/core.c
@@ -130,7 +130,7 @@ static void __init shark_timer_init(void

static void shark_init_early(void)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
}

MACHINE_START(SHARK, "Shark")
Index: linux-2.6/arch/arm/mach-shmobile/suspend.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/suspend.c
+++ linux-2.6/arch/arm/mach-shmobile/suspend.c
@@ -23,13 +23,13 @@ static int shmobile_suspend_default_ente

static int shmobile_suspend_begin(suspend_state_t state)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
return 0;
}

static void shmobile_suspend_end(void)
{
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
}

struct platform_suspend_ops shmobile_suspend_ops = {
Index: linux-2.6/arch/arm/mach-w90x900/dev.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-w90x900/dev.c
+++ linux-2.6/arch/arm/mach-w90x900/dev.c
@@ -531,7 +531,7 @@ static struct platform_device *nuc900_pu

void __init nuc900_board_init(struct platform_device **device, int size)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
platform_add_devices(device, size);
platform_add_devices(nuc900_public_dev, ARRAY_SIZE(nuc900_public_dev));
spi_register_board_info(nuc900_spi_board_info,


2013-03-22 21:24:59

by Kevin Hilman

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

Hi Thomas,

Thomas Gleixner <[email protected]> writes:

> Use the generic idle loop and replace enable/disable_hlt with the
> respective core functions.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Russell King <[email protected]>

I gave patches 1-5 + this a quick spin on ARM (specifially some OMAP3 and
OMAP4 platforms, with and without CPUidle enabled.)

The OMAP stuff needed a couple minor compile fixes (below), but
otherwise it passes the quick "seems to work" test.

So at least for ARM/OMAP:

Tested-by: Kevin Hilman <[email protected]>

Kevin


diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
index 23c653a..db37f49 100644
--- a/arch/arm/mach-omap1/pm.c
+++ b/arch/arm/mach-omap1/pm.c
@@ -43,6 +43,7 @@
#include <linux/module.h>
#include <linux/io.h>
#include <linux/atomic.h>
+#include <linux/cpu.h>

#include <asm/fncpy.h>
#include <asm/system_misc.h>
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 3a6c6b8..2f17f95 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -138,6 +138,7 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/bootmem.h>
+#include <linux/cpu.h>

#include <asm/system_misc.h>

2013-03-25 11:31:24

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Fri, 22 Mar 2013, Kevin Hilman wrote:

> Hi Thomas,
>
> Thomas Gleixner <[email protected]> writes:
>
> > Use the generic idle loop and replace enable/disable_hlt with the
> > respective core functions.
> >
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > Cc: Russell King <[email protected]>
>
> I gave patches 1-5 + this a quick spin on ARM (specifially some OMAP3 and
> OMAP4 platforms, with and without CPUidle enabled.)
>
> The OMAP stuff needed a couple minor compile fixes (below), but
> otherwise it passes the quick "seems to work" test.

Yeah, that's needed for the other arch/arm conversions as well.

Thanks,

tglx


> So at least for ARM/OMAP:
>
> Tested-by: Kevin Hilman <[email protected]>
>
> Kevin
>
>
> diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
> index 23c653a..db37f49 100644
> --- a/arch/arm/mach-omap1/pm.c
> +++ b/arch/arm/mach-omap1/pm.c
> @@ -43,6 +43,7 @@
> #include <linux/module.h>
> #include <linux/io.h>
> #include <linux/atomic.h>
> +#include <linux/cpu.h>
>
> #include <asm/fncpy.h>
> #include <asm/system_misc.h>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 3a6c6b8..2f17f95 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -138,6 +138,7 @@
> #include <linux/spinlock.h>
> #include <linux/slab.h>
> #include <linux/bootmem.h>
> +#include <linux/cpu.h>
>
> #include <asm/system_misc.h>
>

2013-03-25 11:49:32

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Mon, Mar 25, 2013 at 12:31:20PM +0100, Thomas Gleixner wrote:
> On Fri, 22 Mar 2013, Kevin Hilman wrote:
>
> > Hi Thomas,
> >
> > Thomas Gleixner <[email protected]> writes:
> >
> > > Use the generic idle loop and replace enable/disable_hlt with the
> > > respective core functions.
> > >
> > > Signed-off-by: Thomas Gleixner <[email protected]>
> > > Cc: Russell King <[email protected]>
> >
> > I gave patches 1-5 + this a quick spin on ARM (specifially some OMAP3 and
> > OMAP4 platforms, with and without CPUidle enabled.)
> >
> > The OMAP stuff needed a couple minor compile fixes (below), but
> > otherwise it passes the quick "seems to work" test.
>
> Yeah, that's needed for the other arch/arm conversions as well.

So, the only patch I got was this one, which is useless to test on its
own...

2013-03-25 14:02:45

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Mon, 25 Mar 2013, Russell King - ARM Linux wrote:

> On Mon, Mar 25, 2013 at 12:31:20PM +0100, Thomas Gleixner wrote:
> > On Fri, 22 Mar 2013, Kevin Hilman wrote:
> >
> > > Hi Thomas,
> > >
> > > Thomas Gleixner <[email protected]> writes:
> > >
> > > > Use the generic idle loop and replace enable/disable_hlt with the
> > > > respective core functions.
> > > >
> > > > Signed-off-by: Thomas Gleixner <[email protected]>
> > > > Cc: Russell King <[email protected]>
> > >
> > > I gave patches 1-5 + this a quick spin on ARM (specifially some OMAP3 and
> > > OMAP4 platforms, with and without CPUidle enabled.)
> > >
> > > The OMAP stuff needed a couple minor compile fixes (below), but
> > > otherwise it passes the quick "seems to work" test.
> >
> > Yeah, that's needed for the other arch/arm conversions as well.
>
> So, the only patch I got was this one, which is useless to test on its
> own...

I'm testing V2 now and will post an git URL with a testing branch
later tonight.

Thanks,

tglx

Subject: [tip:smp/hotplug] arm: Use generic idle loop

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

arm: Use generic idle loop

Use the generic idle loop and replace enable/disable_hlt with the
respective core functions.

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]>
Cc: Russell King <[email protected]>
Tested-by: Kevin Hilman <[email protected]> # OMAP
Link: http://lkml.kernel.org/r/[email protected]
---
arch/arm/Kconfig | 2 +
arch/arm/include/asm/system_misc.h | 3 --
arch/arm/kernel/process.c | 96 +++++++++++---------------------------
arch/arm/kernel/smp.c | 2 +-
arch/arm/mach-gemini/idle.c | 4 +-
arch/arm/mach-gemini/irq.c | 4 +-
arch/arm/mach-ixp4xx/common.c | 3 +-
arch/arm/mach-omap1/pm.c | 6 +--
arch/arm/mach-omap2/omap_hwmod.c | 7 +--
arch/arm/mach-omap2/pm.c | 5 +-
arch/arm/mach-orion5x/board-dt.c | 3 +-
arch/arm/mach-orion5x/common.c | 2 +-
arch/arm/mach-shark/core.c | 3 +-
arch/arm/mach-shmobile/suspend.c | 6 ++-
arch/arm/mach-w90x900/dev.c | 3 +-
15 files changed, 57 insertions(+), 92 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1cacda4..128551f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -15,6 +15,8 @@ config ARM
select GENERIC_IRQ_SHOW
select GENERIC_PCI_IOMAP
select GENERIC_SMP_IDLE_THREAD
+ select GENERIC_IDLE_LOOP
+ select GENERIC_IDLE_POLL_SETUP
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select HARDIRQS_SW_RESEND
diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
index 5a85f14..21a23e3 100644
--- a/arch/arm/include/asm/system_misc.h
+++ b/arch/arm/include/asm/system_misc.h
@@ -21,9 +21,6 @@ extern void (*arm_pm_idle)(void);

extern unsigned int user_debug;

-extern void disable_hlt(void);
-extern void enable_hlt(void);
-
#endif /* !__ASSEMBLY__ */

#endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 92884c8..c9a5e2c 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -57,34 +57,6 @@ static const char *isa_modes[] = {
"ARM" , "Thumb" , "Jazelle", "ThumbEE"
};

-static volatile int hlt_counter;
-
-void disable_hlt(void)
-{
- hlt_counter++;
-}
-
-void enable_hlt(void)
-{
- hlt_counter--;
- BUG_ON(hlt_counter < 0);
-}
-
-static int __init nohlt_setup(char *__unused)
-{
- hlt_counter = 1;
- return 1;
-}
-
-static int __init hlt_setup(char *__unused)
-{
- hlt_counter = 0;
- return 1;
-}
-
-__setup("nohlt", nohlt_setup);
-__setup("hlt", hlt_setup);
-
extern void call_with_stack(void (*fn)(void *), void *arg, void *sp);
typedef void (*phys_reset_t)(unsigned long);

@@ -168,54 +140,38 @@ static void default_idle(void)
local_irq_enable();
}

-/*
- * The idle thread.
- * We always respect 'hlt_counter' to prevent low power idle.
- */
-void cpu_idle(void)
+void arch_cpu_idle_prepare(void)
{
local_fiq_enable();
+}

- /* endless idle loop with no priority at all */
- while (1) {
- tick_nohz_idle_enter();
- rcu_idle_enter();
- ledtrig_cpu(CPU_LED_IDLE_START);
- while (!need_resched()) {
-#ifdef CONFIG_HOTPLUG_CPU
- if (cpu_is_offline(smp_processor_id()))
- cpu_die();
+void arch_cpu_idle_enter(void)
+{
+ ledtrig_cpu(CPU_LED_IDLE_START);
+#ifdef CONFIG_PL310_ERRATA_769419
+ wmb();
#endif
+}

- /*
- * We need to disable interrupts here
- * to ensure we don't miss a wakeup call.
- */
- local_irq_disable();
-#ifdef CONFIG_PL310_ERRATA_769419
- wmb();
+void arch_cpu_idle_exit(void)
+{
+ ledtrig_cpu(CPU_LED_IDLE_END);
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+void arch_cpu_idle_dead(void)
+{
+ cpu_die();
+}
#endif
- if (hlt_counter) {
- local_irq_enable();
- cpu_relax();
- } else if (!need_resched()) {
- stop_critical_timings();
- if (cpuidle_idle_call())
- default_idle();
- start_critical_timings();
- /*
- * default_idle functions must always
- * return with IRQs enabled.
- */
- WARN_ON(irqs_disabled());
- } else
- local_irq_enable();
- }
- ledtrig_cpu(CPU_LED_IDLE_END);
- rcu_idle_exit();
- tick_nohz_idle_exit();
- schedule_preempt_disabled();
- }
+
+/*
+ * Called from the core idle loop.
+ */
+void arch_cpu_idle(void)
+{
+ if (cpuidle_idle_call())
+ default_idle();
}

static char reboot_mode = 'h';
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 1f2cccc..4619177 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -336,7 +336,7 @@ asmlinkage void __cpuinit secondary_start_kernel(void)
/*
* OK, it's off to the idle thread for us
*/
- cpu_idle();
+ cpu_startup_entry(CPUHP_ONLINE);
}

void __init smp_cpus_done(unsigned int max_cpus)
diff --git a/arch/arm/mach-gemini/idle.c b/arch/arm/mach-gemini/idle.c
index 92bbd6b..87dff4f 100644
--- a/arch/arm/mach-gemini/idle.c
+++ b/arch/arm/mach-gemini/idle.c
@@ -13,9 +13,11 @@ static void gemini_idle(void)
* will never wakeup... Acctualy it is not very good to enable
* interrupts first since scheduler can miss a tick, but there is
* no other way around this. Platforms that needs it for power saving
- * should call enable_hlt() in init code, since by default it is
+ * should enable it in init code, since by default it is
* disabled.
*/
+
+ /* FIXME: Enabling interrupts here is racy! */
local_irq_enable();
cpu_do_idle();
}
diff --git a/arch/arm/mach-gemini/irq.c b/arch/arm/mach-gemini/irq.c
index 020852d..6d8f6d1 100644
--- a/arch/arm/mach-gemini/irq.c
+++ b/arch/arm/mach-gemini/irq.c
@@ -15,6 +15,8 @@
#include <linux/stddef.h>
#include <linux/list.h>
#include <linux/sched.h>
+#include <linux/cpu.h>
+
#include <asm/irq.h>
#include <asm/mach/irq.h>
#include <asm/system_misc.h>
@@ -77,7 +79,7 @@ void __init gemini_init_irq(void)
* Disable the idle handler by default since it is buggy
* For more info see arch/arm/mach-gemini/idle.c
*/
- disable_hlt();
+ cpu_idle_poll_ctrl(true);

request_resource(&iomem_resource, &irq_resource);

diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
index 1dbeb7c..6600cff 100644
--- a/arch/arm/mach-ixp4xx/common.c
+++ b/arch/arm/mach-ixp4xx/common.c
@@ -29,6 +29,7 @@
#include <linux/io.h>
#include <linux/export.h>
#include <linux/gpio.h>
+#include <linux/cpu.h>

#include <mach/udc.h>
#include <mach/hardware.h>
@@ -239,7 +240,7 @@ void __init ixp4xx_init_irq(void)
* ixp4xx does not implement the XScale PWRMODE register
* so it must not call cpu_do_idle().
*/
- disable_hlt();
+ cpu_idle_poll_ctrl(true);

/* Route all sources to IRQ instead of FIQ */
*IXP4XX_ICLR = 0x0;
diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
index 7a7690a..db37f49 100644
--- a/arch/arm/mach-omap1/pm.c
+++ b/arch/arm/mach-omap1/pm.c
@@ -43,6 +43,7 @@
#include <linux/module.h>
#include <linux/io.h>
#include <linux/atomic.h>
+#include <linux/cpu.h>

#include <asm/fncpy.h>
#include <asm/system_misc.h>
@@ -584,8 +585,7 @@ static void omap_pm_init_proc(void)
static int omap_pm_prepare(void)
{
/* We cannot sleep in idle until we have resumed */
- disable_hlt();
-
+ cpu_idle_poll_ctrl(true);
return 0;
}

@@ -621,7 +621,7 @@ static int omap_pm_enter(suspend_state_t state)

static void omap_pm_finish(void)
{
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
}


diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index a202a47..e512253 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -138,6 +138,7 @@
#include <linux/spinlock.h>
#include <linux/slab.h>
#include <linux/bootmem.h>
+#include <linux/cpu.h>

#include <asm/system_misc.h>

@@ -2157,7 +2158,7 @@ static int _enable(struct omap_hwmod *oh)
if (soc_ops.enable_module)
soc_ops.enable_module(oh);
if (oh->flags & HWMOD_BLOCK_WFI)
- disable_hlt();
+ cpu_idle_poll_ctrl(true);

if (soc_ops.update_context_lost)
soc_ops.update_context_lost(oh);
@@ -2221,7 +2222,7 @@ static int _idle(struct omap_hwmod *oh)
_del_initiator_dep(oh, mpu_oh);

if (oh->flags & HWMOD_BLOCK_WFI)
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
if (soc_ops.disable_module)
soc_ops.disable_module(oh);

@@ -2331,7 +2332,7 @@ static int _shutdown(struct omap_hwmod *oh)
_del_initiator_dep(oh, mpu_oh);
/* XXX what about the other system initiators here? dma, dsp */
if (oh->flags & HWMOD_BLOCK_WFI)
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
if (soc_ops.disable_module)
soc_ops.disable_module(oh);
_disable_clocks(oh);
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 673a4c1..dec5533 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -218,7 +218,7 @@ static int omap_pm_enter(suspend_state_t suspend_state)

static int omap_pm_begin(suspend_state_t state)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
if (cpu_is_omap34xx())
omap_prcm_irq_prepare();
return 0;
@@ -226,8 +226,7 @@ static int omap_pm_begin(suspend_state_t state)

static void omap_pm_end(void)
{
- enable_hlt();
- return;
+ cpu_idle_poll_ctrl(false);
}

static void omap_pm_finish(void)
diff --git a/arch/arm/mach-orion5x/board-dt.c b/arch/arm/mach-orion5x/board-dt.c
index 35a8014..94fbb81 100644
--- a/arch/arm/mach-orion5x/board-dt.c
+++ b/arch/arm/mach-orion5x/board-dt.c
@@ -14,6 +14,7 @@
#include <linux/init.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/cpu.h>
#include <asm/system_misc.h>
#include <asm/mach/arch.h>
#include <mach/orion5x.h>
@@ -52,7 +53,7 @@ static void __init orion5x_dt_init(void)
*/
if (dev == MV88F5281_DEV_ID && rev == MV88F5281_REV_D0) {
printk(KERN_INFO "Orion: Applying 5281 D0 WFI workaround.\n");
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
}

if (of_machine_is_compatible("lacie,ethernet-disk-mini-v2"))
diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
index d068f14..ad71c8a 100644
--- a/arch/arm/mach-orion5x/common.c
+++ b/arch/arm/mach-orion5x/common.c
@@ -293,7 +293,7 @@ void __init orion5x_init(void)
*/
if (dev == MV88F5281_DEV_ID && rev == MV88F5281_REV_D0) {
printk(KERN_INFO "Orion: Applying 5281 D0 WFI workaround.\n");
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
}

/*
diff --git a/arch/arm/mach-shark/core.c b/arch/arm/mach-shark/core.c
index b63dec8..1535557 100644
--- a/arch/arm/mach-shark/core.c
+++ b/arch/arm/mach-shark/core.c
@@ -10,6 +10,7 @@
#include <linux/sched.h>
#include <linux/serial_8250.h>
#include <linux/io.h>
+#include <linux/cpu.h>

#include <asm/setup.h>
#include <asm/mach-types.h>
@@ -130,7 +131,7 @@ static void __init shark_timer_init(void)

static void shark_init_early(void)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
}

MACHINE_START(SHARK, "Shark")
diff --git a/arch/arm/mach-shmobile/suspend.c b/arch/arm/mach-shmobile/suspend.c
index 47d83f7..5d92b5d 100644
--- a/arch/arm/mach-shmobile/suspend.c
+++ b/arch/arm/mach-shmobile/suspend.c
@@ -12,6 +12,8 @@
#include <linux/suspend.h>
#include <linux/module.h>
#include <linux/err.h>
+#include <linux/cpu.h>
+
#include <asm/io.h>
#include <asm/system_misc.h>

@@ -23,13 +25,13 @@ static int shmobile_suspend_default_enter(suspend_state_t suspend_state)

static int shmobile_suspend_begin(suspend_state_t state)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
return 0;
}

static void shmobile_suspend_end(void)
{
- enable_hlt();
+ cpu_idle_poll_ctrl(false);
}

struct platform_suspend_ops shmobile_suspend_ops = {
diff --git a/arch/arm/mach-w90x900/dev.c b/arch/arm/mach-w90x900/dev.c
index 7abdb96..e65a80a 100644
--- a/arch/arm/mach-w90x900/dev.c
+++ b/arch/arm/mach-w90x900/dev.c
@@ -19,6 +19,7 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
+#include <linux/cpu.h>

#include <linux/mtd/physmap.h>
#include <linux/mtd/mtd.h>
@@ -531,7 +532,7 @@ static struct platform_device *nuc900_public_dev[] __initdata = {

void __init nuc900_board_init(struct platform_device **device, int size)
{
- disable_hlt();
+ cpu_idle_poll_ctrl(true);
platform_add_devices(device, size);
platform_add_devices(nuc900_public_dev, ARRAY_SIZE(nuc900_public_dev));
spi_register_board_info(nuc900_spi_board_info,

2013-04-08 21:47:42

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
> On Mon, 25 Mar 2013, Russell King - ARM Linux wrote:
>
> > On Mon, Mar 25, 2013 at 12:31:20PM +0100, Thomas Gleixner wrote:
> > > On Fri, 22 Mar 2013, Kevin Hilman wrote:
> > >
> > > > Hi Thomas,
> > > >
> > > > Thomas Gleixner <[email protected]> writes:
> > > >
> > > > > Use the generic idle loop and replace enable/disable_hlt with the
> > > > > respective core functions.
> > > > >
> > > > > Signed-off-by: Thomas Gleixner <[email protected]>
> > > > > Cc: Russell King <[email protected]>
> > > >
> > > > I gave patches 1-5 + this a quick spin on ARM (specifially some OMAP3 and
> > > > OMAP4 platforms, with and without CPUidle enabled.)
> > > >
> > > > The OMAP stuff needed a couple minor compile fixes (below), but
> > > > otherwise it passes the quick "seems to work" test.
> > >
> > > Yeah, that's needed for the other arch/arm conversions as well.
> >
> > So, the only patch I got was this one, which is useless to test on its
> > own...
>
> I'm testing V2 now and will post an git URL with a testing branch
> later tonight.

So, how can I review these changes when all there is is a git URL, and
I *do* not want to pull them into my tree without first looking at the
patches, possibly reviewing them and *replying* with the patch inline?

I see you've now added this stuff to -tip. I guess ARM doesn't matter
to you at all. I guess having ARM people review the core changes doesn't
matter either.

Well, I guess if it's broken, we'll just have to fix it up after the
merge window.

One rule for x86, a different one for ARM. Let x86 maintainers moan
about ARM people, but don't let ARM people moan about x86 maintainers.
Yes, sure, right, I get it.

So, explicitly, my concern is this replacement of enable_hlt()/disable_hlt()
with this cpu_idle_poll_ctrl(), and whether this interface is still
counted - so that it's possible to call disable_hlt() to _permanently_
disable any hlt-based idle loop.

If that's not possible with your revised interface, you have just broken
some ARM platforms. I can only guess... because you never copied your
entire patch set to me nor to the ARM lists.

2013-04-09 09:20:45

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

Russell,

On Mon, 8 Apr 2013, Russell King - ARM Linux wrote:
> On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
> So, how can I review these changes when all there is is a git URL, and
> I *do* not want to pull them into my tree without first looking at the
> patches, possibly reviewing them and *replying* with the patch inline?

the patches were CC'ed to LKML and linux-arch and I expected that you
are at least having the latter. Find the relevant patch inlined below.

> I see you've now added this stuff to -tip. I guess ARM doesn't matter
> to you at all. I guess having ARM people review the core changes doesn't
> matter either.
>
> Well, I guess if it's broken, we'll just have to fix it up after the
> merge window.
>
> One rule for x86, a different one for ARM. Let x86 maintainers moan
> about ARM people, but don't let ARM people moan about x86 maintainers.
> Yes, sure, right, I get it.

Sigh no.

> So, explicitly, my concern is this replacement of enable_hlt()/disable_hlt()
> with this cpu_idle_poll_ctrl(), and whether this interface is still
> counted - so that it's possible to call disable_hlt() to _permanently_
> disable any hlt-based idle loop.

It is a counter. I looked carefully at all the various slightly
differently fcked up implementations and picked the counter based one
as it fits all requirements.

Thanks,

tglx
---
Subject: idle: Implement generic idle function
From: Thomas Gleixner <[email protected]>
Date: Thu, 21 Mar 2013 22:49:35 +0100

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(+)

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_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-09 09:38:57

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Tue, Apr 09, 2013 at 11:20:31AM +0200, Thomas Gleixner wrote:
> On Mon, 8 Apr 2013, Russell King - ARM Linux wrote:
> > On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
> > So, how can I review these changes when all there is is a git URL, and
> > I *do* not want to pull them into my tree without first looking at the
> > patches, possibly reviewing them and *replying* with the patch inline?
>
> the patches were CC'ed to LKML and linux-arch and I expected that you
> are at least having the latter. Find the relevant patch inlined below.

I've not been on linux-arch for a few years now, after it evolved into
yet another lkml-like list with high traffic rates, where mainly specific
x86 issues seemed to be discussed, rather than it being a way to contact
all arch maintainers.

> It is a counter. I looked carefully at all the various slightly
> differently fcked up implementations and picked the counter based one
> as it fits all requirements.

Great, thanks. The attached patch looks fine to me.

2013-04-25 20:03:45

by Stephen Boyd

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On 04/09/13 02:38, Russell King - ARM Linux wrote:
> On Tue, Apr 09, 2013 at 11:20:31AM +0200, Thomas Gleixner wrote:
>> On Mon, 8 Apr 2013, Russell King - ARM Linux wrote:
>>> On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
>>> So, how can I review these changes when all there is is a git URL, and
>>> I *do* not want to pull them into my tree without first looking at the
>>> patches, possibly reviewing them and *replying* with the patch inline?
>> the patches were CC'ed to LKML and linux-arch and I expected that you
>> are at least having the latter. Find the relevant patch inlined below.
> I've not been on linux-arch for a few years now, after it evolved into
> yet another lkml-like list with high traffic rates, where mainly specific
> x86 issues seemed to be discussed, rather than it being a way to contact
> all arch maintainers.
>
>> It is a counter. I looked carefully at all the various slightly
>> differently fcked up implementations and picked the counter based one
>> as it fits all requirements.
> Great, thanks. The attached patch looks fine to me.

I'm pretty sure that we need to apply this patch now that
rcu_idle_enter()/exit() is called lower down in the idle loop. Kevin,
did you test hotplug?

----8<-----

From: Stephen Boyd <[email protected]>
Subject: [PATCH] ARM: smp: Drop RCU_NONIDLE usage in cpu_die()

Before f7b861b (arm: Use generic idle loop, 2013-03-21) ARM would
kill the CPU within the rcu idle section. Now that the
rcu_idle_enter()/exit() pair have been pushed lower down in the
idle loop this is no longer true and so using RCU_NONIDLE here is
no longer necessary and also harmful because RCU is not actually
idle at this point.

Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 4619177..78f1eb5 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -233,7 +233,7 @@ void __ref cpu_die(void)
mb();

/* Tell __cpu_die() that this CPU is now safe to dispose of */
- RCU_NONIDLE(complete(&cpu_died));
+ complete(&cpu_died);

/*
* actual CPU shutdown procedure is at least platform (if not

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-04-25 21:01:11

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Thu, 25 Apr 2013, Stephen Boyd wrote:
> On 04/09/13 02:38, Russell King - ARM Linux wrote:
> > On Tue, Apr 09, 2013 at 11:20:31AM +0200, Thomas Gleixner wrote:
> >> On Mon, 8 Apr 2013, Russell King - ARM Linux wrote:
> >>> On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
> >>> So, how can I review these changes when all there is is a git URL, and
> >>> I *do* not want to pull them into my tree without first looking at the
> >>> patches, possibly reviewing them and *replying* with the patch inline?
> >> the patches were CC'ed to LKML and linux-arch and I expected that you
> >> are at least having the latter. Find the relevant patch inlined below.
> > I've not been on linux-arch for a few years now, after it evolved into
> > yet another lkml-like list with high traffic rates, where mainly specific
> > x86 issues seemed to be discussed, rather than it being a way to contact
> > all arch maintainers.
> >
> >> It is a counter. I looked carefully at all the various slightly
> >> differently fcked up implementations and picked the counter based one
> >> as it fits all requirements.
> > Great, thanks. The attached patch looks fine to me.
>
> I'm pretty sure that we need to apply this patch now that
> rcu_idle_enter()/exit() is called lower down in the idle loop. Kevin,
> did you test hotplug?

If the patch is agreed on, I guess I should take it via my idle
consolidation branch, right ?


> ----8<-----
>
> From: Stephen Boyd <[email protected]>
> Subject: [PATCH] ARM: smp: Drop RCU_NONIDLE usage in cpu_die()
>
> Before f7b861b (arm: Use generic idle loop, 2013-03-21) ARM would
> kill the CPU within the rcu idle section. Now that the
> rcu_idle_enter()/exit() pair have been pushed lower down in the
> idle loop this is no longer true and so using RCU_NONIDLE here is
> no longer necessary and also harmful because RCU is not actually
> idle at this point.
>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/arm/kernel/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 4619177..78f1eb5 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -233,7 +233,7 @@ void __ref cpu_die(void)
> mb();
>
> /* Tell __cpu_die() that this CPU is now safe to dispose of */
> - RCU_NONIDLE(complete(&cpu_died));
> + complete(&cpu_died);
>
> /*
> * actual CPU shutdown procedure is at least platform (if not
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>
>

2013-05-01 00:49:25

by Stephen Boyd

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On 04/25/13 14:01, Thomas Gleixner wrote:
> On Thu, 25 Apr 2013, Stephen Boyd wrote:
>> On 04/09/13 02:38, Russell King - ARM Linux wrote:
>>> On Tue, Apr 09, 2013 at 11:20:31AM +0200, Thomas Gleixner wrote:
>>>> On Mon, 8 Apr 2013, Russell King - ARM Linux wrote:
>>>>> On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
>>>>> So, how can I review these changes when all there is is a git URL, and
>>>>> I *do* not want to pull them into my tree without first looking at the
>>>>> patches, possibly reviewing them and *replying* with the patch inline?
>>>> the patches were CC'ed to LKML and linux-arch and I expected that you
>>>> are at least having the latter. Find the relevant patch inlined below.
>>> I've not been on linux-arch for a few years now, after it evolved into
>>> yet another lkml-like list with high traffic rates, where mainly specific
>>> x86 issues seemed to be discussed, rather than it being a way to contact
>>> all arch maintainers.
>>>
>>>> It is a counter. I looked carefully at all the various slightly
>>>> differently fcked up implementations and picked the counter based one
>>>> as it fits all requirements.
>>> Great, thanks. The attached patch looks fine to me.
>> I'm pretty sure that we need to apply this patch now that
>> rcu_idle_enter()/exit() is called lower down in the idle loop. Kevin,
>> did you test hotplug?
> If the patch is agreed on, I guess I should take it via my idle
> consolidation branch, right ?

Yes I think so. Hopefully Russell King or Paul McKenney can ack this patch.

>
>> ----8<-----
>>
>> From: Stephen Boyd <[email protected]>
>> Subject: [PATCH] ARM: smp: Drop RCU_NONIDLE usage in cpu_die()
>>
>> Before f7b861b (arm: Use generic idle loop, 2013-03-21) ARM would
>> kill the CPU within the rcu idle section. Now that the
>> rcu_idle_enter()/exit() pair have been pushed lower down in the
>> idle loop this is no longer true and so using RCU_NONIDLE here is
>> no longer necessary and also harmful because RCU is not actually
>> idle at this point.
>>
>> Signed-off-by: Stephen Boyd <[email protected]>
>> ---
>> arch/arm/kernel/smp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>> index 4619177..78f1eb5 100644
>> --- a/arch/arm/kernel/smp.c
>> +++ b/arch/arm/kernel/smp.c
>> @@ -233,7 +233,7 @@ void __ref cpu_die(void)
>> mb();
>>
>> /* Tell __cpu_die() that this CPU is now safe to dispose of */
>> - RCU_NONIDLE(complete(&cpu_died));
>> + complete(&cpu_died);
>>
>> /*
>> * actual CPU shutdown procedure is at least platform (if not
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> hosted by The Linux Foundation
>>
>>


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

2013-05-01 00:55:35

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [patch 08/34] arm: Use generic idle loop

On Tue, Apr 30, 2013 at 05:49:06PM -0700, Stephen Boyd wrote:
> On 04/25/13 14:01, Thomas Gleixner wrote:
> > On Thu, 25 Apr 2013, Stephen Boyd wrote:
> >> On 04/09/13 02:38, Russell King - ARM Linux wrote:
> >>> On Tue, Apr 09, 2013 at 11:20:31AM +0200, Thomas Gleixner wrote:
> >>>> On Mon, 8 Apr 2013, Russell King - ARM Linux wrote:
> >>>>> On Mon, Mar 25, 2013 at 03:02:39PM +0100, Thomas Gleixner wrote:
> >>>>> So, how can I review these changes when all there is is a git URL, and
> >>>>> I *do* not want to pull them into my tree without first looking at the
> >>>>> patches, possibly reviewing them and *replying* with the patch inline?
> >>>> the patches were CC'ed to LKML and linux-arch and I expected that you
> >>>> are at least having the latter. Find the relevant patch inlined below.
> >>> I've not been on linux-arch for a few years now, after it evolved into
> >>> yet another lkml-like list with high traffic rates, where mainly specific
> >>> x86 issues seemed to be discussed, rather than it being a way to contact
> >>> all arch maintainers.
> >>>
> >>>> It is a counter. I looked carefully at all the various slightly
> >>>> differently fcked up implementations and picked the counter based one
> >>>> as it fits all requirements.
> >>> Great, thanks. The attached patch looks fine to me.
> >> I'm pretty sure that we need to apply this patch now that
> >> rcu_idle_enter()/exit() is called lower down in the idle loop. Kevin,
> >> did you test hotplug?
> > If the patch is agreed on, I guess I should take it via my idle
> > consolidation branch, right ?
>
> Yes I think so. Hopefully Russell King or Paul McKenney can ack this patch.
>
> >
> >> ----8<-----
> >>
> >> From: Stephen Boyd <[email protected]>
> >> Subject: [PATCH] ARM: smp: Drop RCU_NONIDLE usage in cpu_die()
> >>
> >> Before f7b861b (arm: Use generic idle loop, 2013-03-21) ARM would
> >> kill the CPU within the rcu idle section. Now that the
> >> rcu_idle_enter()/exit() pair have been pushed lower down in the
> >> idle loop this is no longer true and so using RCU_NONIDLE here is
> >> no longer necessary and also harmful because RCU is not actually
> >> idle at this point.
> >>
> >> Signed-off-by: Stephen Boyd <[email protected]>

>From an RCU perspective:

Acked-by: Paul E. McKenney <[email protected]>

> >> ---
> >> arch/arm/kernel/smp.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> >> index 4619177..78f1eb5 100644
> >> --- a/arch/arm/kernel/smp.c
> >> +++ b/arch/arm/kernel/smp.c
> >> @@ -233,7 +233,7 @@ void __ref cpu_die(void)
> >> mb();
> >>
> >> /* Tell __cpu_die() that this CPU is now safe to dispose of */
> >> - RCU_NONIDLE(complete(&cpu_died));
> >> + complete(&cpu_died);
> >>
> >> /*
> >> * actual CPU shutdown procedure is at least platform (if not
> >>
> >> --
> >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> >> hosted by The Linux Foundation
> >>
> >>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
>

2013-05-21 00:57:18

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH] ARM: smp: Drop RCU_NONIDLE usage in cpu_die()

Before f7b861b (arm: Use generic idle loop, 2013-03-21) ARM would
kill the CPU within the rcu idle section. Now that the
rcu_idle_enter()/exit() pair have been pushed lower down in the
idle loop this is no longer true and so using RCU_NONIDLE here is
no longer necessary and also harmful because RCU is not actually
idle at this point.

Cc: Russell King <[email protected]>
Acked-by: Paul E. McKenney <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 4619177..78f1eb5 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -233,7 +233,7 @@ void __ref cpu_die(void)
mb();

/* Tell __cpu_die() that this CPU is now safe to dispose of */
- RCU_NONIDLE(complete(&cpu_died));
+ complete(&cpu_died);

/*
* actual CPU shutdown procedure is at least platform (if not
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation