|
| From: | wangyanan (Y) |
| Subject: | Re: [RFC 08/52] machine: Add helpers to get cpu topology info from MachineState.topo |
| Date: | Fri, 17 Feb 2023 15:41:30 +0800 |
| User-agent: | Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 |
在 2023/2/17 11:07, Zhao Liu 写道:
On Thu, Feb 16, 2023 at 04:38:38PM +0800, wangyanan (Y) wrote:Date: Thu, 16 Feb 2023 16:38:38 +0800 From: "wangyanan (Y)" <wangyanan55@huawei.com> Subject: Re: [RFC 08/52] machine: Add helpers to get cpu topology info from MachineState.topo Hi Zhao, 在 2023/2/13 17:49, Zhao Liu 写道:From: Zhao Liu <zhao1.liu@intel.com> When MachineState.topo is introduced, the topology related structures become complicated. In the general case (hybrid or smp topology), accessing the topology information needs to determine whether it is currently smp or hybrid topology, and then access the corresponding MachineState.topo.smp or MachineState.topo.hybrid. The best way to do this is to wrap the access to the topology to avoid having to check each time it is accessed. The following helpers are provided here: - General interfaces - no need to worry about whether the underlying topology is smp or hybrid: * machine_topo_get_cpus() * machine_topo_get_max_cpus() * machine_topo_is_smp() * machine_topo_get_sockets() * machine_topo_get_dies() * machine_topo_get_clusters() * machine_topo_get_threads(); * machine_topo_get_cores(); * machine_topo_get_threads_by_idx() * machine_topo_get_cores_by_idx() * machine_topo_get_cores_per_socket() * machine_topo_get_threads_per_socket() - SMP-specific interfaces - provided for the cases that are clearly known to be smp topology: * machine_topo_get_smp_cores() * machine_topo_get_smp_threads() Since for hybrid topology, each core may has different threads, if someone wants "cpus per core", the cpu_index is need to target a specific core (machine_topo_get_threads_by_idx()). But for smp, there is no need to be so troublesome, so for this case, we provide smp-specific interfaces. Signed-off-by: Zhao Liu <zhao1.liu@intel.com> --- hw/core/machine-topo.c | 142 +++++++++++++++++++++++++++++++++++++++++ include/hw/boards.h | 35 ++++++++++ 2 files changed, 177 insertions(+) diff --git a/hw/core/machine-topo.c b/hw/core/machine-topo.c index 7223f73f99b0..b20160479629 100644 --- a/hw/core/machine-topo.c +++ b/hw/core/machine-topo.c @@ -21,6 +21,148 @@ #include "hw/boards.h" #include "qapi/error.h" +unsigned int machine_topo_get_sockets(const MachineState *ms) +{ + return machine_topo_is_smp(ms) ? ms->topo.smp.sockets : + ms->topo.hybrid.sockets; +} + +unsigned int machine_topo_get_dies(const MachineState *ms) +{ + return machine_topo_is_smp(ms) ? ms->topo.smp.dies : + ms->topo.hybrid.dies; +} + +unsigned int machine_topo_get_clusters(const MachineState *ms) +{ + return machine_topo_is_smp(ms) ? ms->topo.smp.clusters : + ms->topo.hybrid.clusters; +} + +unsigned int machine_topo_get_smp_cores(const MachineState *ms) +{ + g_assert(machine_topo_is_smp(ms)); + return ms->topo.smp.cores; +} + +unsigned int machine_topo_get_smp_threads(const MachineState *ms) +{ + g_assert(machine_topo_is_smp(ms)); + return ms->topo.smp.threads; +} + +unsigned int machine_topo_get_threads(const MachineState *ms, + unsigned int cluster_id, + unsigned int core_id) +{ + if (machine_topo_is_smp(ms)) { + return ms->topo.smp.threads; + } else { + return ms->topo.hybrid.cluster_list[cluster_id] + .core_list[core_id].threads; + } + + return 0; +} + +unsigned int machine_topo_get_cores(const MachineState *ms, + unsigned int cluster_id) +{ + if (machine_topo_is_smp(ms)) { + return ms->topo.smp.cores; + } else { + return ms->topo.hybrid.cluster_list[cluster_id].cores; + } +}Is it possible to use variadic function so that those two smp specific helpers can be avoided? It's a bit wired that we have the generic machine_topo_get_threads but also need machine_topo_get_smp_threads at the same time.I am not sure about this, because variadic functions unify function naming, but eliminate the "smp-specific" information from the name. Trying to get the cres/threads without considering the cpu index can only be used in smp scenarios, and I think the caller needs to understand that he knows it's smp.
Ok, I get the point. When it comes to the naming, would it be more concise to remove the *_get_* in the fun name, such as machine_topo_get_cpus to machine_topo_cpus, machine_topo_get_clusters to machine_topo_clusters. And maybe rename machine_topo_get_cores(int cluster_id, int core_id) to machine_topo_cores_by_ids? Or machine_topo_get_cores() to machine_topo_cores_by_topo_ids() and machine_topo_get_cores_by_idx to machine_topo_cores_by_cpu_idx()
+ +unsigned int machine_topo_get_threads_by_idx(const MachineState *ms, + unsigned int cpu_index) +{ + unsigned cpus_per_die; + unsigned tmp_idx; + HybridCluster *cluster; + HybridCore *core; + + if (machine_topo_is_smp(ms)) { + return ms->topo.smp.threads; + } + + cpus_per_die = ms->topo.max_cpus / (ms->topo.hybrid.sockets * + ms->topo.hybrid.dies); + tmp_idx = cpu_index % cpus_per_die; + + for (int i = 0; i < ms->topo.hybrid.clusters; i++) { + cluster = &ms->topo.hybrid.cluster_list[i]; + + for (int j = 0; j < cluster->cores; j++) { + core = &cluster->core_list[j]; + + if (tmp_idx < core->threads) { + return core->threads; + } else { + tmp_idx -= core->threads; + } + } + } + + return 0; +} + +unsigned int machine_topo_get_cores_by_idx(const MachineState *ms, + unsigned int cpu_index) +{ + unsigned cpus_per_die; + unsigned tmp_idx; + HybridCluster *cluster; + HybridCore *core; + + if (machine_topo_is_smp(ms)) { + return ms->topo.smp.cores; + } + + cpus_per_die = ms->topo.max_cpus / (ms->topo.hybrid.sockets * + ms->topo.hybrid.dies); + tmp_idx = cpu_index % cpus_per_die; + + for (int i = 0; i < ms->topo.hybrid.clusters; i++) { + cluster = &ms->topo.hybrid.cluster_list[i]; + + for (int j = 0; j < cluster->cores; j++) { + core = &cluster->core_list[j]; + + if (tmp_idx < core->threads) { + return cluster->cores; + } else { + tmp_idx -= core->threads; + } + } + } + + return 0; +} + +unsigned int machine_topo_get_cores_per_socket(const MachineState *ms) +{ + unsigned int cores_per_die = 0; + + if (machine_topo_is_smp(ms)) { + return ms->topo.smp.cores * ms->topo.smp.clusters * ms->topo.smp.dies; + } + + for (int i = 0; i < ms->topo.hybrid.clusters; i++) { + cores_per_die += ms->topo.hybrid.cluster_list[i].cores; + } + + return cores_per_die * ms->topo.hybrid.dies; +} + +unsigned int machine_topo_get_threads_per_socket(const MachineState *ms) +{ + unsigned int sockets = machine_topo_is_smp(ms) ? ms->topo.smp.sockets : + ms->topo.hybrid.sockets; + return ms->topo.max_cpus / sockets; +} + /* * Report information of a machine's supported CPU topology hierarchy. * Topology members will be ordered from the largest to the smallest diff --git a/include/hw/boards.h b/include/hw/boards.h index 0a61855499e3..34b64b012022 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -461,4 +461,39 @@ extern const size_t hw_compat_2_2_len; extern GlobalProperty hw_compat_2_1[]; extern const size_t hw_compat_2_1_len; +static inline +unsigned int machine_topo_get_cpus(const MachineState *ms) +{ + return ms->topo.cpus; +} + +static inline +unsigned int machine_topo_get_max_cpus(const MachineState *ms) +{ + return ms->topo.max_cpus; +} + +static inline +bool machine_topo_is_smp(const MachineState *ms) +{ + return ms->topo.topo_type == CPU_TOPO_TYPE_SMP; +} + +unsigned int machine_topo_get_sockets(const MachineState *ms); +unsigned int machine_topo_get_dies(const MachineState *ms); +unsigned int machine_topo_get_clusters(const MachineState *ms); +unsigned int machine_topo_get_smp_cores(const MachineState *ms); +unsigned int machine_topo_get_smp_threads(const MachineState *ms); +unsigned int machine_topo_get_threads(const MachineState *ms, + unsigned int cluster_id, + unsigned int core_id); +unsigned int machine_topo_get_cores(const MachineState *ms, + unsigned int cluster_id); +unsigned int machine_topo_get_threads_by_idx(const MachineState *ms, + unsigned int cpu_index); +unsigned int machine_topo_get_cores_by_idx(const MachineState *ms, + unsigned int cpu_index); +unsigned int machine_topo_get_cores_per_socket(const MachineState *ms); +unsigned int machine_topo_get_threads_per_socket(const MachineState *ms); + #endifI think it's necessary to document the ablity for each helper. For example, at a flance, I cant figure out what machine_topo_get_threads_idx does. Add some something like: /* * Get number of threads within the CPU core where a processor locates, * according to the processor index. * * @param: ... */ will be friendly to future users.Yeah, thanks! I will.Thanks, Yanan
| [Prev in Thread] | Current Thread | [Next in Thread] |