Re: [PATCH RFC 1/3] arch_topology: Add a die level to CPU topology

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arch_topology: missing generic die topology API macros
--

commit 6649d6f4ff943b18e39749fa5d7c01258bf6f28c
Author: Troy Mitchell <troy.mitchell@linux.dev>

arch_topology: Add a die level to CPU topology

This commit introduces a die level to the generic CPU topology model by adding
die identifiers and sibling masks between the package and cluster levels.
Die sibling masks are built when firmware supplies a valid identifier,
otherwise falling back to an unknown die.

> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 8c5e47c28d9a3..ee82827be3351 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c

[ ... ]

> @@ -824,6 +836,8 @@ void remove_cpu_topology(unsigned int cpu)
>  		cpumask_clear_cpu(cpu, topology_sibling_cpumask(sibling));
>  	for_each_cpu(sibling, topology_cluster_cpumask(cpu))
>  		cpumask_clear_cpu(cpu, topology_cluster_cpumask(sibling));
> +	for_each_cpu(sibling, &cpu_topology[cpu].die_sibling)
> +		cpumask_clear_cpu(cpu, &cpu_topology[sibling].die_sibling);

[Severity: Medium]
Why does this access the raw cpu_topology struct directly?

The surrounding code uses macros like topology_cluster_cpumask() to retrieve
the sibling masks. Since the patch adds die_sibling, should there be a
matching topology_die_cpumask() macro exported for consistent access?

>  	for_each_cpu(sibling, topology_llc_cpumask(cpu))
>  		cpumask_clear_cpu(cpu, topology_llc_cpumask(sibling));

[ ... ]

> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index ebd7f8935f969..2a5591bb95075 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -63,10 +63,12 @@ struct cpu_topology {
>  	int thread_id;
>  	int core_id;
>  	int cluster_id;
> +	int die_id;
>  	int package_id;
>  	cpumask_t thread_sibling;
>  	cpumask_t core_sibling;
>  	cpumask_t cluster_sibling;
> +	cpumask_t die_sibling;
>  	cpumask_t llc_sibling;
>  };

[Severity: Medium]
Are we missing the corresponding generic macro definitions for the new die
topology API?

While die_id and die_sibling are added to the structure here and computed
internally, the standard generic topology accessors (like topology_die_cpumask
and topology_die_id) are omitted from this generic header.

Without these definitions in the generic header, do architectures utilizing
generic topology (such as ARM64) silently fall back to the dummy defaults
in <linux/topology.h>?

If these macros are only defined later in an architecture-specific header
(such as RISC-V in a follow-up patch), it leaves the generic API abstraction
broken. The newly computed die boundaries would remain functionally
inaccessible and completely hidden from sysfs and the scheduler on other
architectures like ARM64.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920-riscv-die-topology-rfc-v1-0-071c0bf61d5f@linux.dev?part=1