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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

>  {
>      unsigned int order, socket = cpu_to_socket(cpu);
>      struct cpuinfo_x86 *c = cpu_data;
> @@ -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.

The rest looks fine to me.

Wei.

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