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

Jan Beulich <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.