Re: [Qemu-devel] [PATCH 2.1 00/28] Current state of NUMA series, and hostmem improvements

[Igor Mammedov]

On Fri, 07 Mar 2014 13:20:45 +0100
Paolo Bonzini <address@hidden> wrote:

> Il 07/03/2014 12:59, Andreas Färber ha scritto:
> > Am 05.03.2014 12:30, schrieb Paolo Bonzini:
> >> Il 05/03/2014 12:05, Andreas Färber ha scritto:
> >>> I didn't review it in-depth yet, but minor technical issues apart, I
> >>> think we need to keep NUMA and CPU separate,
> >>
> >> I agree.
> >
> > Here you agreed ...
> >
> >>> Compare that to:
> >>>
> >>> /machine
> >>>   /node[0] # This is not really telling!
> >>>     /socket[0]
> >>>       /core[0]
> >>>         /thread[0] # So CPUState != thread?
> >>>           cpu -> /machine/unassigned/device[0]
> >>>   /unassigned
> >>>     /device[0]
> >>
> >> I think this is better; in our world we can have multiple sockets in the
> >> same NUMA node.  But CPUState == thread, so you can have just /thread[0]
> >> -> /machine/unassigned/device[0].
> >
> > ... but you seem to have missed my point about separation. Here the
> > socket object is a child<> of the NUMA node and would get realized
> > together with it but separate from the link<>ed CPUState.
> 
> Ah, I didn't think the socket object as anything but a container.  For 
> me, "keep NUMA and CPU separate" meant "keep NUMA and CPUState separate".
> 
> >> Alternatively, and to keep CPU + NUMA even *more* separate:
> >>
> >>   /machine
> >>     /node[0]
> >>        /cpu[0] -> /machine/unassigned/device[0]
> >>        ...
> >>     /socket[0]
> >>        /core[0]
> >>           /thread[0] -> /machine/unassigned/device[0]
> >>     /unassigned
> >>        /device[0]
> >
> > Now this is pretty much my proposal ;) except that you retained the
> > criticized "node" as name and moved "socket[0]" out of
> > /machine/unassigned (I had /machine/peripheral in mind for -device) and
> > keep the CPUState out of the socket object.
> >
> > Anthony had requested hot-add to happen via "device_add Xeon4242",
> > adding a full socket object with 6 cores at once. In that case CPUState
> > needs to be an integral part of that socket-derived device for recursive
> > realization. Objects that are just link<>ed to wouldn't get
> > automatically realized.
we possible can't do it in arch independent manner, but if we are talking
about 'pc' machine and would ever model real CPU composition there (is there
reasons to do it?), then composite CPU object could still stay
in internal /machine/unassigned|/machine/peripheral trees in parallel with
public /machine/node[x]/socket[y]/core[z]/link<CPUstate>[j] topology interface

> 
> Yes, if you want to do this then you're right and /socket[n] needs to be 
> a device.
> 
> However, I'd still like it to be mostly a container, and that is why I 
> liked the idea of having /node[n] with "flat" links to the actual 
> CPUStates (and also memdevs).
Is there a point in having flat links to CPUState at /nodeX level,

idea to create [*] /node[x]/socket[y]/core[z]/link<CPUstate>[j] tree, was
suggested as way:
 1. to expose stable arch independent topology interface to user
 2. use * as argument to -device / device_add/del cpu,path=foo to avoid
    exposing arch dependent APIC ID to the user.
while keeping /machine/node/socket/core objects mostly as containers to express
above things.

> 
> >> I think we can.  Children and links look exactly the same from the outside.
> >
> > Well, we can't qom-get/qom-set a path string from/to a child<> property,
> > can we?
> 
> We can get it but not set it.  But Stefan's series provides a way to 
> make links read-only too, and these links should be read-only I think.
CPUState links are readonly only until no hotplug supported.

> 
> Paolo


-- 
Regards,
  Igor