Re: [Xen-devel] [PATCH 2/8] x86: distinguish CPU offlining from CPU removal

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.07.18 at 12:53, <wei.liu2@xxxxxxxxxx> wrote:
> On Wed, Jul 11, 2018 at 06:06:04AM -0600, Jan Beulich wrote:
> [...]
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -63,6 +63,8 @@ static cpumask_t scratch_cpu0mask;
>>  cpumask_t cpu_online_map __read_mostly;
>>  EXPORT_SYMBOL(cpu_online_map);
>>  
>> +bool __read_mostly park_offline_cpus;
>> +
>>  unsigned int __read_mostly nr_sockets;
>>  cpumask_t **__read_mostly socket_cpumask;
>>  static cpumask_t *secondary_socket_cpumask;
>> @@ -887,7 +889,7 @@ static void cleanup_cpu_root_pgt(unsigne
>>      }
>>  }
>>  
>> -static void cpu_smpboot_free(unsigned int cpu)
>> +static void cpu_smpboot_free(unsigned int cpu, bool all)
> 
> I think "all" is too vague. It doesn't convey the idea what constitutes
> "partial". But I don't have any better suggestion either.

Indeed I've been trying to come up with a better name before
posting, but couldn't. One thing though - I don't think the name
should in anyway be required to express what "partial" means.

>> @@ -898,15 +900,24 @@ static void cpu_smpboot_free(unsigned in
>>          socket_cpumask[socket] = NULL;
>>      }
>>  
>> -    c[cpu].phys_proc_id = XEN_INVALID_SOCKET_ID;
>> -    c[cpu].cpu_core_id = XEN_INVALID_CORE_ID;
>> -    c[cpu].compute_unit_id = INVALID_CUID;
>>      cpumask_clear_cpu(cpu, &cpu_sibling_setup_map);
>>  
>> -    free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
>> -    free_cpumask_var(per_cpu(cpu_core_mask, cpu));
>> -    if ( per_cpu(scratch_cpumask, cpu) != &scratch_cpu0mask )
>> -        free_cpumask_var(per_cpu(scratch_cpumask, cpu));
>> +    if ( all )
>> +    {
>> +        c[cpu].phys_proc_id = XEN_INVALID_SOCKET_ID;
>> +        c[cpu].cpu_core_id = XEN_INVALID_CORE_ID;
>> +        c[cpu].compute_unit_id = INVALID_CUID;
>> +
>> +        free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
>> +        clear_cpumask_var(&per_cpu(cpu_sibling_mask, cpu));
>> +        free_cpumask_var(per_cpu(cpu_core_mask, cpu));
>> +        clear_cpumask_var(&per_cpu(cpu_core_mask, cpu));
>> +        if ( per_cpu(scratch_cpumask, cpu) != &scratch_cpu0mask )
>> +        {
>> +            free_cpumask_var(per_cpu(scratch_cpumask, cpu));
>> +            clear_cpumask_var(&per_cpu(scratch_cpumask, cpu));
>> +        }
> 
> One thing that's not mentioned in the commit message is why various
> masks are kept. I guess they are needed for some other parts of the
> system to remain operational.

Hmm, I've taken the opposite approach: Free in "partial" mode
only what I could prove won't be needed.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel