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

[Amit Kachhap]

On 7 October 2011 17:47, Thomas Gleixner <tglx at linutronix.de> 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.
>
> 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.
>
> 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();

I tested these modifications on samsung exynos4 platform and they work
fine with almost same bogomips. I also enabled cpu topology patches
and they also don't create any race conditions. Looks like a good
approach.

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