On Wed, Jan 18, 2017 at 06:13:17PM +0100, Igor Mammedov wrote: > Move vcpu's assocciated numa_node field out of generic CPUState > into inherited classes that actually care about cpu<->numa mapping > and make monitor 'info numa' get vcpu's assocciated node id via > node-id property. > It allows to drop implicit node id intialization in > numa_post_machine_init() and would allow switching to mapping > defined by target's CpuInstanceProperties instead of global > numa_info[i].node_cpu bitmaps. > > As side effect it fixes 'info numa' displaying wrong mapping > for CPUs specified with -device/device_add. > Before patch following CLI would produce: > QEMU -smp 1,sockets=3,maxcpus=3 \ > -device qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0 \ > -numa node,nodeid=0,cpus=0 \ > -numa node,nodeid=1,cpus=1 \ > -numa node,nodeid=2,cpus=2 > (qemu) device_add qemu64-x86_64-cpu,socket-id=2,core-id=0,thread-id=0 > (qemu) info numa > 3 nodes > node 0 cpus: 0 1 2 > node 0 size: 40 MB > node 1 cpus: > node 1 size: 40 MB > node 2 cpus: > node 2 size: 48 MB > > after patch all CPUs are on nodes they are supposed to be: > (qemu) device_add qemu64-x86_64-cpu,socket-id=2,core-id=0,thread-id=0 > (qemu) info numa > 3 nodes > node 0 cpus: 0 > node 0 size: 40 MB > node 1 cpus: 1 > node 1 size: 40 MB > node 2 cpus: 2 > node 2 size: 48 MB > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
So, in addition to the other comments I had, it looks like this patch is doing 3 things at the same time: 1) Adding a "node-id" property to CPU objects. 2) Moving CPUState::numa_node to arch-specific CPU structs. 3) Fixing the bug where the NUMA node info isn't initialized for PC on CPUs created by -device/device_add. All of them are independent from each other. For example, all you need to fix the bug you describe above is (3), which is contained in a single hunk from this patch, that is: > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f721fde..9d2b265 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1978,6 +1978,11 @@ static void pc_cpu_pre_plug(HotplugHandler > *hotplug_dev, > > cs = CPU(cpu); > cs->cpu_index = idx; > + > + idx = numa_get_node_for_cpu(cs->cpu_index); > + if (idx < nb_numa_nodes) { > + cpu->numa_nid = idx; > + } > } All the rest seems irrelevant to fix the bug. (1) and (2) may be useful, but they need separate justification. -- Eduardo