Re: [PATCH] Fix cpu offline bug

[Keir Fraser <keir.xen@gmail.com>]

Urk, good point!

 Thanks,
 Keir


On 05/03/2011 15:10, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:

> Keir,
> 
> Thanks for comments and update!
> However, I think we still need update it a little :)
> 
> =============================
> Fix cpu offline bug
> 
> Remove BUG_ON since it's of some risk, considering the small time window:
> cpu0 read (cpu_state) --> offlining cpu write (cpu_state) --> cpu0 read
> (cpu_state)
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> 
> diff -r 1ecb840bea17 xen/arch/x86/smpboot.c
> --- a/xen/arch/x86/smpboot.c    Sat Mar 05 23:30:29 2022 +0800
> +++ b/xen/arch/x86/smpboot.c    Sun Mar 06 01:21:34 2022 +0800
> @@ -864,7 +864,6 @@ void __cpu_die(unsigned int cpu)
> 
>      while ( cpu_state != CPU_STATE_DEAD )
>      {
> -        BUG_ON(cpu_state != CPU_STATE_DYING);
>          mdelay(100);
>          cpu_relax();
>          process_pending_softirqs();
> =============================
> 
> Thanks,
> Jinsong
> 
> 
> 
> Keir Fraser wrote:
>> On 04/03/2011 10:22, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:
>> 
>>> Fix cpu offline bug
>>> 
>>> At current xen, when cpu offline, cpu0 will wait the 1st cpu offline;
>>> However, if offline 2nd, 3rd, ... cpu, cpu0 will not wait it.
>>> This patch is used to fix the bug.
>> 
>> Alternative fix applied tio unstable and 4.1-testing. The fix below
>> doesn't account for the fact that cpu_state can also ==
>> CPU_STATE_DEAD after an (unsuccessful) CPU online operation.
>> 
>>  -- Keir
>> 
>>> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
>>> 
>>> diff -r d1631540bcc4 xen/arch/x86/smpboot.c
>>> --- a/xen/arch/x86/smpboot.c Tue Jan 18 17:23:24 2011 +0000
>>> +++ b/xen/arch/x86/smpboot.c Sat Feb 12 03:48:09 2011 +0800
>>> @@ -78,7 +78,8 @@ static enum cpu_state {
>>>      CPU_STATE_INIT,     /* master -> slave: Early bringup phase 1 */
>>>      CPU_STATE_CALLOUT,  /* master -> slave: Early bringup phase 2 */
>>>      CPU_STATE_CALLIN,   /* slave -> master: Completed phase 2 */
>>> -    CPU_STATE_ONLINE    /* master -> slave: Go fully online now. */
>>> +    CPU_STATE_ONLINE,   /* master -> slave: Go fully online now. */
>>> +    CPU_STATE_STAY      /* after slave dead, global cpu_state stay
>>>  here */  } cpu_state; #define set_cpu_state(state) do { mb();
>>> cpu_state = (state); } while (0)
>>> 
>>> @@ -867,6 +868,8 @@ void __cpu_die(unsigned int cpu)
>>>          if ( (++i % 10) == 0 )
>>>              printk(KERN_ERR "CPU %u still not dead...\n", cpu);
>>> } +
>>> +    set_cpu_state(CPU_STATE_STAY);
>>>  }
>>> 
>>>  int cpu_add(uint32_t apic_id, uint32_t acpi_id, uint32_t pxm)
>