lkml.org 
[lkml]   [2014]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/3] x86: fix hang when AP bringup is too slow


On 03/18/2014 02:49 PM, Igor Mammedov wrote:
> On Tue, 18 Mar 2014 08:21:19 -0400
> Prarit Bhargava <prarit@redhat.com> wrote:
>
>>
>>
>> On 03/13/2014 10:25 AM, Igor Mammedov wrote:
>>> Hang is observed on virtual machines during CPU hotplug,
>>> especially in big guests with many CPUs. (It happens more
>>> often if host is over-committed).
>>>
>>
>> Hey Igor, I like this better than the previous version. Thanks for taking into
>> account the possible races in this code.
>>
>> A quick question on system behaviour. As you know I've been more concerned
>> lately with error handling, etc., through the cpu hotplug code as we've seen
>> several customer reports of silent failures or cascading failures in the cpu
>> hotplug code when users have been attempting to perform physical hotplug.
>>
>> After your patches have been applied, in theory the following can happen:
>>
>> The master CPU is completing the AP cpu's bring up. The AP cpu is doing (sorry
>> for the cut-and-paste),
>>
>> void cpu_init(void)
>> {
>> int cpu = smp_processor_id();
>> struct task_struct *curr = current;
>> struct tss_struct *t = &per_cpu(init_tss, cpu);
>> struct thread_struct *thread = &curr->thread;
>>
>> /*
>> * wait till the master CPU completes it's STARTUP sequence,
>> * and decides to wait till this AP boots
>> */
>> while (!cpumask_test_cpu(cpu, cpu_callout_mask)) {
>> cpu_relax();
>> if (per_cpu(x86_cpu_to_apicid, cpu) == BAD_APICID)
>> halt();
>> }
>>
>> and is spinning on cpu_relax(). Suppose something goes wrong and the softlockup
>> watchdog fires on the AP cpu:
>>
>> 1. Can it? :) ie) will the softlockup fire at this point of the AP init? Okay,
>> I'm being really lazy and not looking at the code ;)
> It shouldn't, CPU is in pristine state and just came from boot trampoline at
> this point without interrupts configured yet.

Okay, not a big problem.

>
>>
>> 2. Is there anything we can do in this code to notify the user of a problem?
>> Even a pr_crit() here I think would help to indicate what went wrong; it might
>> be useful for future debugging in this area to have some sort of output. I
>> think a WARN() or BUG() is necessary here as there are several calls to cpu_init().
> Do you mean something like this:
>
> + if (per_cpu(x86_cpu_to_apicid, cpu) == BAD_APICID) {
> + WARN(1);
> + halt();
> + }

Yeah, maybe WARN_ON(1, "some comment") though.

>
>>
>> 3. Change this comment:
>>
>> * wait till the master CPU completes it's STARTUP sequence,
>> * and decides to wait till this AP boots
>>
>> to
>>
>> /* wait for the master CPU to complete this cpu's STARTUP. */ ?
> well, that is not quite the same as above, comment should underline that
> AP waits for ACK from master CPU before continuing with this AP initialization.
>
> How about:
> /* wait for ACK from master CPU before continuing with AP initialization */

Awesome :)

P.

>
>>
>> Apologies for the late review,
>>
>> P.
>
>


\
 
 \ /
  Last update: 2014-03-19 14:01    [W:0.083 / U:3.316 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site