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

Re: [Xen-devel] [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl

On mar, 2014-06-10 at 15:06 +0100, Wei Liu wrote:
> On Tue, Jun 10, 2014 at 03:01:00PM +0100, Ian Campbell wrote:
> > On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > index 0ea0464..4765fb6 100644
> > > --- a/tools/libxl/libxl_types.idl
> > > +++ b/tools/libxl/libxl_types.idl
> > > @@ -305,6 +305,7 @@ libxl_domain_build_info =
> > > Struct("domain_build_info",[
> > >      ("avail_vcpus",     libxl_bitmap),
> > >      ("cpumap",          libxl_bitmap),
> > >      ("nodemap",         libxl_bitmap),
> > > +    ("vcpu_affinity",   Array(libxl_bitmap, "num_vcpumaps")),
> > 
> > Looking at one of Dario's patches I became confused about how this new
> > field relates to the existing cpumap field.
> > 
> > Am I right that the new field is just a per-vcpu version of the old
> > (which only allows you to set the affinity of every vcpu together)?
> > 
> Yes, you're right. The old field denotes the PCPUs this domain is
> allowed to run on. The new array specifies the PCPUs each VCPU can run
> on.
Well, there's no "new" and "old". I mean, as can clearly be seen in the
hunk above, 'cpumap' is still there, and vcpu_affinity is being added.

Fact is, considering how xl parses XXX in "cpus=XXX", only one between
'cpumap' and 'vcpu_affinity' will contain actual information.

With my series on top of this one (or vice versa), we'll have:

 ("cpumap",         libxl_bitmap)
 ("cpumap_soft",    libxl_bitmap)
 ("vcpu_affinity",  Array(libxl_bitmap, "num_vcpumaps"))

which is missing the soft affinity equivalent of 'vcpu_affinity'.
meaning that, either me or Wei, when rebasing on top of the other series
which went in first, will have to add it, meaning we'll end up with the

 ("cpumap",              libxl_bitmap)
 ("cpumap_soft",         libxl_bitmap)
 ("vcpu_hard_affinity",  Array(libxl_bitmap, "num_vcpumaps"))
 ("vcpu_soft_affinity",  Array(libxl_bitmap, "num_vcpumaps"))

which is a lot of fields, and not particularly easy to understand, IMHO.

> > Can this relationship be mentioned in the commit message and/or comments
> > please.
> > 
I think we could go farther than that... way farther. I mean, "cpumap"
contains the vcpu affinity to be applied to all the vcpus of the domain,
if the config file was something like "cpus="1,3-6". "vcpu_affinity"
contains a cpumap for each vcpu, if the config file was something like
"cpus=["1", "5"] (and all this repeated for soft affinity, with my

Can't we kill "cpumap" (and "cpumap_soft" too, in my series) and use
"vcpu_affinity" (i.e., "vcpu_hard_affinity" and "vcpu_soft_affinity"
after my series) only? What would happen is, in case we find
"cpus="4-5", we set all the bitmaps of "vcpu_hard_affinity" to "4-5".

What do you think?


<<This happens because I choose it to happen!>> (Raistlin Majere)
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.