|
| From: | Gavin Shan |
| Subject: | Re: [PATCH] hw/arm/virt: Fix CPU's default NUMA node ID |
| Date: | Tue, 15 Feb 2022 16:19:01 +0800 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0 |
On 1/28/22 3:05 PM, wangyanan (Y) via wrote
On 2022/1/26 17:14, Igor Mammedov wrote:On Wed, 26 Jan 2022 13:24:10 +0800 Gavin Shan <gshan@redhat.com> wrote:The default CPU-to-NUMA association is given by mc->get_default_cpu_node_id() when it isn't provided explicitly. However, the CPU topology isn't fully considered in the default association and it causes CPU topology broken warnings on booting Linux guest. For example, the following warning messages are observed when the Linux guest is booted with the following command lines. /home/gavin/sandbox/qemu.main/build/qemu-system-aarch64 \ -accel kvm -machine virt,gic-version=host \ -cpu host \ -smp 6,sockets=2,cores=3,threads=1 \ -m 1024M,slots=16,maxmem=64G \ -object memory-backend-ram,id=mem0,size=128M \ -object memory-backend-ram,id=mem1,size=128M \ -object memory-backend-ram,id=mem2,size=128M \ -object memory-backend-ram,id=mem3,size=128M \ -object memory-backend-ram,id=mem4,size=128M \ -object memory-backend-ram,id=mem4,size=384M \ -numa node,nodeid=0,memdev=mem0 \ -numa node,nodeid=1,memdev=mem1 \ -numa node,nodeid=2,memdev=mem2 \ -numa node,nodeid=3,memdev=mem3 \ -numa node,nodeid=4,memdev=mem4 \ -numa node,nodeid=5,memdev=mem5 : alternatives: patching kernel code BUG: arch topology borken the CLS domain not a subset of the MC domain <the above error log repeats> BUG: arch topology borken the DIE domain not a subset of the NODE domain With current implementation of mc->get_default_cpu_node_id(), CPU#0 to CPU#5 are associated with NODE#0 to NODE#5 separately. That's incorrect because CPU#0/1/2 should be associated with same NUMA node because they're seated in same socket. This fixes the issue by considering the socket when default CPU-to-NUMA is given. With this applied, no more CPU topology broken warnings are seen from the Linux guest. The 6 CPUs are associated with NODE#0/1, but there are no CPUs associated with NODE#2/3/4/5.>From migration point of view it looks fine to me, and doesn't need a compat knob since NUMA data (on virt-arm) only used to construct ACPI tables (and we don't version those unless something is broken by it).Signed-off-by: Gavin Shan <gshan@redhat.com> --- hw/arm/virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 141350bf21..b4a95522d3 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2499,7 +2499,7 @@ virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index) static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx) { - return idx % ms->numa_state->num_nodes; + return idx / (ms->smp.dies * ms->smp.clusters * ms->smp.cores * ms->smp.threads);I'd like for ARM folks to confirm whether above is correct (i.e. socket is NUMA node boundary and also if above topo vars could have odd values. Don't look at horribly complicated x86 as example, but it showed that vendors could stash pretty much anything there, so we should consider it here as well and maybe forbid that in smp virt-arm parser)We now have a generic smp parser in machine-smp.c and it guarantees different machine boards a correct group of topo vars: supported topo vars being valid and value of unsupported ones being 1. I think it's safe to use them here. Or am I missing something else? Also, we may not need to include "dies" here because it's not supported on ARM virt machine. I believe we will always have "ms->smp.dies==1" for this machine.
I'm sorry for the delayed response because I'm just back from two weeks
holiday.
The issue isn't related to CPU topology directly. It's actually related
to the fact: the default NUMA node ID will be picked for one particular
CPU if the associated NUMA node ID isn't provided by users explicitly.
So it's related to the CPU-to-NUMA association.
For example, the CPU-to-NUMA association is breaking socket boundary
without the code change included in this patch when the guest is booted
with the command lines like below. With this patch applied, the CPU-to-NUMA
association is following socket boundary, to make Linux guest happy.
-smp 6,sockets=2,cores=3,threads=1 \
-m 1024M,slots=16,maxmem=64G \
-object memory-backend-ram,id=mem0,size=128M \
-object memory-backend-ram,id=mem1,size=128M \
-object memory-backend-ram,id=mem2,size=128M \
-object memory-backend-ram,id=mem3,size=128M \
-object memory-backend-ram,id=mem4,size=128M \
-object memory-backend-ram,id=mem4,size=384M \
-numa node,nodeid=0,memdev=mem0 \
-numa node,nodeid=1,memdev=mem1 \
-numa node,nodeid=2,memdev=mem2 \
-numa node,nodeid=3,memdev=mem3 \
-numa node,nodeid=4,memdev=mem4 \
-numa node,nodeid=5,memdev=mem5
CPU Core Socket NUMA-Node NUA-Node-with-patch
------------------------------------------------------------------
0 0 0 0 0
1 1 0 1 0
2 2 0 2 0
3 0 1 3 1
4 1 1 4 1
5 2 1 5 1
I think it's fine to include "ms->smp.dies" here since it's always 1 on
virt machine. We needn't change the code once it's supported some day.
} static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
Thanks, Gavin
| [Prev in Thread] | Current Thread | [Next in Thread] |