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

[Thomas Gleixner <tglx@linutronix.de>]

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