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

Re: [Xen-devel] [PATCH 09 of 10 v3] libxl: have NUMA placement deal with cpupools



On 04/07/12 17:18, Dario Faggioli wrote:
# HG changeset patch
# User Dario Faggioli<raistlin@xxxxxxxx>
# Date 1341416324 -7200
# Node ID 885e2f385601d66179058bfb6bd3960f17d5e068
# Parent  7087d3622ee2051654c9e78fe4829da10c2d46f1
libxl: have NUMA placement deal with cpupools

In such a way that only the cpus belonging to the cpupool of the
domain being placed are considered for the placement itself.

This happens by filtering out all the nodes in which the cpupool has
not any cpu from the placement candidates. After that -- as a cpu pooling
not necessarily happens at NUMA nodes boundaries -- we also make sure
only the actual cpus that are part of the pool are considered when
counting how much processors a placement candidate is able to provide.

Signed-off-by: Dario Faggioli<dario.faggioli@xxxxxxxxxx>
Acked-by: Ian Campbell<ian.campbell@xxxxxxxxxx>
If I'm reading this right, the filtering won't prevent duplicate entries returned from get_numa_candidates; is that right? I.e., suppose you have a 4-node system and you run "xl cpupool-numa-split" to get one pool per node. Before this patch, your generator might return the following sets containing node 0:
{0}
{0,1}
{0,2}
{0,3}
{0,1,2}
{0,1,3}
{0,1,2,3}
{0,2,3}

But now, if the domain is placed in a cpupool that has only numa node 0, it will return 8 copies of {0}. Is that correct?

 -George


---
Changes from v2:
  * fixed typos in comments.

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -132,25 +132,29 @@ static int numa_cmpf(const void *v1, con
  }

  /* The actual automatic NUMA placement routine */
-static int numa_place_domain(libxl__gc *gc, libxl_domain_build_info *info)
+static int numa_place_domain(libxl__gc *gc, uint32_t domid,
+                             libxl_domain_build_info *info)
  {
      int nr_candidates = 0;
      libxl__numa_candidate *candidates = NULL;
      libxl_bitmap candidate_nodemap;
-    libxl_cpupoolinfo *pinfo;
-    int nr_pools, rc = 0;
+    libxl_cpupoolinfo cpupool_info;
+    int i, cpupool, rc = 0;
      uint32_t memkb;

      libxl_bitmap_init(&candidate_nodemap);

-    /* First of all, if cpupools are in use, better not to mess with them */
-    pinfo = libxl_list_cpupool(CTX,&nr_pools);
-    if (!pinfo)
-        return ERROR_FAIL;
-    if (nr_pools>  1) {
-        LOG(NOTICE, "skipping NUMA placement as cpupools are in use");
-        goto out;
-    }
+    /*
+     * Extract the cpumap from the cpupool the domain belong to. In fact,
+     * it only makes sense to consider the cpus/nodes that are in there
+     * for placement.
+     */
+    rc = cpupool = libxl__domain_cpupool(gc, domid);
+    if (rc<  0)
+        return rc;
+    rc = libxl_cpupool_info(CTX,&cpupool_info, cpupool);
+    if (rc)
+        return rc;

      rc = libxl_domain_need_memory(CTX, info,&memkb);
      if (rc)
@@ -162,7 +166,8 @@ static int numa_place_domain(libxl__gc *

      /* Find all the candidates with enough free memory and at least
       * as much pcpus as the domain has vcpus.  */
-    rc = libxl__get_numa_candidates(gc, memkb, info->max_vcpus, 0, 0,
+    rc = libxl__get_numa_candidates(gc, memkb, info->max_vcpus,
+                                    0, 0,&cpupool_info.cpumap,
                                      &candidates,&nr_candidates);
      if (rc)
          goto out;
@@ -188,13 +193,20 @@ static int numa_place_domain(libxl__gc *
      if (rc)
          goto out;

+    /* Avoid trying to set the affinity to cpus that might be in the
+     * nodemap but not in our cpupool. */
+    libxl_for_each_set_bit(i, info->cpumap) {
+        if (!libxl_bitmap_test(&cpupool_info.cpumap, i))
+            libxl_bitmap_reset(&info->cpumap, i);
+    }
+
      LOG(DETAIL, "NUMA placement candidate with %d nodes, %d cpus and "
                  "%"PRIu32" KB free selected", candidates[0].nr_nodes,
                  candidates[0].nr_cpus, candidates[0].free_memkb / 1024);

   out:
      libxl_bitmap_dispose(&candidate_nodemap);
-    libxl_cpupoolinfo_list_free(pinfo, nr_pools);
+    libxl_cpupoolinfo_dispose(&cpupool_info);
      return rc;
  }

@@ -217,7 +229,7 @@ int libxl__build_pre(libxl__gc *gc, uint
       * whatever that turns out to be.
       */
      if (libxl_bitmap_is_full(&info->cpumap)) {
-        int rc = numa_place_domain(gc, info);
+        int rc = numa_place_domain(gc, domid, info);
          if (rc)
              return rc;
      }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2289,14 +2289,17 @@ typedef struct {
   * least that amount of free memory and that number of cpus, respectively. If
   * min_free_memkb and/or min_cpus are 0, the candidates' free memory and 
number
   * of cpus won't be checked at all, which means a candidate will always be
- * considered suitable wrt the specific constraint.  cndts is where the list of
- * exactly nr_cndts candidates is returned. Note that, in case no candidates
- * are found at all, the function returns successfully, but with nr_cndts equal
- * to zero.
+ * considered suitable wrt the specific constraint. suitable_cpumap is useful
+ * for specifying we want only the cpus in that mask to be considered while
+ * generating placement candidates (for example because of cpupools). cndts is
+ * where the list of exactly nr_cndts candidates is returned. Note that, in
+ * case no candidates are found at all, the function returns successfully, but
+ * with nr_cndts equal to zero.
   */
  _hidden int libxl__get_numa_candidates(libxl__gc *gc,
                                  uint32_t min_free_memkb, int min_cpus,
                                  int min_nodes, int max_nodes,
+                                const libxl_bitmap *suitable_cpumap,
                                  libxl__numa_candidate *cndts[], int 
*nr_cndts);

  /* Initialization, allocation and deallocation for placement candidates */
diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c
--- a/tools/libxl/libxl_numa.c
+++ b/tools/libxl/libxl_numa.c
@@ -122,15 +122,27 @@ static void comb_get_nodemap(comb_iter_t
          libxl_bitmap_set(nodemap, it[i]);
  }

+/* Retrieve how many nodes a nodemap spans */
+static int nodemap_to_nr_nodes(const libxl_bitmap *nodemap)
+{
+    int i, nr_nodes = 0;
+
+    libxl_for_each_set_bit(i, *nodemap)
+        nr_nodes++;
+    return nr_nodes;
+}
+
  /* Retrieve the number of cpus that the nodes that are part of the nodemap
- * span. */
+ * span and are also set in suitable_cpumap. */
  static int nodemap_to_nr_cpus(libxl_cputopology *tinfo, int nr_cpus,
+                              const libxl_bitmap *suitable_cpumap,
                                const libxl_bitmap *nodemap)
  {
      int i, nodes_cpus = 0;

      for (i = 0; i<  nr_cpus; i++) {
-        if (libxl_bitmap_test(nodemap, tinfo[i].node))
+        if (libxl_bitmap_test(suitable_cpumap, i)&&
+            libxl_bitmap_test(nodemap, tinfo[i].node))
              nodes_cpus++;
      }
      return nodes_cpus;
@@ -236,13 +248,14 @@ static int cpus_per_node_count(libxl_cpu
  int libxl__get_numa_candidates(libxl__gc *gc,
                                 uint32_t min_free_memkb, int min_cpus,
                                 int min_nodes, int max_nodes,
+                               const libxl_bitmap *suitable_cpumap,
                                 libxl__numa_candidate *cndts[], int *nr_cndts)
  {
      libxl__numa_candidate *new_cndts = NULL;
      libxl_cputopology *tinfo = NULL;
      libxl_numainfo *ninfo = NULL;
      int nr_nodes = 0, nr_cpus = 
0;/tmp/extdiff.HJ7jEN/xen-upstream.hg.2019315297ee/tools/libxl/libxl_numa.c
-    libxl_bitmap nodemap;
+    libxl_bitmap suitable_nodemap, nodemap;
      int array_size, rc;

      libxl_bitmap_init(&nodemap);
@@ -267,6 +280,15 @@ int libxl__get_numa_candidates(libxl__gc
      if (rc)
          goto out;

+    /* Allocate and prepare the map of the node that can be utilized for
+     * placement, basing on the map of suitable cpus. */
+    rc = libxl_node_bitmap_alloc(CTX,&suitable_nodemap, 0);
+    if (rc)
+        goto out;
+    rc = libxl_cpumap_to_nodemap(CTX, suitable_cpumap,&suitable_nodemap);
+    if (rc)
+        goto out;
+
      /*
       * If the minimum number of NUMA nodes is not explicitly specified
       * (i.e., min_nodes == 0), we try to figure out a sensible number of nodes
@@ -314,9 +336,14 @@ int libxl__get_numa_candidates(libxl__gc
          for (comb_ok = comb_init(gc,&comb_iter, nr_nodes, min_nodes); comb_ok;
               comb_ok = comb_next(comb_iter, nr_nodes, min_nodes)) {
              uint32_t nodes_free_memkb;
-            int nodes_cpus;
+            int i, nodes_cpus;

+            /* Get the nodemap for the combination and filter unwanted nodes */
              comb_get_nodemap(comb_iter,&nodemap, min_nodes);
+            libxl_for_each_set_bit(i, nodemap) {
+                if (!libxl_bitmap_test(&suitable_nodemap, i))
+                    libxl_bitmap_reset(&nodemap, i);
+            }

              /* If there is not enough memory in this combination, skip it
               * and go generating the next one... */
@@ -325,7 +352,8 @@ int libxl__get_numa_candidates(libxl__gc
                  continue;

              /* And the same applies if this combination is short in cpus */
-            nodes_cpus = nodemap_to_nr_cpus(tinfo, nr_cpus,&nodemap);
+            nodes_cpus = nodemap_to_nr_cpus(tinfo, nr_cpus, suitable_cpumap,
+&nodemap);
              if (min_cpus&&  nodes_cpus<  min_cpus)
                  continue;

@@ -350,12 +378,13 @@ int libxl__get_numa_candidates(libxl__gc
              new_cndts[*nr_cndts].nr_domains =
                                      nodemap_to_nr_domains(gc, tinfo,&nodemap);
              new_cndts[*nr_cndts].free_memkb = nodes_free_memkb;
-            new_cndts[*nr_cndts].nr_nodes = min_nodes;
+            new_cndts[*nr_cndts].nr_nodes = nodemap_to_nr_nodes(&nodemap);
              new_cndts[*nr_cndts].nr_cpus = nodes_cpus;

              LOG(DEBUG, "NUMA placement candidate #%d found: nr_nodes=%d, "
                         "nr_cpus=%d, free_memkb=%"PRIu32"", *nr_cndts,
-                       min_nodes, new_cndts[*nr_cndts].nr_cpus,
+                       new_cndts[*nr_cndts].nr_nodes,
+                       new_cndts[*nr_cndts].nr_cpus,
                         new_cndts[*nr_cndts].free_memkb / 1024);

              (*nr_cndts)++;
@@ -365,6 +394,7 @@ int libxl__get_numa_candidates(libxl__gc

      *cndts = new_cndts;
   out:
+    libxl_bitmap_dispose(&suitable_nodemap);
      libxl_bitmap_dispose(&nodemap);
      libxl_cputopology_list_free(tinfo, nr_cpus);
      libxl_numainfo_list_free(ninfo, nr_nodes);


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


 


Rackspace

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