On 4/20/22 7:50 PM, Igor Mammedov wrote:
On Wed, 20 Apr 2022 18:31:02 +0800
Gavin Shan <gshan@redhat.com> wrote:
On 4/20/22 4:32 PM, Igor Mammedov wrote:
On Mon, 18 Apr 2022 10:09:18 +0800
Gavin Shan <gshan@redhat.com> 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. Besides, the used SMP
configuration in qtest/numa-test/aarch64_numa_cpu() is corrcted
to avoid testing failure
Signed-off-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
---
hw/arm/virt.c | 15 ++++++++++++++-
tests/qtest/numa-test.c | 3 ++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d2e5ecd234..5443ecae92 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,20 @@ 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->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;
}
diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index 90bf68a5b3..aeda8c774c 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -223,7 +223,8 @@ 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.clusters=1,smp.cores=1,smp.threads=2 "
Is cluster-less config possible?
(looks like it used to work before and it doesn't after this series)
Nope, it's impossible. This specific test case uses arm/virt machine
where cluster is always supported.mc->smp_props.clusters_supported
has been set to true in hw/arm/virt.c::virt_machine_class_init().
Exactly, the changes to virt_possible_cpu_arch_ids() included in this patch
breaks
the test. It's why the fix to qtest/numa-test has been squashed to this patch,
to
make it 'bit bisect' friendly as Yanan suggested.
so what was error that broke the test?
(probably should be mentioned in commit message)
(also is it possible to split out the test patch into
a separate one and put it before this one)
With amend to the command lines, the following one is used and below error
is raised from the test. The error is mentioned in the commit log in
PATCH[v7 2/4].
-machine smp.cpus=2 \
-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 \
-numa cpu,node-id=1,thread-id=0 \
-numa cpu,node-id=0,thread-id=1
qemu-system-aarch64: -numa cpu,node-id=0,thread-id=1: no match found
(reported from hw/core/machine.c::machine_set_cpu_numa_node())
After the changes to virt_possible_cpu_arch_ids() is applied, "thread-id=1"
isn't valid any more. The CPU topology becomes like below. Note that
mc->smp_props.prefer_sockets is true on arm/virt machine.
index socket cluster core thread
--------------------------------------------
0 0 0 0 0
1 1 0 0 0
With the amended command lines, the topology changes again so
that "thread-id=1" is valid:
index socket cluster core thread
--------------------------------------------
0 0 0 0 0
1 0 0 0 1
It should be ok to split the test/qtest/aarch64_numa_cpu() changes into
a separate patch and put it before this one. In that case, the specified
smp.{socket, cluster, core, threads} isn't used by arm/virt machine yet,
and 'thread-id=2' should be still valid. Lets do this if I need post v8.
Otherwise, I guess it's also fine to squash the test/qtest/aarch64_numa_cpu()
changes into PATCH[2/4], as we're doing.