Re: [patch 20/34] metag: Use generic idle loop

James Hogan
2013-03-22T10:16:47Z

On 21/03/13 21:53, Thomas Gleixner wrote:
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: James Hogan <james.hogan@imgtec.com>
> ---
> arch/metag/Kconfig | 1 +
> arch/metag/kernel/process.c | 32 +++-----------------------------
> arch/metag/kernel/smp.c | 2 +-
> 3 files changed, 5 insertions(+), 30 deletions(-)
>
> Index: linux-2.6/arch/metag/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/metag/Kconfig
> +++ linux-2.6/arch/metag/Kconfig
> @@ -9,6 +9,7 @@ config METAG
> select GENERIC_CLOCKEVENTS
> select GENERIC_IRQ_SHOW
> select GENERIC_SMP_IDLE_THREAD
> + select GENERIC_IDLE_LOOP

Please preserve alphabetical ordering

> select HAVE_64BIT_ALIGNED_ACCESS
> select HAVE_ARCH_TRACEHOOK
> select HAVE_C_RECORDMCOUNT
> Index: linux-2.6/arch/metag/kernel/process.c
> ===================================================================
> --- linux-2.6.orig/arch/metag/kernel/process.c
> +++ linux-2.6/arch/metag/kernel/process.c
> @@ -31,7 +31,7 @@
> /*
> * Wait for the next interrupt and enable local interrupts
> */
> -static inline void arch_idle(void)
> +void arch_cpu_idle(void)
> {
> int tmp;
>
> @@ -59,35 +59,9 @@ static inline void arch_idle(void)
> : "r" (get_trigger_mask()));
> }
>
> -void cpu_idle(void)
> +void arch_cpu_idle_dead(void)
> {
> - set_thread_flag(TIF_POLLING_NRFLAG);
> -
> - while (1) {
> - tick_nohz_idle_enter();
> - rcu_idle_enter();
> -
> - while (!need_resched()) {
> - /*
> - * We need to disable interrupts here to ensure we don't
> - * miss a wakeup call.
> - */
> - local_irq_disable();
> - if (!need_resched()) {
> -#ifdef CONFIG_HOTPLUG_CPU
> - if (cpu_is_offline(smp_processor_id()))
> - cpu_die();
> -#endif
> - arch_idle();
> - } else {
> - local_irq_enable();
> - }
> - }
> -
> - rcu_idle_exit();
> - tick_nohz_idle_exit();
> - schedule_preempt_disabled();
> - }
> + cpu_die();

This requires #include <linux/smp.h> and even then the definition of
cpu_die() is guarded by #ifdef CONFIG_HOTPLUG_CPU, like blackfin & s390.

> }
>
> void (*pm_power_off)(void);
> Index: linux-2.6/arch/metag/kernel/smp.c
> ===================================================================
> --- linux-2.6.orig/arch/metag/kernel/smp.c
> +++ linux-2.6/arch/metag/kernel/smp.c
> @@ -297,7 +297,7 @@ asmlinkage void secondary_start_kernel(v
> /*
> * 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)
>
>

I also hit the same problems Tony Breeds hit.

After hacking around them, and enabling SMP and HOTPLUG_CPU, I hit the
WARN_ON_ONCE(!irqs_disabled()) after arch_cpu_idle() in cpu_idle_loop():

> META213-Thread1 DSP [LogF] WARNING: at kernel/cpu/idle.c:94 _cpu_startup_entry+0x1cc/0x1e0()

arch_cpu_idle() enables interrupts. See Arnd's comment in
https://patchwork.kernel.org/patch/2002571/:

> To avoid that, you need to disable interrupts around
> the inner loop. The sleep instruction should return with
> interrupts implicitly enabled if ARC behaves like most
> other architectures doing this.

Cheers
James