[PATCH 0/3] Automatically derive soft affinity from vnuma information

[PATCH 0/3] Automatically derive soft affinity from vnuma information

[Dario Faggioli]

Hi,

This small series is meant at achieving the following two goals:
 - when building a vnuma enabled guest, if any vcpu hard or soft affinity is
   specified, and that does not match with the vnuma configuration supplied
   (e.g., vcpu X, belonging to vnode Y on pnode W, has affinity with pcpus
   outside of pnode W), we want to warn the user about it;
 - when building a vnuma enabled guest, if no _soft_ affinity is specified, we
   want to automatically build it up, basing on the vnuma configuration supplied
   (e.g., vcpu X, belonging to vnode Y on pnode W will have soft affinity with
   pcpus of pnode W).

IOW, with the following in the config file:

 vcpus  = '4'
 memory = '2000'
 vnuma  = [ [ "pnode=0","size=1000","vcpus=0-1","vdistances=10,20"  ],
            [ "pnode=1","size=1000","vcpus=2-3","vdistances=20,10"  ] ]

We'll, automatically, end up with this:

 root@Zhaman:~# xl vcpu-list 1
 Name                                ID  VCPU   CPU State   Time(s) Affinity (Hard / Soft)
 test-pv                              1     0    7   -b-       8.1  all / 0-7
 test-pv                              1     1    3   -b-       6.2  all / 0-7
 test-pv                              1     2   10   -b-       0.1  all / 8-15
 test-pv                              1     3    8   -b-       0.1  all / 8-15

While at it, I'm taking the chance (in patch #3) for doing some trivial
cleanups.

Thanks and Regards,
Dario

---
Dario Faggioli (3):
      libxl: check whether vcpu affinity and vnuma info match
      libxl: automatically set soft affinity after vnuma info
      libxl: cleanup some misuse of 'cpumap' as parameter

 tools/libxl/libxl_dom.c   |   46 ++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_utils.h |    6 +++--
 tools/libxl/libxl_vnuma.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 4 deletions(-)

[PATCH 1/3] libxl: check whether vcpu affinity and vnuma info match

[Dario Faggioli]

More specifically, vcpus are assigned to a vnode, which in
turn is associated with a pnode. If a vcpu also has, in its
(hard or soft) affinity, some pcpus that are not part of the
said pnode, print a warning to the user.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_vnuma.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/tools/libxl/libxl_vnuma.c b/tools/libxl/libxl_vnuma.c
index aad297e..3383724 100644
--- a/tools/libxl/libxl_vnuma.c
+++ b/tools/libxl/libxl_vnuma.c
@@ -33,6 +33,43 @@ static int compare_vmemrange(const void *a, const void *b)
     return 0;
 }
 
+/* Check if a vcpu has an hard (or soft) affinity set in such
+ * a way that it does not match the pnode to which the vcpu itself
+ * is assigned to.
+ */
+static int check_vnuma_affinity(libxl__gc *gc,
+                                 unsigned int vcpu,
+                                 unsigned int pnode,
+                                 unsigned int num_affinity,
+                                 const libxl_bitmap *affinity,
+                                 const char *kind)
+{
+    libxl_bitmap nodemap;
+    int rc = 0;
+
+    libxl_bitmap_init(&nodemap);
+
+    rc = libxl_node_bitmap_alloc(CTX, &nodemap, 0);
+    if (rc) {
+        LOG(ERROR, "Can't allocate nodemap");
+        goto out;
+    }
+
+    rc = libxl_cpumap_to_nodemap(CTX, affinity, &nodemap);
+    if (rc) {
+        LOG(ERROR, "Can't convert Vcpu %d affinity to nodemap", vcpu);
+        goto out;
+    }
+
+    if (libxl_bitmap_count_set(&nodemap) != 1 ||
+        !libxl_bitmap_test(&nodemap, pnode))
+        LOG(WARN, "Vcpu %d %s affinity and vnuma info mismatch", vcpu, kind);
+
+out:
+    libxl_bitmap_dispose(&nodemap);
+    return rc;
+}
+
 /* Check if vNUMA configuration is valid:
  *  1. all pnodes inside vnode_to_pnode array are valid
  *  2. each vcpu belongs to one and only one vnode
@@ -101,6 +138,23 @@ int libxl__vnuma_config_check(libxl__gc *gc,
         }
     }
 
+    /* Check whether vcpu affinity (if any) matches vnuma configuration */
+    for (i = 0; i < b_info->num_vnuma_nodes; i++) {
+        v = &b_info->vnuma_nodes[i];
+        libxl_for_each_set_bit(j, v->vcpus) {
+            if (b_info->num_vcpu_hard_affinity > j)
+                check_vnuma_affinity(gc, j, v->pnode,
+                                     b_info->num_vcpu_hard_affinity,
+                                     &b_info->vcpu_hard_affinity[j],
+                                     "hard");
+            if (b_info->num_vcpu_soft_affinity > j)
+                check_vnuma_affinity(gc, j, v->pnode,
+                                     b_info->num_vcpu_soft_affinity,
+                                     &b_info->vcpu_soft_affinity[j],
+                                     "soft");
+        }
+    }
+
     /* Check vmemranges */
     qsort(state->vmemranges, state->num_vmemranges, sizeof(xen_vmemrange_t),
           compare_vmemrange);

Re: [PATCH 1/3] libxl: check whether vcpu affinity and vnuma info match

[Wei Liu]

On Tue, Mar 24, 2015 at 02:41:48PM +0100, Dario Faggioli wrote:
> More specifically, vcpus are assigned to a vnode, which in
> turn is associated with a pnode. If a vcpu also has, in its
> (hard or soft) affinity, some pcpus that are not part of the
> said pnode, print a warning to the user.
> 

Currently all the checks that returns error are all guest visible
mis-configurations. I'm trying to reason whether we should return an
error or just print a warning.

What is the outcome if you have conflicting setting in vNUMA and vcpu
affinity? I guess it's just performance penalty but nothing guest
visible could happen?

Wei.

Re: [PATCH 1/3] libxl: check whether vcpu affinity and vnuma info match

[Dario Faggioli]

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

On Tue, 2015-03-24 at 14:41 +0000, Wei Liu wrote:
> On Tue, Mar 24, 2015 at 02:41:48PM +0100, Dario Faggioli wrote:

> Currently all the checks that returns error are all guest visible
> mis-configurations. I'm trying to reason whether we should return an
> error or just print a warning.
> 
> What is the outcome if you have conflicting setting in vNUMA and vcpu
> affinity? 
>
The outcome is that vcpus will either always run (if hard affinity and
vnuma info mismatch) or prefer to run (if soft affinity and vnuma info
mismatch) on pcpus from pnode(s) different from the pnode specified in
vnuma configuration, and hence where the memory is.

So, for instance, with this:

 vnuma = [ [ "pnode=0","size=1000","vcpus=0-1","vdistances=10,20"  ],
           [ "pnode=1","size=1000","vcpus=2-3","vdistances=20,10"  ] ]

and this:

 cpus_soft = "node:1"
 cpus = "node:0"

in the config file, we have a soft affinity mismatch for vcpus 0,1 and
an hard affinity mismatch for vcpus 2,3.

This means that vcpus 0,1 can run everywhere, but they will prefer to
run on pcpus from pnode 1, while, when the domain was built, they've
been assigned to vnode 0, which has its memory allocated from pnode 0.
This will (potentially) cause a lot of remote memory accesses.

It also means that vcpus 2,3 will only run on pcpus from node:0, while
they've been assigned to vnode 1, which has its memory allocated from
pnode 1. This will mean all memory accesses will be remote memory
accesses

So no functional consequences, but performance will most likely be
affected.

> I guess it's just performance penalty but nothing guest
> visible could happen?
> 
Exactly.

Regards,
Dario

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

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

Re: [PATCH 1/3] libxl: check whether vcpu affinity and vnuma info match

[Wei Liu]

On Tue, Mar 24, 2015 at 03:47:27PM +0000, Dario Faggioli wrote:
> On Tue, 2015-03-24 at 14:41 +0000, Wei Liu wrote:
> > On Tue, Mar 24, 2015 at 02:41:48PM +0100, Dario Faggioli wrote:
> 
> > Currently all the checks that returns error are all guest visible
> > mis-configurations. I'm trying to reason whether we should return an
> > error or just print a warning.
> > 
> > What is the outcome if you have conflicting setting in vNUMA and vcpu
> > affinity? 
> >
> The outcome is that vcpus will either always run (if hard affinity and
> vnuma info mismatch) or prefer to run (if soft affinity and vnuma info
> mismatch) on pcpus from pnode(s) different from the pnode specified in
> vnuma configuration, and hence where the memory is.
> 
> So, for instance, with this:
> 
>  vnuma = [ [ "pnode=0","size=1000","vcpus=0-1","vdistances=10,20"  ],
>            [ "pnode=1","size=1000","vcpus=2-3","vdistances=20,10"  ] ]
> 
> and this:
> 
>  cpus_soft = "node:1"
>  cpus = "node:0"
> 
> in the config file, we have a soft affinity mismatch for vcpus 0,1 and
> an hard affinity mismatch for vcpus 2,3.
> 
> This means that vcpus 0,1 can run everywhere, but they will prefer to
> run on pcpus from pnode 1, while, when the domain was built, they've
> been assigned to vnode 0, which has its memory allocated from pnode 0.
> This will (potentially) cause a lot of remote memory accesses.
> 
> It also means that vcpus 2,3 will only run on pcpus from node:0, while
> they've been assigned to vnode 1, which has its memory allocated from
> pnode 1. This will mean all memory accesses will be remote memory
> accesses
> 
> So no functional consequences, but performance will most likely be
> affected.
> 
> > I guess it's just performance penalty but nothing guest
> > visible could happen?
> > 
> Exactly.

OK. I think we can be lenient on this sort of thing. I think you also
need to add to the comment before libxl__vnuma_config_check.

With that change added:

Acked-by: Wei Liu <wei.liu2@citrix.com>

> 
> Regards,
> Dario

[PATCH 2/3] libxl: automatically set soft affinity after vnuma info

[Dario Faggioli]

More specifically, vcpus are assigned to a vnode, which in
turn is associated with a pnode. If a vcpu does not have any
soft affinity, automatically build up one, matching the pcpus
of the said pnode.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_dom.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index ace8a66..554ea68 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -439,6 +439,44 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     return rc;
 }
 
+static int set_vnuma_affinity(libxl__gc *gc, uint32_t domid,
+                              libxl_domain_build_info *info)
+{
+    libxl_bitmap cpumap;
+    libxl_vnode_info *v;
+    unsigned int i, j;
+    int rc = 0;
+
+    libxl_bitmap_init(&cpumap);
+
+    rc = libxl_cpu_bitmap_alloc(CTX, &cpumap, 0);
+    if (rc) {
+        LOG(ERROR, "Can't allocate nodemap");
+        goto out;
+    }
+
+    /*
+     * For each vcpu in each vnode, set its soft affinity to
+     * the pcpus belonging to the pnode the vnode is on
+     */
+    for (i = 0; i < info->num_vnuma_nodes; i++) {
+        v = &info->vnuma_nodes[i];
+
+        rc = libxl_node_to_cpumap(CTX, v->pnode, &cpumap);
+        if (rc) {
+            LOG(ERROR, "Can't get cpumap for vnode %d", i);
+            goto out;
+        }
+
+        libxl_for_each_set_bit(j, v->vcpus)
+            libxl_set_vcpuaffinity(CTX, domid, j, NULL, &cpumap);
+    }
+
+out:
+    libxl_bitmap_dispose(&cpumap);
+    return rc;
+}
+
 int libxl__build_post(libxl__gc *gc, uint32_t domid,
                       libxl_domain_build_info *info,
                       libxl__domain_build_state *state,
@@ -450,6 +488,12 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     char **ents;
     int i, rc;
 
+    if (info->num_vnuma_nodes && !info->num_vcpu_soft_affinity) {
+        rc = set_vnuma_affinity(gc, domid, info);
+        if (rc)
+            return rc;
+    }
+
     rc = libxl_domain_sched_params_set(CTX, domid, &info->sched_params);
     if (rc)
         return rc;

Re: [PATCH 2/3] libxl: automatically set soft affinity after vnuma info

[Wei Liu]

On Tue, Mar 24, 2015 at 02:42:15PM +0100, Dario Faggioli wrote:
> More specifically, vcpus are assigned to a vnode, which in
> turn is associated with a pnode. If a vcpu does not have any
> soft affinity, automatically build up one, matching the pcpus
> of the said pnode.
> 

What about hard affinity? Do we need to generate hard affinity map as
well? What's your thought on this?

The code itself looks good to me.

Wei.

Re: [PATCH 2/3] libxl: automatically set soft affinity after vnuma info

[Dario Faggioli]

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

On Tue, 2015-03-24 at 14:32 +0000, Wei Liu wrote:
> On Tue, Mar 24, 2015 at 02:42:15PM +0100, Dario Faggioli wrote:
> > More specifically, vcpus are assigned to a vnode, which in
> > turn is associated with a pnode. If a vcpu does not have any
> > soft affinity, automatically build up one, matching the pcpus
> > of the said pnode.
> > 
> 
> What about hard affinity? Do we need to generate hard affinity map as
> well? What's your thought on this?
> 
I think we should not do that. Hard affinity has a bigger impact on the
system performance and overall configuration than soft, which is really
just a preference, so I would let the user deal with that explicitly.

For example, setting the hard affinity to the same set of pcpus for
multiple domains could, depending on the system load, prevent some of
(the vcpus of) these domain to be able to execute for quite some time,
while there are idle pcpus in the system. This may well be something the
user wants, but I'd let him say so, if that is the case, rather than
automatically setup things like that for everyone! By only affecting
soft affinity, that doesn't happen, and we stay work conserving by
default, which is important.

Also, only touching soft affinity is exactly what automatic NUMA
placement already does, which in turn is consistent with using the
result of placement to influence the domain's node affinity (i.e., what
Xen 4.3 and 4.4 do, since soft affinity was not there), and it is the
best solution, IMO.

Perhaps we can have another config option, telling whether one wants
vnuma (and/or automatic placement) to affect soft or hard affinity (or
both). I'm not sure how much this would be useful, while it would
certainly make things more complex from both the implementation and the
interface point of view (the latter being the more concerning aspect).
In any case, we can add this at any point in future, if we feel it could
be useful, and I still think the default setting --if such option is not
specified-- should remain "affect soft affinity only".

> The code itself looks good to me.
> 
Thanks for looking at it. :-)

Regards,
Dario

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

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

Re: [PATCH 2/3] libxl: automatically set soft affinity after vnuma info

[Wei Liu]

On Tue, Mar 24, 2015 at 03:06:24PM +0000, Dario Faggioli wrote:
> On Tue, 2015-03-24 at 14:32 +0000, Wei Liu wrote:
> > On Tue, Mar 24, 2015 at 02:42:15PM +0100, Dario Faggioli wrote:
> > > More specifically, vcpus are assigned to a vnode, which in
> > > turn is associated with a pnode. If a vcpu does not have any
> > > soft affinity, automatically build up one, matching the pcpus
> > > of the said pnode.
> > > 
> > 
> > What about hard affinity? Do we need to generate hard affinity map as
> > well? What's your thought on this?
> > 
> I think we should not do that. Hard affinity has a bigger impact on the
> system performance and overall configuration than soft, which is really
> just a preference, so I would let the user deal with that explicitly.
> 
> For example, setting the hard affinity to the same set of pcpus for
> multiple domains could, depending on the system load, prevent some of
> (the vcpus of) these domain to be able to execute for quite some time,
> while there are idle pcpus in the system. This may well be something the
> user wants, but I'd let him say so, if that is the case, rather than
> automatically setup things like that for everyone! By only affecting
> soft affinity, that doesn't happen, and we stay work conserving by
> default, which is important.
> 
> Also, only touching soft affinity is exactly what automatic NUMA
> placement already does, which in turn is consistent with using the
> result of placement to influence the domain's node affinity (i.e., what
> Xen 4.3 and 4.4 do, since soft affinity was not there), and it is the
> best solution, IMO.
> 
> Perhaps we can have another config option, telling whether one wants
> vnuma (and/or automatic placement) to affect soft or hard affinity (or
> both). I'm not sure how much this would be useful, while it would
> certainly make things more complex from both the implementation and the
> interface point of view (the latter being the more concerning aspect).
> In any case, we can add this at any point in future, if we feel it could
> be useful, and I still think the default setting --if such option is not
> specified-- should remain "affect soft affinity only".

Right. Thanks for the explanation. Regarding new option, we can add it
later if it turns out to be necessary.

Acked-by: Wei Liu <wei.liu2@citrix.com>

> 
> > The code itself looks good to me.
> > 
> Thanks for looking at it. :-)
> 
> Regards,
> Dario

[PATCH 3/3] libxl: cleanup some misuse of 'cpumap' as parameter

[Dario Faggioli]

in favour of the more generic 'bitmap', which is better
since these are generic libxl_bitmap_* functions.

Also fix a typo, and remove a stale (and wrong) comment.

No functional change intended.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_dom.c   |    2 +-
 tools/libxl/libxl_utils.h |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 554ea68..26a0382 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -376,7 +376,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     }
     if (info->nodemap.size)
         libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
-    /* As mentioned in libxl.h, vcpu_hard_array takes precedence */
+
     if (info->num_vcpu_hard_affinity || info->num_vcpu_soft_affinity) {
         libxl_bitmap *hard_affinity, *soft_affinity;
         int i, n_vcpus;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..68b5580 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -89,8 +89,8 @@ int libxl_bitmap_is_empty(const libxl_bitmap *bitmap);
 int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
 void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
 void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
-int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
-char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
+int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
+char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap);
 static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
 {
     memset(bitmap->map, -1, bitmap->size);
@@ -145,7 +145,7 @@ int libxl_node_to_cpumap(libxl_ctx *ctx, int node,
                          libxl_bitmap *cpumap);
 /* Populate nodemap with the nodes of the cpus in cpumap */
 int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
-                            const libxl_bitmap *cpuemap,
+                            const libxl_bitmap *cpumap,
                             libxl_bitmap *nodemap);
 
  static inline uint32_t libxl__sizekb_to_mb(uint32_t s) {

Re: [PATCH 3/3] libxl: cleanup some misuse of 'cpumap' as parameter

[Wei Liu]

On Tue, Mar 24, 2015 at 02:42:27PM +0100, Dario Faggioli wrote:
> in favour of the more generic 'bitmap', which is better
> since these are generic libxl_bitmap_* functions.
> 
> Also fix a typo, and remove a stale (and wrong) comment.
> 
> No functional change intended.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>