All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dario Faggioli <dario.faggioli@citrix.com>
To: xen-devel@lists.xen.org
Cc: Andrew.Cooper3@citrix.com, Wei Liu <wei.liu2@citrix.com>,
	Ian.Campbell@citrix.com, Ian.Jackson@citrix.com
Subject: [PATCH v10 05/11] libxl: Change default for b_info->{cpu, node}map to "not allocated"
Date: Fri, 20 Jun 2014 18:19:29 +0200	[thread overview]
Message-ID: <20140620161929.450.5352.stgit@Solace> (raw)
In-Reply-To: <20140620161751.450.15846.stgit@Solace>

by avoiding allocating them in libxl__domain_build_info_setdefault.
In fact, back in 7e449837 ("libxl: provide _init and _setdefault for
libxl_domain_build_info") and a5d30c23 ("libxl: allow for explicitly
specifying node-affinity"), it was decided that the default for these
fields was for them to be allocted and filled.

That is now causing problem, whenever we have to figure out whether
the caller is using or not one of those fields. In fact, when we see
a full bitmap, is it just the default value, or is the user that
wants it that way?

Since that kind of knowledge has become important, change the default
to be "bitmap not allocated". It then becomes easy to know whether a
libxl caller is using one of the fields, just by checking whether the
bitmap is actually there with a non-zero size.

This is very important for the following patches introducing new ways
of specifying hard and soft affinity. It also allows us to improve
the checks around NUMA automatic placement, during domain creation
(and that bit is done in this very patch).

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Chenges from v9:
 * new patch. Doing something like thiswas discussed on xen-devel,
   during v9 and (I think) v7 review.
---
 tools/libxl/libxl_create.c |   12 ------------
 tools/libxl/libxl_dom.c    |   35 +++++++++++++++++++++++------------
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index d6b8a29..8cc3ca5 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -187,20 +187,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     } else if (b_info->avail_vcpus.size > HVM_MAX_VCPUS)
         return ERROR_FAIL;
 
-    if (!b_info->cpumap.size) {
-        if (libxl_cpu_bitmap_alloc(CTX, &b_info->cpumap, 0))
-            return ERROR_FAIL;
-        libxl_bitmap_set_any(&b_info->cpumap);
-    }
-
     libxl_defbool_setdefault(&b_info->numa_placement, true);
 
-    if (!b_info->nodemap.size) {
-        if (libxl_node_bitmap_alloc(CTX, &b_info->nodemap, 0))
-            return ERROR_FAIL;
-        libxl_bitmap_set_any(&b_info->nodemap);
-    }
-
     if (b_info->max_memkb == LIBXL_MEMKB_DEFAULT)
         b_info->max_memkb = 32 * 1024;
     if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index a90a8d5..5d8a0be 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -241,28 +241,39 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     }
 
     /*
-     * Check if the domain has any CPU affinity. If not, try to build
-     * up one. In case numa_place_domain() find at least a suitable
-     * candidate, it will affect info->nodemap accordingly; if it
-     * does not, it just leaves it as it is. This means (unless
-     * some weird error manifests) the subsequent call to
-     * libxl_domain_set_nodeaffinity() will do the actual placement,
-     * whatever that turns out to be.
+     * Check if the domain has any CPU or node affinity already. If not, try
+     * to build up the latter via automatic NUMA placement. In fact, in case
+     * numa_place_domain() manage to find a placement, in info->nodemap is
+     * updated accordingly; if it does not manage, inf->nodemap is just left
+     * alone. It is then the the subsequent call to
+     * libxl_domain_set_nodeaffinity() that enacts the actual placement.
      */
     if (libxl_defbool_val(info->numa_placement)) {
-        if (!libxl_bitmap_is_full(&info->cpumap)) {
+        if (info->cpumap.size) {
             LOG(ERROR, "Can run NUMA placement only if no vcpu "
-                       "affinity is specified");
+                       "affinity is specified explicitly");
             return ERROR_INVAL;
         }
+        if (info->nodemap.size) {
+            LOG(ERROR, "Can run NUMA placement only if the domain does not "
+                       "have any NUMA node affinity set already");
+            return ERROR_INVAL;
+        }
+
+        rc = libxl_node_bitmap_alloc(ctx, &info->nodemap, 0);
+        if (rc)
+            return rc;
+        libxl_bitmap_set_any(&info->nodemap);
 
         rc = numa_place_domain(gc, domid, info);
         if (rc)
             return rc;
     }
-    libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
-    libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
-                               &info->cpumap, NULL);
+    if (info->nodemap.size)
+        libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
+    if (info->cpumap.size)
+        libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,
+                                   &info->cpumap, NULL);
 
     if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
         LIBXL_MAXMEM_CONSTANT) < 0) {

  parent reply	other threads:[~2014-06-20 16:19 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-20 16:18 [PATCH v10 00/11] Series short description Dario Faggioli
2014-06-20 16:18 ` [PATCH v10 01/11] libxc/libxl: bump library SONAMEs Dario Faggioli
2014-06-20 16:19 ` [PATCH v10 02/11] libxc: get and set soft and hard affinity Dario Faggioli
2014-06-20 16:19 ` [PATCH v10 03/11] libxl: get and set soft affinity Dario Faggioli
2014-06-20 16:19 ` [PATCH v10 04/11] xl: enable getting and setting " Dario Faggioli
2014-06-27 12:33   ` Ian Campbell
2014-06-27 13:18     ` Dario Faggioli
2014-06-27 13:22       ` Ian Campbell
2014-06-27 13:45         ` Dario Faggioli
2014-06-20 16:19 ` Dario Faggioli [this message]
2014-06-27 10:44   ` [PATCH v10 05/11] libxl: Change default for b_info->{cpu, node}map to "not allocated" Ian Campbell
2014-06-27 12:07     ` Dario Faggioli
2014-06-27 12:44       ` Ian Campbell
2014-06-20 16:19 ` [PATCH v10 06/11] libxl/xl: push VCPU affinity pinning down to libxl Dario Faggioli
2014-06-27 10:55   ` Ian Campbell
2014-06-20 16:19 ` [PATCH v10 07/11] libxl/xl: deprecate the build_info->cpumap field Dario Faggioli
2014-06-27 10:57   ` Ian Campbell
2014-06-27 12:08     ` Dario Faggioli
2014-06-20 16:19 ` [PATCH v10 08/11] xl: move away from the (deprecated) use of cpumap for hard affinity Dario Faggioli
2014-06-27 11:00   ` Ian Campbell
2014-06-27 12:24     ` Dario Faggioli
2014-06-20 16:20 ` [PATCH v10 09/11] xl: move the vcpu affinity parsing in a function Dario Faggioli
2014-06-20 16:20 ` [PATCH v10 10/11] libxl/xl: make it possible to specify soft-affinity in domain config file Dario Faggioli
2014-06-27 12:47   ` Ian Campbell
2014-06-27 13:07     ` Dario Faggioli
2014-06-27 16:41     ` Dario Faggioli
2014-06-20 16:20 ` [PATCH v10 11/11] libxl: automatic NUMA placement affects soft affinity Dario Faggioli
2014-06-27 13:32 ` [PATCH v10 00/11] Series short description Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140620161929.450.5352.stgit@Solace \
    --to=dario.faggioli@citrix.com \
    --cc=Andrew.Cooper3@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=Ian.Jackson@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.