[patch] ARM: smpboot: Enable interrupts after marking CPU online/active

[Kukjin Kim]

Thomas Gleixner wrote:
> 
> On Fri, 7 Oct 2011, Kukjin Kim wrote:
> > I think, basically, if SPIs in GIC are used in local timer, the routing
> > interrupt like calling irq_set_affinity() to offline CPUs should be
> > available when boot time before calling set_cpu_online() because as you
know
> > the local_timer_setup() which includes setup interrupt is called in
> > percpu_timer_setup() now...
> >
> > Is there any way to get the flags of struct irqaction from struct
irq_data
> > in gic_set_affinity()? Or?
> 
> No, and you should not even think about it at all.
> 
OK, I see. Thanks :)

> The problem I can see from all this discussion is related to the early
> enabling of interrupts and the per cpu timer setup before the cpu is
> marked online. I really do not understand at all why this needs to be
> done in order to call calibrate_delay().
> 
> calibrate_delay() monitors jiffies, which are updated from the CPU
> which is waiting for the new CPU to set the online bit.
> 
> So you simply can call calibrate_delay() on the new CPU just from the
> interrupt disabled region and move the local timer setup after you
> stored the cpu data and before enabling interrupts.
> 
> This solves both the cpu_online vs. cpu_active problem and the
> affinity setting of the per cpu timers.
> 
Looks good to me......and of course, it works fine on SMDK4210 (EXYNOS4210).

Russell, how about this? If you're ok how this can be handled before v3.1?
As you know, I need to revert regarding one commit and this......or just
fixing it with this?

Anyway, I'd like to know your suggestion about this.

> Thanks,
> 
> 	tglx
> ----
> 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
> @@ -301,17 +301,7 @@ asmlinkage void __cpuinit secondary_star
>  	 */
>  	platform_secondary_init(cpu);
> 
> -	/*
> -	 * Enable local interrupts.
> -	 */
>  	notify_cpu_starting(cpu);
> -	local_irq_enable();
> -	local_fiq_enable();
> -
> -	/*
> -	 * Setup the percpu timer for this CPU.
> -	 */
> -	percpu_timer_setup();
> 
>  	calibrate_delay();
> 
> @@ -323,10 +313,23 @@ asmlinkage void __cpuinit secondary_star
>  	 * before we continue.
>  	 */
>  	set_cpu_online(cpu, true);
> +
> +	/*
> +	 * Setup the percpu timer for this CPU.
> +	 */
> +	percpu_timer_setup();
> +
>  	while (!cpu_active(cpu))
>  		cpu_relax();
> 
>  	/*
> +	 * cpu_active bit is set, so it's safe to enable interrupts
> +	 * now.
> +	 */
> +	local_irq_enable();
> +	local_fiq_enable();
> +
> +	/*
>  	 * OK, it's off to the idle thread for us
>  	 */
>  	cpu_idle();


Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim at samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.