On 2022/4/14 15:35, Gavin Shan wrote:
Hi Yanan,
On 4/14/22 10:49 AM, wangyanan (Y) wrote:
On 2022/4/14 10:37, Gavin Shan wrote:
On 4/14/22 10:27 AM, wangyanan (Y) wrote:
On 2022/4/14 8:08, Gavin Shan wrote:
On 4/13/22 8:39 PM, wangyanan (Y) wrote:
On 2022/4/3 22:59, Gavin Shan wrote:
Currently, the SMP configuration isn't considered when the CPU
topology is populated. In this case, it's impossible to provide
the default CPU-to-NUMA mapping or association based on the socket
ID of the given CPU.
This takes account of SMP configuration when the CPU topology
is populated. The die ID for the given CPU isn't assigned since
it's not supported on arm/virt machine yet.
Signed-off-by: Gavin Shan <gs...@redhat.com>
---
hw/arm/virt.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d2e5ecd234..3174526730 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2505,6 +2505,7 @@ static const CPUArchIdList
*virt_possible_cpu_arch_ids(MachineState *ms)
int n;
unsigned int max_cpus = ms->smp.max_cpus;
VirtMachineState *vms = VIRT_MACHINE(ms);
+ MachineClass *mc = MACHINE_GET_CLASS(vms);
if (ms->possible_cpus) {
assert(ms->possible_cpus->len == max_cpus);
@@ -2518,8 +2519,21 @@ static const CPUArchIdList
*virt_possible_cpu_arch_ids(MachineState *ms)
ms->possible_cpus->cpus[n].type = ms->cpu_type;
ms->possible_cpus->cpus[n].arch_id =
virt_cpu_mp_affinity(vms, n);
+
+ assert(!mc->smp_props.dies_supported);
+ ms->possible_cpus->cpus[n].props.has_socket_id = true;
+ ms->possible_cpus->cpus[n].props.socket_id =
+ (n / (ms->smp.clusters * ms->smp.cores *
ms->smp.threads)) %
+ ms->smp.sockets;
No need for "% ms->smp.sockets".
Yeah, lets remove it in v6.
+ ms->possible_cpus->cpus[n].props.has_cluster_id = true;
+ ms->possible_cpus->cpus[n].props.cluster_id =
+ (n / (ms->smp.cores * ms->smp.threads)) %
ms->smp.clusters;
+ ms->possible_cpus->cpus[n].props.has_core_id = true;
+ ms->possible_cpus->cpus[n].props.core_id =
+ (n / ms->smp.threads) % ms->smp.cores;
ms->possible_cpus->cpus[n].props.has_thread_id = true;
- ms->possible_cpus->cpus[n].props.thread_id = n;
+ ms->possible_cpus->cpus[n].props.thread_id =
+ n % ms->smp.threads;
}
return ms->possible_cpus;
}
Otherwise, looks good to me:
Reviewed-by: Yanan Wang <wangyana...@huawei.com>
Thanks for your time to review :)
Oh, after further testing this patch breaks numa-test for aarch64,
which should be checked and fixed. I guess it's because we have
more IDs supported for ARM. We have to fully running the QEMU
tests before sending some patches to ensure that they are not
breaking anything. :)
Thanks for catching the failure and reporting back. I'm not
too much familar with QEMU's test workframe. Could you please
share the detailed commands to reproduce the failure? I will
fix in v6, which will be done in a separate patch :)
There is a reference link: https://wiki.qemu.org/Testing
To catch the failure of this patch: "make check" will be enough.
Thanks for the pointer. The issue is caused by
ms->possible_cpus->cpus[n].props.thread_id.
Before this patch, it's value of [0 ... smp.cpus]. However, it's
always zero
after this patch is applied because '%' operation is applied for the
thread
ID.
What we need to do is to specify SMP configuration for the test case.
I will
add PATCH[v6 5/5] for it.
Better to keep the fix together with this patch for good bisection.
diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index 90bf68a5b3..6178ac21a5 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -223,7 +223,7 @@ static void aarch64_numa_cpu(const void *data)
QTestState *qts;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-machine smp.cpus=2 "
+ cli = make_cli(data, "-machine
smp.cpus=2,smp.sockets=1,smp.cores=1,smp.threads=2 "
Maybe it's better to extend aarch64_numa_cpu() by adding
"socket_id, cluster_id, core_id" configuration to the -numa cpu,
given that we have them for aarch64 now.
Thanks,
Yanan
.
