qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC 01/13] numa: access CPU's node id via property in


From: Igor Mammedov
Subject: Re: [Qemu-devel] [RFC 01/13] numa: access CPU's node id via property in hmp_info_numa()
Date: Thu, 19 Jan 2017 15:41:56 +0100

On Wed, 18 Jan 2017 16:18:20 -0200
Eduardo Habkost <address@hidden> wrote:

> 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 <address@hidden>  
> 
> 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.
ok, I'll split this in 3 patches

> 
> 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.
rationale for (1) and (2) is the first 2 paragraphs of this commit message




reply via email to

[Prev in Thread] Current Thread [Next in Thread]