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

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3150 bytes --]

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
following:

 ("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
series).

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?

Regards,
Dario

-- 
<<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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel