Re: [PATCH v6 2/5] xen/sched: Link CPU topology to scheduler

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.07.2026 12:44, Hirokazu Takahashi wrote:
> @@ -35,7 +36,9 @@
>   */
>  static unsigned int cpu_nr_siblings(unsigned int cpu)
>  {
> -#ifdef CONFIG_X86
> +#if defined(CONFIG_GENERIC_CPU_TOPOLOGY)
> +    return cpu_topology ? cpu_topology[cpu].num_siblings : 1;
> +#elif defined(CONFIG_X86)
>      return cpu_data[cpu].x86_num_siblings;
>  #else
>      return 1;

Now that this is ordered more sensibly, an issue is becoming apparent: If and
when x86 also supports GENERIC_CPU_TOPOLOGY, the present x86 logic should also
be engaged when !cpu_topology. By re-arranging accordingly, you also avoid the
need to duplicate the literal 1.

> @@ -11,15 +22,48 @@ struct cpu_topology {
>      cpumask_var_t thread_sibling;
>      cpumask_var_t core_sibling;
>      cpumask_var_t cluster_sibling;
> +    unsigned int phys_core_id;
> +    unsigned int phys_cluster_id;
> +    unsigned int phys_socket_id;
> +    unsigned int num_siblings;
>  };
>  
>  extern struct cpu_topology *cpu_topology;
>  void init_cpu_topology(void);
>  
> +static inline void init_cpu_sibling_map(unsigned int cpu)
> +{
> +    if ( cpu_topology )
> +    {
> +        cpumask_copy(per_cpu(cpu_sibling_mask, cpu),
> +                     cpu_topology[cpu].thread_sibling);
> +        cpumask_copy(per_cpu(cpu_core_mask, cpu),
> +                     cpu_topology[cpu].core_sibling);
> +    }
> +    else
> +        init_cpu_sibling_map_default(cpu);
> +}

Personally I consider this already being too redundant with ...

> +#define cpu_to_core(cpu) (cpu_topology ? cpu_topology[cpu].phys_core_id : 0)
> +#define cpu_to_socket(cpu) (cpu_topology ? cpu_topology[cpu].phys_socket_id 
> : 0)
> +
>  #else /* CONFIG_GENERIC_CPU_TOPOLOGY */
>  
>  static inline void init_cpu_topology(void) {}
>  
> +static inline void init_cpu_sibling_map(unsigned int cpu)
> +{
> +    init_cpu_sibling_map_default(cpu);
> +}

... this. Imo it would better be

static inline void init_cpu_sibling_map(unsigned int cpu)
{
    if ( IS_ENABLED(CONFIG_GENERIC_CPU_TOPOLOGY) && cpu_topology )
    {
        cpumask_copy(per_cpu(cpu_sibling_mask, cpu),
                     cpu_topology[cpu].thread_sibling);
        cpumask_copy(per_cpu(cpu_core_mask, cpu),
                     cpu_topology[cpu].core_sibling);
    }
    else
        init_cpu_sibling_map_default(cpu);
}

Which then of course requires the decl of cpu_topology (not its definition) to
always be visible.

Jan