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

Thomas Gleixner
2013-03-25T11:26:56Z

On Fri, 22 Mar 2013, James Hogan wrote:

> 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

Ok.

> > 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.

Will fix.

> > }
> >
> > 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():

My stupidity. I fatfingered it when last minute changing it to
WARN_ON_ONCE

> > 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.

Right. My fault. See above.

Thanks,

tglx