Re: [patch 4/6] Fix SMP poweroff hangs

[Mark Lord <lkml@rtr.ca>]

(reposting this.. somehow left LKML off the TO/CC list)

Mark Lord wrote:
> Eric W. Biederman wrote:
>> Mark Lord <lkml@rtr.ca> writes:
..
>> The code path on i386 should be:
>> machine_power_off
>>   native_machine_power_off
>>      machine_shutdown(); (which disables the other cpus)
>>     smp_call_function
>>        stop_this_cpu (on each cpu to be stopped.
>>      pm_power_off(); (which turns off the power)
> ..
>> This does sound like a race of some sort.
>
> Mmm... thanks for the tour.
>
> The cpu hotplug code appears to take great precautions against internal races
> (dunno if it succeeds or not, though), but the correspond code in native_smp_send_stop()
> looks a bit iffy by comparison.  I wonder if that's where it gets stuck?
>
> static void native_smp_send_stop(void)
> {
>        /* Don't deadlock on the call lock in panic */
>        int nolock = !spin_trylock(&call_lock);
>        unsigned long flags;
>
>        local_irq_save(flags);
>        __smp_call_function(stop_this_cpu, NULL, 0, 0);
>        if (!nolock)
>                spin_unlock(&call_lock);
>        disable_local_APIC();
>        local_irq_restore(flags);
> }
>
> So basically, it tries to avoid races by grabbing the call_lock,
> but then ignores that lock and proceeds anyway.  Recipe for a race?

Hmmmm.. and that code also fails to wait for the other CPU(s)
to actually halt (not sure how it could wait, but it doesn't even try),
so the other CPU(s) could easily still be active when we then stumble
into our ACPI call to turn the power off.

I suppose that might possibly hurt something.

Cheers
-- 
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com