] Automatically place guest on host's NUMA nodes with xl

In preparation for adding an xc_nodemap_t and its related hadling logic. Just add one indirection layer, and try to retain the interface as much as possible (although some small bits here and there have been affected).

Signed-off-by Dario Faggioli<[email protected]>

tools/libxc/xc_cpupool.c +2 -2

@@ -90,7 +90,7 @@ xc_cpupoolinfo_t *xc_cpupool_getinfo(xc_
      sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_INFO;
      sysctl.u.cpupool_op.cpupool_id = poolid;
      set_xen_guest_handle(sysctl.u.cpupool_op.cpumap.bitmap, local);
-    sysctl.u.cpupool_op.cpumap.nr_cpus = local_size * 8;
+    sysctl.u.cpupool_op.cpumap.nr_elems = local_size * 8;
 
      err = do_sysctl_save(xch,&sysctl);
 

@@ -184,7 +184,7 @@ xc_cpumap_t xc_cpupool_freeinfo(xc_inter
      sysctl.cmd = XEN_SYSCTL_cpupool_op;
      sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_FREEINFO;
      set_xen_guest_handle(sysctl.u.cpupool_op.cpumap.bitmap, local);
-    sysctl.u.cpupool_op.cpumap.nr_cpus = mapsize * 8;
+    sysctl.u.cpupool_op.cpumap.nr_elems = mapsize * 8;
 
      err = do_sysctl_save(xch,&sysctl);
 

tools/libxc/xc_domain.c +2 -2

@@ -142,7 +142,7 @@ int xc_vcpu_setaffinity(xc_interface *xc
 
      set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap, local);
 
-    domctl.u.vcpuaffinity.cpumap.nr_cpus = cpusize * 8;
+    domctl.u.vcpuaffinity.cpumap.nr_elems = cpusize * 8;
 
      ret = do_domctl(xch,&domctl);
 

@@ -182,7 +182,7 @@ int xc_vcpu_getaffinity(xc_interface *xc
      domctl.u.vcpuaffinity.vcpu = vcpu;
 
      set_xen_guest_handle(domctl.u.vcpuaffinity.cpumap.bitmap, local);
-    domctl.u.vcpuaffinity.cpumap.nr_cpus = cpusize * 8;
+    domctl.u.vcpuaffinity.cpumap.nr_elems = cpusize * 8;
 
      ret = do_domctl(xch,&domctl);
 

tools/libxc/xc_tbuf.c +1 -1

@@ -134,7 +134,7 @@ int xc_tbuf_set_cpu_mask(xc_interface *x
      bitmap_64_to_byte(bytemap,&mask64, sizeof (mask64) * 8);
 
      set_xen_guest_handle(sysctl.u.tbuf_op.cpu_mask.bitmap, bytemap);
-    sysctl.u.tbuf_op.cpu_mask.nr_cpus = sizeof(bytemap) * 8;
+    sysctl.u.tbuf_op.cpu_mask.nr_elems = sizeof(bytemap) * 8;
 
      ret = do_sysctl(xch,&sysctl);
 

xen/arch/x86/platform_hypercall.c +2 -2

@@ -365,7 +365,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
      {
          uint32_t cpu;
          uint64_t idletime, now = NOW();
-        struct xenctl_cpumap ctlmap;
+        struct xenctl_map ctlmap;
          cpumask_var_t cpumap;
          XEN_GUEST_HANDLE(uint8) cpumap_bitmap;
          XEN_GUEST_HANDLE(uint64) idletimes;

@@ -378,7 +378,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
          if ( cpufreq_controller != FREQCTL_dom0_kernel )
              break;
 
-        ctlmap.nr_cpus  = op->u.getidletime.cpumap_nr_cpus;
+        ctlmap.nr_elems  = op->u.getidletime.cpumap_nr_cpus;
          guest_from_compat_handle(cpumap_bitmap,
                                   op->u.getidletime.cpumap_bitmap);
          ctlmap.bitmap.p = cpumap_bitmap.p; /* handle ->  handle_64 conversion */

xen/common/domctl.c +45 -22

@@ -31,28 +31,29 @@
  static DEFINE_SPINLOCK(domctl_lock);
  DEFINE_SPINLOCK(vcpu_alloc_lock);
 
-int cpumask_to_xenctl_cpumap(
-    struct xenctl_cpumap *xenctl_cpumap, const cpumask_t *cpumask)
+int bitmap_to_xenctl_map(struct xenctl_map *xenctl_map,
+                         const unsigned long *bitmap,
+                         unsigned int nbits)
  {
      unsigned int guest_bytes, copy_bytes, i;
      uint8_t zero = 0;
      int err = 0;
-    uint8_t *bytemap = xmalloc_array(uint8_t, (nr_cpu_ids + 7) / 8);
+    uint8_t *bytemap = xmalloc_array(uint8_t, (nbits + 7) / 8);
 
      if ( !bytemap )
          return -ENOMEM;
 
-    guest_bytes = (xenctl_cpumap->nr_cpus + 7) / 8;
-    copy_bytes  = min_t(unsigned int, guest_bytes, (nr_cpu_ids + 7) / 8);
+    guest_bytes = (xenctl_map->nr_elems + 7) / 8;
+    copy_bytes  = min_t(unsigned int, guest_bytes, (nbits + 7) / 8);
 
-    bitmap_long_to_byte(bytemap, cpumask_bits(cpumask), nr_cpu_ids);
+    bitmap_long_to_byte(bytemap, bitmap, nbits);
 
      if ( copy_bytes != 0 )
-        if ( copy_to_guest(xenctl_cpumap->bitmap, bytemap, copy_bytes) )
+        if ( copy_to_guest(xenctl_map->bitmap, bytemap, copy_bytes) )
              err = -EFAULT;
 
      for ( i = copy_bytes; !err&&  i<  guest_bytes; i++ )
-        if ( copy_to_guest_offset(xenctl_cpumap->bitmap, i,&zero, 1) )
+        if ( copy_to_guest_offset(xenctl_map->bitmap, i,&zero, 1) )
              err = -EFAULT;
 
      xfree(bytemap);

@@ -60,36 +61,58 @@ int cpumask_to_xenctl_cpumap(
      return err;
  }
 
-int xenctl_cpumap_to_cpumask(
-    cpumask_var_t *cpumask, const struct xenctl_cpumap *xenctl_cpumap)
+int xenctl_map_to_bitmap(unsigned long *bitmap,
+                         const struct xenctl_map *xenctl_map,
+                         unsigned int nbits)
  {
      unsigned int guest_bytes, copy_bytes;
      int err = 0;
-    uint8_t *bytemap = xzalloc_array(uint8_t, (nr_cpu_ids + 7) / 8);
+    uint8_t *bytemap = xzalloc_array(uint8_t, (nbits + 7) / 8);
 
      if ( !bytemap )
          return -ENOMEM;
 
-    guest_bytes = (xenctl_cpumap->nr_cpus + 7) / 8;
-    copy_bytes  = min_t(unsigned int, guest_bytes, (nr_cpu_ids + 7) / 8);
+    guest_bytes = (xenctl_map->nr_elems + 7) / 8;
+    copy_bytes  = min_t(unsigned int, guest_bytes, (nbits + 7) / 8);
 
      if ( copy_bytes != 0 )
      {
-        if ( copy_from_guest(bytemap, xenctl_cpumap->bitmap, copy_bytes) )
+        if ( copy_from_guest(bytemap, xenctl_map->bitmap, copy_bytes) )
              err = -EFAULT;
-        if ( (xenctl_cpumap->nr_cpus&  7)&&  (guest_bytes<= sizeof(bytemap)) )
-            bytemap[guest_bytes-1]&= ~(0xff<<  (xenctl_cpumap->nr_cpus&  7));
+        if ( (xenctl_map->nr_elems&  7)&&  (guest_bytes<= sizeof(bytemap)) )
+            bytemap[guest_bytes-1]&= ~(0xff<<  (xenctl_map->nr_elems&  7));
      }
 
-    if ( err )
-        /* nothing */;
-    else if ( alloc_cpumask_var(cpumask) )
-        bitmap_byte_to_long(cpumask_bits(*cpumask), bytemap, nr_cpu_ids);
+    if ( !err )
+        bitmap_byte_to_long(bitmap, bytemap, nbits);
+
+    xfree(bytemap);
+
+    return err;
+}
+
+int cpumask_to_xenctl_cpumap(struct xenctl_map *xenctl_cpumap,
+                             const cpumask_t *cpumask)
+{
+    return bitmap_to_xenctl_map(xenctl_cpumap, cpumask_bits(cpumask),
+                                nr_cpu_ids);
+}
+
+int xenctl_cpumap_to_cpumask(cpumask_var_t *cpumask,
+                             const struct xenctl_map *xenctl_cpumap)
+{
+    int err = 0;
+
+    if ( alloc_cpumask_var(cpumask) ) {
+        err = xenctl_map_to_bitmap(cpumask_bits(*cpumask), xenctl_cpumap,
+                                   nr_cpu_ids);
+        /* In case of error, cleanup is up to us, as the caller won't care! */
+        if ( err )
+            free_cpumask_var(*cpumask);
+    }
      else
          err = -ENOMEM;
 
-    xfree(bytemap);
-
      return err;
  }
 

xen/include/public/arch-x86/xen-mca.h +1 -1

@@ -414,7 +414,7 @@ struct xen_mc_mceinject {
 
  struct xen_mc_inject_v2 {
  	uint32_t flags;
-	struct xenctl_cpumap cpumap;
+	struct xenctl_map cpumap;
  };
  #endif
 

xen/include/public/domctl.h +1 -1

@@ -283,7 +283,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_getvc
  /* XEN_DOMCTL_getvcpuaffinity */
  struct xen_domctl_vcpuaffinity {
      uint32_t  vcpu;              /* IN */
-    struct xenctl_cpumap cpumap; /* IN/OUT */
+    struct xenctl_map cpumap;    /* IN/OUT */
  };
  typedef struct xen_domctl_vcpuaffinity xen_domctl_vcpuaffinity_t;
  DEFINE_XEN_GUEST_HANDLE(xen_domctl_vcpuaffinity_t);

xen/include/public/sysctl.h +3 -3

@@ -71,8 +71,8 @@ struct xen_sysctl_tbuf_op {
  #define XEN_SYSCTL_TBUFOP_disable      5
      uint32_t cmd;
      /* IN/OUT variables */
-    struct xenctl_cpumap cpu_mask;
-    uint32_t             evt_mask;
+    struct xenctl_map cpu_mask;
+    uint32_t          evt_mask;
      /* OUT variables */
      uint64_aligned_t buffer_mfn;
      uint32_t size;  /* Also an IN variable! */

@@ -531,7 +531,7 @@ struct xen_sysctl_cpupool_op {
      uint32_t domid;       /* IN: M              */
      uint32_t cpu;         /* IN: AR             */
      uint32_t n_dom;       /*            OUT: I  */
-    struct xenctl_cpumap cpumap; /*     OUT: IF */
+    struct xenctl_map cpumap;    /*     OUT: IF */
  };
  typedef struct xen_sysctl_cpupool_op xen_sysctl_cpupool_op_t;
  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpupool_op_t);

xen/include/public/xen.h +2 -2

@@ -822,9 +822,9 @@ typedef uint8_t xen_domain_handle_t[16];
  #endif
 
  #ifndef __ASSEMBLY__
-struct xenctl_cpumap {
+struct xenctl_map {
      XEN_GUEST_HANDLE_64(uint8) bitmap;
-    uint32_t nr_cpus;
+    uint32_t nr_elems;
  };
  #endif
 

xen/include/xen/cpumask.h +0 -0

  #endif /* __XEN_CPUMASK_H */

xen/include/xlat.lst +1 -1

@@ -2,7 +2,7 @@
  # ! - needs translation
  # ? - needs checking
  ?	dom0_vga_console_info		xen.h
-?	xenctl_cpumap			xen.h
+?	xenctl_map			xen.h
  ?	mmu_update			xen.h
  !	mmuext_op			xen.h
  !	start_info			xen.h

Quoted 4 lines · Dario Faggioli

In preparation for adding an xc_nodemap_t and its related hadling logic. Just add one indirection layer, and try to retain the interface as much as possible (although some small bits here and there have been affected).

This patch is fine with me on the whole (one comment below), but in this kind of a patch I think you need to include exactly what it is patch does. I.e.:

1. Replace xenctl_cpumap with xenctl_map
2. Implement bitmap_to_xenctl_map and the reverse
3. Re-implement cpumask_to_xenctl_map with bitmap_to_xenctl_map and vice versa.
4. Other than #3, no functional changes.

Quoted 6 lines

xen/include/xen/cpumask.h +0 -0

@@ -424,8 +424,8 @@ extern cpumask_t cpu_present_map;
  #define for_each_present_cpu(cpu)  for_each_cpu(cpu,&cpu_present_map)
 

xen/include/xen/cpumask.h extern cpumask_t cpu_present_map; · line 429

Quoted 7 lines

xen/include/xen/cpumask.h +3 -3

  /* Copy to/from cpumap provided by control tools. */
-struct xenctl_cpumap;
-int cpumask_to_xenctl_cpumap(struct xenctl_cpumap *, const cpumask_t *);
-int xenctl_cpumap_to_cpumask(cpumask_var_t *, const struct xenctl_cpumap *);
+struct xenctl_map;
+int cpumask_to_xenctl_cpumap(struct xenctl_map *, const cpumask_t *);
+int xenctl_cpumap_to_cpumask(cpumask_var_t *, const struct xenctl_map *);

You should probably s/cpumap/map/; in the function names as well.

Exactly as we have xc_cpumap_t, something similar for representing NUMA nodes (i.e., xc_nodemap_t and nodemap_t) could be useful. The same applies to libxl_cpumap, which on its turn now has its node-related counterpart, libxl_nodemap.

This is all in preparation for making it possible explicit specification of the node affinity of a guest.

Signed-off-by Dario Faggioli<[email protected]>

tools/libxc/xc_misc.c +21 -3

@@ -35,21 +35,49 @@ int xc_get_max_cpus(xc_interface *xch)
      return max_cpus;
  }
 
+int xc_get_max_nodes(xc_interface *xch)
+{
+    static int max_nodes = 0;
+    xc_physinfo_t physinfo;
+
+    if ( max_nodes )
+        return max_nodes;
+
+    if ( !xc_physinfo(xch,&physinfo) )
+        max_nodes = physinfo.max_node_id + 1;
+
+    return max_nodes;
+}
+
  int xc_get_cpumap_size(xc_interface *xch)
  {
      return (xc_get_max_cpus(xch) + 7) / 8;
  }
 
-xc_cpumap_t xc_cpumap_alloc(xc_interface *xch)
+int xc_get_nodemap_size(xc_interface *xch)
  {
-    int sz;
+    return (xc_get_max_nodes(xch) + 7) / 8;
+}
 
-    sz = xc_get_cpumap_size(xch);
+/* XXX See [*] below ... */
+static xc_cpumap_t __xc_map_alloc(xc_interface *xch, int sz)
+{
      if (sz == 0)
          return NULL;
      return calloc(1, sz);
  }
 
+xc_cpumap_t xc_cpumap_alloc(xc_interface *xch)

tools/libxc/xc_misc.c +0 -0

  int xc_readconsolering(xc_interface *xch,
                         char *buffer,
                         unsigned int *pnr_chars,

tools/libxc/xenctrl.h +14 -0

@@ -330,6 +330,20 @@ int xc_get_cpumap_size(xc_interface *xch
  xc_cpumap_t xc_cpumap_alloc(xc_interface *xch);
 
  /*
+ * NODEMAP handling
+ */
+typedef uint8_t *xc_nodemap_t;
+
+/* return maximum number of NUMA nodes the hypervisor supports */
+int xc_get_max_nodes(xc_interface *xch);
+
+/* return array size for nodemap */
+int xc_get_nodemap_size(xc_interface *xch);
+
+/* allocate a nodemap */
+xc_nodemap_t xc_nodemap_alloc(xc_interface *xch);
+
+/*
   * DOMAIN DEBUGGING FUNCTIONS
   */
 

tools/libxl/libxl_utils.c +16 -0

@@ -486,11 +486,27 @@ int libxl_cpumap_alloc(libxl_ctx *ctx, l
      return libxl_map_alloc(ctx, cpumap, max_cpus);
  }
 
+int libxl_nodemap_alloc(libxl_ctx *ctx, libxl_nodemap *nodemap)
+{
+    int max_nodes;
+
+    max_nodes = libxl_get_max_nodes(ctx);
+    if (max_nodes == 0)
+        return ERROR_FAIL;
+
+    return libxl_map_alloc(ctx, nodemap, max_nodes);
+}
+
  int libxl_get_max_cpus(libxl_ctx *ctx)
  {
      return xc_get_max_cpus(ctx->xch);
  }
 
+int libxl_get_max_nodes(libxl_ctx *ctx)
+{
+    return xc_get_max_nodes(ctx->xch);
+}
+
  int libxl__enum_from_string(const libxl_enum_string_table *t,
                              const char *s, int *e)
  {

tools/libxl/libxl_utils.h +29 -0

@@ -109,6 +109,35 @@ static inline int libxl_cpumap_cpu_valid
  #define libxl_for_each_set_cpu(v, m) for (v = 0; v<  (m).size * 8; v++) \
                                               if (libxl_cpumap_test(&(m), v))
 
+int libxl_nodemap_alloc(libxl_ctx *ctx, libxl_nodemap *nodemap);
+static inline int libxl_nodemap_test(libxl_nodemap *nodemap, int node)
+{
+    return libxl_map_test(nodemap, node);
+}
+static inline void libxl_nodemap_set(libxl_nodemap *nodemap, int node)
+{
+    libxl_map_set(nodemap, node);
+}
+static inline void libxl_nodemap_reset(libxl_nodemap *nodemap, int node)
+{
+    libxl_map_reset(nodemap, node);
+}
+static inline void libxl_nodemap_set_any(libxl_nodemap *nodemap)
+{
+    libxl_map_set_any(nodemap);
+}
+static inline void libxl_nodemap_set_none(libxl_nodemap *nodemap)
+{
+    libxl_map_set_none(nodemap);
+}
+static inline int libxl_nodemap_node_valid(libxl_nodemap *nodemap, int node)
+{
+    return libxl_map_elem_valid(nodemap, node);
+}
+#define libxl_for_each_node(var, map) for (var = 0; var<  (map).size * 8; var++)
+#define libxl_for_each_set_node(v, m) for (v = 0; v<  (m).size * 8; v++) \
+                                              if (libxl_nodemap_test(&(m), v))
+
  static inline uint32_t libxl__sizekb_to_mb(uint32_t s) {
      return (s + 1023) / 1024;
  }

xen/common/domctl.c +8 -0

@@ -116,6 +116,14 @@ int xenctl_cpumap_to_cpumask(cpumask_var
      return err;
  }
 
+int xenctl_nodemap_to_nodemask(nodemask_t *nodemask,
+                               const struct xenctl_map *xenctl_nodemap)
+{
+    return xenctl_map_to_bitmap(nodes_addr(*nodemask),
+                                xenctl_nodemap,
+                                MAX_NUMNODES);
+}
+
  static inline int is_free_domid(domid_t dom)
  {
      struct domain *d;

Quoted 9 lines

xen/include/xen/cpumask.h extern cpumask_t cpu_present_map; · line 429

Quoted 7 lines

/* Copy to/from cpumap provided by control tools. */

xen/include/xen/cpumask.h +3 -3

-struct xenctl_cpumap;
-int cpumask_to_xenctl_cpumap(struct xenctl_cpumap *, const cpumask_t *);
-int xenctl_cpumap_to_cpumask(cpumask_var_t *, const struct xenctl_cpumap *);
+struct xenctl_map;
+int cpumask_to_xenctl_cpumap(struct xenctl_map *, const cpumask_t *);
+int xenctl_cpumap_to_cpumask(cpumask_var_t *, const struct xenctl_map *);

You should probably s/cpumap/map/; in the function names as well.

Well, I retained the old name as the callers expect to use these functions as that old name says, i.e., passing a cpumap and wanting a cpumask back (for the first one).

Quoted 9 lines

tools/libxc/xc_misc.c +3 -1

-    sz = xc_get_cpumap_size(xch);
+/* XXX See [*] below ... */
+static xc_cpumap_t __xc_map_alloc(xc_interface *xch, int sz)
+{
      if (sz == 0)
          return NULL;
      return calloc(1, sz);
  }

tools/libxc/xc_misc.c int xc_get_max_cpus(xc_interface *xch) · line 80

Quoted 11 lines

tools/libxc/xc_misc.c +11 -0

+xc_cpumap_t xc_cpumap_alloc(xc_interface *xch)
+{
+    return __xc_map_alloc(xch, xc_get_cpumap_size(xch));
+}
+
+xc_nodemap_t xc_nodemap_alloc(xc_interface *xch)
+{
+    /* XXX [*] How bad is this? Ideas? */
+    return (xc_nodemap_t) __xc_map_alloc(xch, xc_get_nodemap_size(xch));
+}
+

I don't think it's incredibly terrible if it would buy us something; but I don't really see that it does.

Two functions would be a lot more
readable, IMHO.

Other than that, looks fine.

Add some rationale and usage documentation for the new automatic NUMA placement feature of xl.

TODO: * Decide whether we want to have things like "Future Steps/Roadmap"
and/or "Performances/Benchmarks Results" here as well.

docs/misc/xl-numa-placement.txt new file +17 -0

@@ -0,0 +1,205 @@
+               -------------------------------------
+               NUMA Guest Placement Design and Usage
+               -------------------------------------
+
+Xen deals with Non-Uniform Memory Access (NUMA) machines in many ways. For
+example each domain has its own "node affinity", i.e., a set of NUMA nodes
+of the host from which memory for that domain is allocated. That becomes
+very important as soon as many domains start running memory-intensive
+workloads on a shared host. In fact, accessing non node-local memory
+locations costs much more than node-local ones, to the point that the
+degradation in performance is likely to be noticeable.
+
+It is then quite obvious that, any mechanism that enable the most of the
+memory accesses for the most of the most of the guest domains to stay
+local is something very important to achieve when dealing with NUMA
+platforms.
+

docs/misc/xl-numa-placement.txt new file +38 -0

+what nodes these CPUs belongs to. Actually, that is exactly what is being
+done by the hypervisor by default, as soon as it finds out a domain (or
+better, the vcpus of a domain, but let's avoid getting into too much
+details here) has a cpu affinity.
+
+This is working quite well, but it requires the user/system administrator
+to explicitly specify such property --- the cpu affinity --- while the
+domain is being created, or Xen won't be able to exploit that for ensuring
+accesses locality.
+
+On the other hand, as node affinity directly affects where domain's memory
+lives, it makes a lot of sense for it to be involved in scheduling decisions,
+as it would be great if the hypervisor would manage in scheduling all the
+vcpus of all the domains on CPUs attached to the various domains' local
+memory. That is why, the node affinity of a domain is treated by the scheduler
+as the set of nodes on which it would be preferable to run it, although
+not at the cost of violating the scheduling algorithm behavior and
+invariants. This means it Xen will check whether a vcpu of a domain can run
+on one of the CPUs belonging to the nodes of the domain's node affinity,
+but will better run it somewhere else --- even on another, remote, CPU ---
+than violating the priority ordering (e.g., by kicking out from there another
+running vcpu with higher priority) it is designed to enforce.
+
+So, last but not least, what if a domain has both vcpu and node affinity, and
+they only partially match or they do not match at all (to understand how that
+can happen, see the following sections)? Well, in such case, all the domain
+memory will be allocated reflecting its node affinity, while scheduling will
+happen according to its vcpu affinities, meaning that it is easy enough to
+construct optimal, sub-optimal, neutral and even bad and awful configurations
+(which is something nice, e.g., for benchmarking purposes). The remainder
+part of this document is explaining how to do so.
+
+
+Specifying Node Affinity
+------------------------
+
+Besides being automatically computed from the vcpu affinities of a domain
+(or also from it being part of a cpupool) within Xen, it might make sense

docs/misc/xl-numa-placement.txt new file +20 -0

+related, and if only one is present, it makes sense to use the other for
+inferring it, but it is always possible to explicitly specify both of them,
+independently on how good or awful it could end up being.
+
+Therefore, this is what one should expect when using "nodes", perhaps in
+conjunction with "cpus" in a domain configuration file:
+
+ * `cpus = "0, 1"` and no `nodes=` at all
+   (i.e., only vcpu affinity specified):
+     domain's vcpus can and will run only on host CPUs 0 and 1. Also, as
+     domain's node affinity will be computed by Xen and set to whatever
+     nodes host CPUs 0 and 1 belongs, all the domain's memory accesses will
+     be local accesses;
+
+ * `nodes = [ '0', '1' ]` and no `cpus=` at all
+   (i.e., only node affinity present):
+     domain's vcpus can run on any of the host CPUs, but the scheduler (at
+     least if credit is used, as it is the only scheduler supporting this
+     right now) will try running them on the CPUs that are part of host
+     NUMA nodes 0 and 1. Memory-wise, all the domain's memory will be

docs/misc/xl-numa-placement.txt new file +10 -0

+   (i.e., cpu affinity disjointed with node affinity):
+     domain's vcpus can and will run only on host CPU 4, i.e., completely
+     "outside" of the chosen node affinity. That necessarily means all the
+     domain's memory access will be remote.
+
+
+Automatic NUMA Placement
+------------------------
+
+Just in case one does not want to take the burden of manually specifying

docs/misc/xl-numa-placement.txt new file +39 -0

+
+ * "auto": automatic placement by means of a not better specified (xl
+           implementation dependant) algorithm. It is basically for those
+           who do want automatic placement, but have no idea what policy
+           or algorithm would be better... <<Just give me a sane default!>>
+
+ * "ffit": automatic placement via the First Fit algorithm, applied checking
+           the memory requirement of the domain against the amount of free
+           memory in the various host NUMA nodes;
+
+ * "bfit": automatic placement via the Best Fit algorithm, applied checking
+           the memory requirement of the domain against the amount of free
+           memory in the various host NUMA nodes;
+
+ * "wfit": automatic placement via the Worst Fit algorithm, applied checking
+           the memory requirement of the domain against the amount of free
+           memory in the various host NUMA nodes;
+
+The various algorithms have been implemented as they offer different behavior
+and performances (for different performance metrics). For instance, First Fit
+is known to be efficient and quick, and it generally works better than Best
+Fit wrt memory fragmentation, although it tends to occupy "early" nodes more
+than "late" ones. On the other hand, Best Fit aims at optimizing memory usage,
+although it introduces quite a bit of fragmentation, by leaving large amounts
+of small free memory areas. Finally, the idea behind Worst Fit is that it will
+leave big enough free memory chunks to limit the amount of fragmentation, but
+it (as well as Best Fit does) is more expensive in terms of execution time, as
+it needs the "list" of free memory areas to be kept sorted.
+
+Therefore, achieving automatic placement actually happens by properly using
+the "nodes" and "nodes_config" configuration options as follows:
+
+ * `nodes="auto` or `nodes_policy="auto"`:
+     xl will try fitting the domain on the host NUMA nodes by using its
+     own default placing algorithm, with default parameters. Most likely,
+     all nodes will be considered suitable for the domain (unless a vcpu
+     affinity is specified, see the last entry of this list;
+
+ * `nodes_policy="ffit"` (or `"bfit"`, `"wfit"`) and no `nodes=` at all:

docs/misc/xl-numa-placement.txt new file +14 -0

 All the nodes will be considered suitable for
+     the domain, and consecutive attempts will be performed while
+     increasing such a value;
+
+ * `nodes_policy="auto"` (or `"ffit"`, `"bfit"`, `"wfit"`) and `cpus="0-6":
+     xl will try fitting the domain on the host NUMA nodes to which the CPUs
+     specified as vcpu affinity (0 to 6 in this example) belong, by using the
+     requested policy. In case it fails, consecutive fitting attempts will
+     be performed with both a reduced (first) and an increased (next) number
+     of nodes).
+
+Different usage patterns --- like specifying both a policy and a list of nodes
+are accepted, but does not make much sense after all. Therefore, although xl
+will try at its best to interpret user's will, the resulting behavior is
+somehow unspecified.

Let the user specify the NUMA node affinity of a guest via the 'nodes=' config switch. Explicitly listing the intended target host nodes is required as of now.

A valid setting will directly impact the node_affinity mask of the guest, i.e., it will change the behaviour of the low level memory allocator. On the other hand, this commit does not affect by any means how the guest's vCPUs are scheduled on the host's pCPUs.

TODO: * Better investigate and test interactions with cpupools.

Signed-off-by Dario Faggioli<[email protected]>

docs/man/xl.cfg.pod.5 +0 -0

@@ -112,6 +112,15 @@ List of which cpus the guest is allowed
  (all vcpus will run on cpus 0,2,3,5), or `cpus=["2", "3"]` (all vcpus
  will run on cpus 2 and 3).

docs/man/xl.cfg.pod.5 +0 -0

  =item B<memory=MBYTES>
 
  Start the guest with MBYTES megabytes of RAM.

tools/libxc/xc_domain.c +38 -0

@@ -110,6 +110,44 @@ int xc_domain_shutdown(xc_interface *xch
  }
 
 
+int xc_domain_node_affinity(xc_interface *xch,
+                            uint32_t domid,
+                            xc_nodemap_t node_affinity)
+{
+    DECLARE_DOMCTL;
+    DECLARE_HYPERCALL_BUFFER(uint8_t, local);
+    int ret = -1;
+    int nodesize;
+
+    nodesize = xc_get_nodemap_size(xch);
+    if (!nodesize)
+    {
+        PERROR("Could not get number of nodes");
+        goto out;
+    }
+
+    local = xc_hypercall_buffer_alloc(xch, local, nodesize);
+    if ( local == NULL )
+    {
+        PERROR("Could not allocate memory for domain_node_affinity domctl hypercall");
+        goto out;
+    }
+
+    domctl.cmd = XEN_DOMCTL_node_affinity;
+    domctl.domain = (domid_t)domid;
+
+    memcpy(local, node_affinity, nodesize);
+    set_xen_guest_handle(domctl.u.node_affinity.nodemap.bitmap, local);
+    domctl.u.node_affinity.nodemap.nr_elems = nodesize * 8;
+
+    ret = do_domctl(xch,&domctl);
+
+    xc_hypercall_buffer_free(xch, local);
+
+ out:
+    return ret;
+}
+
  int xc_vcpu_setaffinity(xc_interface *xch,
                          uint32_t domid,
                          int vcpu,

tools/libxc/xenctrl.h +3 -0

@@ -520,6 +520,19 @@ int xc_watchdog(xc_interface *xch,
                 uint32_t id,
                 uint32_t timeout);
 
+/**
+ * This function explicitly sets the affinity of a domain with the
+ * host NUMA nodes.

tools/libxc/xenctrl.h +0 -0

  int xc_vcpu_setaffinity(xc_interface *xch,
                          uint32_t domid,
                          int vcpu,

tools/libxl/gentest.py +14 -1

@@ -20,7 +20,7 @@ def randomize_case(s):
  def randomize_enum(e):
      return random.choice([v.name for v in e.values])
 
-handcoded = ["libxl_cpumap", "libxl_key_value_list",
+handcoded = ["libxl_cpumap", "libxl_nodemap", "libxl_key_value_list",
               "libxl_cpuid_policy_list", "libxl_file_reference",
               "libxl_string_list"]
 

@@ -119,6 +119,19 @@ static void libxl_cpumap_rand_init(libxl
      }
  }
 
+static void libxl_nodemap_rand_init(libxl_nodemap *nodemap)
+{
+    int i;
+    nodemap->size = rand() % 16;
+    nodemap->map = calloc(nodemap->size, sizeof(*nodemap->map));
+    libxl_for_each_node(i, *nodemap) {
+        if (rand() % 2)
+            libxl_nodemap_set(nodemap, i);
+        else
+            libxl_nodemap_reset(nodemap, i);
+    }
+}
+
  static void libxl_key_value_list_rand_init(libxl_key_value_list *pkvl)
  {
      int i, nr_kvp = rand() % 16;

tools/libxl/libxl.c +10 -0

@@ -3082,6 +3082,16 @@ int libxl_set_vcpuaffinity_all(libxl_ctx
      return rc;
  }
 
+int libxl_set_node_affinity(libxl_ctx *ctx, uint32_t domid,
+                            libxl_nodemap *nodemap)
+{
+    if (xc_domain_node_affinity(ctx->xch, domid, nodemap->map)) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting node affinity");
+        return ERROR_FAIL;
+    }
+    return 0;
+}
+
  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_cpumap *cpumap)
  {
      GC_INIT(ctx);

tools/libxl/libxl.h +0 -0

  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_cpumap *cpumap);
 
  libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx);

tools/libxl/libxl_create.c +6 -0

@@ -121,6 +121,12 @@ int libxl__domain_build_info_setdefault(
          libxl_cpumap_set_any(&b_info->cpumap);
      }
 
+    if (!b_info->nodemap.size) {
+        if (libxl_nodemap_alloc(CTX,&b_info->nodemap))
+            return ERROR_NOMEM;
+        libxl_nodemap_set_none(&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)

tools/libxl/libxl_dom.c +1 -0

@@ -67,6 +67,7 @@ int libxl__build_pre(libxl__gc *gc, uint
      char *xs_domid, *con_domid;
      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
      libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus,&info->cpumap);
+    libxl_set_node_affinity(ctx, domid,&info->nodemap);
      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
      if (info->type == LIBXL_DOMAIN_TYPE_PV)
          xc_domain_set_memmap_limit(ctx->xch, domid,

tools/libxl/libxl_json.c +20 -0

@@ -119,6 +119,26 @@ out:
      return s;
  }
 
+yajl_gen_status libxl_nodemap_gen_json(yajl_gen hand,
+                                       libxl_nodemap *nodemap)
+{
+    yajl_gen_status s;
+    int i;
+
+    s = yajl_gen_array_open(hand);
+    if (s != yajl_gen_status_ok) goto out;
+
+    libxl_for_each_node(i, *nodemap) {
+        if (libxl_nodemap_test(nodemap, i)) {
+            s = yajl_gen_integer(hand, i);
+            if (s != yajl_gen_status_ok) goto out;
+        }
+    }
+    s = yajl_gen_array_close(hand);
+out:
+    return s;
+}
+
  yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
                                                libxl_key_value_list *pkvl)
  {

tools/libxl/libxl_json.h +1 -0

@@ -27,6 +27,7 @@ yajl_gen_status libxl_domid_gen_json(yaj
  yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
  yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
  yajl_gen_status libxl_cpumap_gen_json(yajl_gen hand, libxl_cpumap *p);
+yajl_gen_status libxl_nodemap_gen_json(yajl_gen hand, libxl_nodemap *p);
  yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
                                                   libxl_cpuid_policy_list *p);
  yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);

tools/libxl/libxl_types.idl +2 -0

@@ -11,6 +11,7 @@ libxl_domid = Builtin("domid", json_fn =
  libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
  libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
  libxl_cpumap = Builtin("cpumap", dispose_fn="libxl_cpumap_dispose", passby=PASS_BY_REFERENCE)
+libxl_nodemap = Builtin("nodemap", dispose_fn="libxl_nodemap_dispose", passby=PASS_BY_REFERENCE)
  libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE)
 
  libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)

@@ -233,6 +234,7 @@ libxl_domain_build_info = Struct("domain
      ("max_vcpus",       integer),
      ("cur_vcpus",       integer),
      ("cpumap",          libxl_cpumap),
+    ("nodemap",         libxl_nodemap),
      ("tsc_mode",        libxl_tsc_mode),
      ("max_memkb",       MemKB),
      ("target_memkb",    MemKB),

tools/libxl/xl_cmdimpl.c +22 -1

@@ -515,7 +515,7 @@ static void parse_config_data(const char
      const char *buf;
      long l;
      XLU_Config *config;
-    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
+    XLU_ConfigList *cpus, *nodes, *vbds, *nics, *pcis, *cvfbs, *cpuids;
      int pci_power_mgmt = 0;
      int pci_msitranslate = 1;
      int pci_permissive = 0;

@@ -628,6 +628,39 @@ static void parse_config_data(const char
          free(buf2);
      }
 
+    if (!xlu_cfg_get_list (config, "nodes",&nodes, 0, 0)) {
+        int i, n_nodes = 0;
+
+        if (libxl_nodemap_alloc(ctx,&b_info->nodemap)) {
+            fprintf(stderr, "Unable to allocate nodemap\n");
+            exit(1);
+        }
+
+        libxl_nodemap_set_none(&b_info->nodemap);
+        while ((buf = xlu_cfg_get_listitem(nodes, n_nodes)) != NULL) {
+            i = atoi(buf);
+            if (!libxl_nodemap_node_valid(&b_info->nodemap, i)) {
+                fprintf(stderr, "node %d illegal\n", i);
+                exit(1);
+            }
+            libxl_nodemap_set(&b_info->nodemap, i);
+            n_nodes++;
+        }
+
+        if (n_nodes == 0)
+            fprintf(stderr, "No valid node found: no affinity will be set\n");

tools/libxl/xl_cmdimpl.c +2 -0

+    }
+
      if (!xlu_cfg_get_long (config, "memory",&l, 0)) {
          b_info->max_memkb = l * 1024;
          b_info->target_memkb = b_info->max_memkb;

tools/python/xen/lowlevel/xl/xl.c +12 -0

@@ -243,6 +243,18 @@ int attrib__libxl_cpumap_set(PyObject *v
      return 0;
  }
 
+int attrib__libxl_nodemap_set(PyObject *v, libxl_nodemap *pptr)
+{
+    int i;
+    long node;
+
+    for (i = 0; i<  PyList_Size(v); i++) {
+        node = PyInt_AsLong(PyList_GetItem(v, i));
+        libxl_nodemap_set(pptr, node);
+    }
+    return 0;
+}
+
  int attrib__libxl_file_reference_set(PyObject *v, libxl_file_reference *pptr)
  {
      return genwrap__string_set(v,&pptr->path);

xen/common/domain.c +44 -3

@@ -222,6 +222,7 @@ struct domain *domain_create(
 
      spin_lock_init(&d->node_affinity_lock);
      d->node_affinity = NODE_MASK_ALL;
+    d->has_node_affinity = 0;
 
      spin_lock_init(&d->shutdown_lock);
      d->shutdown_code = -1;

@@ -337,7 +338,7 @@ void domain_update_node_affinity(struct
  {
      cpumask_var_t cpumask;
      cpumask_var_t online_affinity;
-    const cpumask_t *online;
+    const cpumask_t *online = cpupool_online_cpumask(d->cpupool);
      nodemask_t nodemask = NODE_MASK_NONE;
      struct vcpu *v;
      unsigned int node;

@@ -350,9 +351,22 @@ void domain_update_node_affinity(struct
          return;
      }
 
-    online = cpupool_online_cpumask(d->cpupool);
+    spin_lock(&d->node_affinity_lock);
 
-    spin_lock(&d->node_affinity_lock);
+    /*
+     * If someone explicitly told us our NUMA affinity, avoid messing
+     * that up. Notice, however, that vcpu affinity is still what
+     * drives all scheduling decisions.
+     *
+     * XXX I'm quite sure this is fine wrt to the various v->cpu_affinity
+     *     (at least, that was the intended design!). Could it be useful
+     *     to cross-check d->node_affinity against `online`? The basic
+     *     idea here is "Hey, if you shoot yourself in the foot... You've
+     *     shot in the foot!", but, you know...
+     */
+    if ( d->has_node_affinity )
+        goto out;
+
 
      for_each_vcpu ( d, v )
      {

@@ -365,6 +379,8 @@ void domain_update_node_affinity(struct
              node_set(node, nodemask);
 
      d->node_affinity = nodemask;
+
+out:
      spin_unlock(&d->node_affinity_lock);
 
      free_cpumask_var(online_affinity);

@@ -372,6 +388,31 @@ void domain_update_node_affinity(struct
  }
 
 
+int domain_set_node_affinity(struct domain *d, const nodemask_t *affinity)
+{
+    spin_lock(&d->node_affinity_lock);
+
+    /*
+     * Being/becoming affine to _no_ nodes is not going to work,
+     * let's take it as the `reset node affinity` command.
+     */
+    if ( nodes_empty(*affinity) )
+    {
+        d->has_node_affinity = 0;
+        spin_unlock(&d->node_affinity_lock);
+        domain_update_node_affinity(d);
+        return 0;
+    }
+
+    d->has_node_affinity = 1;
+    d->node_affinity = *affinity;
+
+    spin_unlock(&d->node_affinity_lock);
+
+    return 0;
+}
+
+
  struct domain *get_domain_by_id(domid_t dom)
  {
      struct domain *d;

xen/common/domctl.c +1 -0

@@ -621,6 +621,27 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
      }
      break;
 
+    case XEN_DOMCTL_node_affinity:

xen/common/domctl.c +10 -0

+
+        ret = xenctl_nodemap_to_nodemask(&new_affinity,
+&op->u.node_affinity.nodemap);
+        if ( !ret )
+            ret = domain_set_node_affinity(d,&new_affinity);
+
+        rcu_unlock_domain(d);
+    }
+    break;
+
      case XEN_DOMCTL_setvcpuaffinity:
      case XEN_DOMCTL_getvcpuaffinity:
      {

xen/common/keyhandler.c +11 -0

@@ -217,6 +217,14 @@ static void cpuset_print(char *set, int
      *set++ = '\0';
  }
 
+static void nodeset_print(char *set, int size, const nodemask_t *mask)
+{
+    *set++ = '[';
+    set += nodelist_scnprintf(set, size-2, mask);
+    *set++ = ']';
+    *set++ = '\0';
+}
+
  static void periodic_timer_print(char *str, int size, uint64_t period)
  {
      if ( period == 0 )

@@ -272,6 +280,9 @@ static void dump_domains(unsigned char k
 
          dump_pageframe_info(d);
 
+        nodeset_print(tmpstr, sizeof(tmpstr),&d->node_affinity);
+        printk("NODE affinity for domain %d: %s\n", d->domain_id, tmpstr);
+
          printk("VCPU information and callbacks for domain %u:\n",
                 d->domain_id);
          for_each_vcpu ( d, v )

xen/include/public/domctl.h +11 -0

@@ -278,6 +278,15 @@ typedef struct xen_domctl_getvcpuinfo xe
  DEFINE_XEN_GUEST_HANDLE(xen_domctl_getvcpuinfo_t);
 
 
+/* Set the NUMA node(s) with which the guest is `affine`. */
+/* XEN_DOMCTL_node_affinity */
+struct xen_domctl_node_affinity {
+    struct xenctl_map nodemap;   /* IN */
+};
+typedef struct xen_domctl_node_affinity xen_domctl_node_affinity_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_node_affinity_t);
+
+
  /* Get/set which physical cpus a vcpu can execute on. */
  /* XEN_DOMCTL_setvcpuaffinity */
  /* XEN_DOMCTL_getvcpuaffinity */

@@ -914,6 +923,7 @@ struct xen_domctl {
  #define XEN_DOMCTL_set_access_required           64
  #define XEN_DOMCTL_audit_p2m                     65
  #define XEN_DOMCTL_set_virq_handler              66
+#define XEN_DOMCTL_node_affinity                 67
  #define XEN_DOMCTL_gdbsx_guestmemio            1000
  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002

@@ -927,6 +937,7 @@ struct xen_domctl {
          struct xen_domctl_getpageframeinfo  getpageframeinfo;
          struct xen_domctl_getpageframeinfo2 getpageframeinfo2;
          struct xen_domctl_getpageframeinfo3 getpageframeinfo3;
+        struct xen_domctl_node_affinity     node_affinity;
          struct xen_domctl_vcpuaffinity      vcpuaffinity;
          struct xen_domctl_shadow_op         shadow_op;
          struct xen_domctl_max_mem           max_mem;

xen/include/xen/nodemask.h +12 -2

@@ -8,8 +8,9 @@
   * See detailed comments in the file linux/bitmap.h describing the
   * data type on which these nodemasks are based.
   *
- * For details of nodemask_scnprintf() and nodemask_parse(),
- * see bitmap_scnprintf() and bitmap_parse() in lib/bitmap.c.
+ * For details of nodemask_scnprintf(), nodelist_scnpintf() and
+ * nodemask_parse(), see bitmap_scnprintf() and bitmap_parse()
+ * in lib/bitmap.c.
   *
   * The available nodemask operations are:
   *

@@ -48,6 +49,7 @@
   * unsigned long *nodes_addr(mask)     Array of unsigned long's in mask
   *
   * int nodemask_scnprintf(buf, len, mask) Format nodemask for printing
+ * int nodelist_scnprintf(buf, len, mask) Format nodemask as a list for printing
   * int nodemask_parse(ubuf, ulen, mask)        Parse ascii string as nodemask
   *
   * for_each_node_mask(node, mask)      for-loop node over mask

@@ -280,6 +282,14 @@ static inline int __first_unset_node(con
 
  #define nodes_addr(src) ((src).bits)
 
+#define nodelist_scnprintf(buf, len, src) \
+                       __nodelist_scnprintf((buf), (len), (src), MAX_NUMNODES)
+static inline int __nodelist_scnprintf(char *buf, int len,
+                                       const nodemask_t *srcp, int nbits)
+{
+       return bitmap_scnlistprintf(buf, len, srcp->bits, nbits);
+}
+
  #if 0
  #define nodemask_scnprintf(buf, len, src) \
                         __nodemask_scnprintf((buf), (len),&(src), MAX_NUMNODES)

xen/include/xen/sched.h +1 -0

      unsigned int last_alloc_node;
      spinlock_t node_affinity_lock;
  };

@@ -416,6 +420,7 @@ static inline void get_knownalive_domain
      ASSERT(!(atomic_read(&d->refcnt)&  DOMAIN_DESTROYED));
  }
 
+int domain_set_node_affinity(struct domain *d, const nodemask_t *affinity);
  void domain_update_node_affinity(struct domain *d);
 
  struct domain *domain_create(

For feature parity with vcpu affinity, allow for specifying node affinity not only at domain creation time, but at run-time too.

Of course this is not going to be equally effective, as it will only affect future memory allocations without touching what's already there. However, in future we might want to change this, and use this as an interface for sort-of manual "domain node migration".

Signed-off-by Dario Faggioli<[email protected]>

docs/man/xl.pod.1 +20 -0

@@ -556,6 +556,26 @@ different run state is appropriate.  Pin
  this, by ensuring certain VCPUs can only run on certain physical
  CPUs.
 
+=item B<node-affinity>  I<domain-id>  I<nodes>
+
+Set the NUMA node affinity for the domain, i.e., the set of NUMA
+nodes of the host from which the memory of the domain will be
+allocated. More specificallly, the domain's memory will be split
+in equal (well, as equal as possible) parts among all the nodes
+it is affine with, The keyword B<all>  can be used to have the
+domain affine to all NUMA nodes in the host.
+
+Normally NUMA node affinity of a domain is automatically computed
+from its VCPU affinity. The default behaviour is to have it equal
+to all the nodes the PCPUs onto which the VCPUs of the domain are
+pinned belong to. Manually specifying it can be used to restrict
+this to a specific subset of the host NUMA nodes, for improved
+locality of memory accesses by the domain. Notice, however, that
+this will not affect the memory that has already been allocated.
+For having the full amount of memory allocated on specific node(s)
+at domain creation time, the domain's configuration file is what
+should be used.
+
  =item B<vncviewer>  [I<OPTIONS>] I<domain-id>
 
  Attach to domain's VNC server, forking a vncviewer process.

tools/libxl/libxl_utils.c +1 -1

@@ -442,7 +442,7 @@ void libxl_map_dispose(struct libxl_map
      free(map->map);
  }
 
-static int libxl_map_alloc(libxl_ctx *ctx, struct libxl_map *map, int n_elems)
+int libxl_map_alloc(libxl_ctx *ctx, struct libxl_map *map, int n_elems)
  {
      int sz;
 

tools/libxl/libxl_utils.h +5 -0

@@ -64,6 +64,7 @@ int libxl_devid_to_device_nic(libxl_ctx
  int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid, const char *vdev,
                                 libxl_device_disk *disk);
 
+int libxl_map_alloc(libxl_ctx *ctx, struct libxl_map *map, int n_elems);
  int libxl_map_test(struct libxl_map *map, int elem);
  void libxl_map_set(struct libxl_map *map, int elem);
  void libxl_map_reset(struct libxl_map *map, int elem);

@@ -79,6 +80,10 @@ static inline int libxl_map_elem_valid(s
  {
      return elem>= 0&&  elem<  (map->size * 8);
  }
+#define libxl_for_each_elem(v, m) for (v = 0; v<  (m).size * 8; v++)
+#define libxl_for_each_set_elem(v, m) for (v = 0; v<  (m).size * 8; v++) \
+                                              if (libxl_map_test(&(m), v))
+
 
  int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap);
  static inline int libxl_cpumap_test(libxl_cpumap *cpumap, int cpu)

tools/libxl/xl.h +1 -0

@@ -54,6 +54,7 @@ int main_config_update(int argc, char **
  int main_button_press(int argc, char **argv);
  int main_vcpupin(int argc, char **argv);
  int main_vcpuset(int argc, char **argv);
+int main_nodeaffinity(int argc, char **argv);
  int main_memmax(int argc, char **argv);
  int main_memset(int argc, char **argv);
  int main_sched_credit(int argc, char **argv);

tools/libxl/xl_cmdimpl.c +71 -27

@@ -448,65 +448,75 @@ static void split_string_into_string_lis
      free(s);
  }
 
-static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap)
+static int affinity_parse(char *str, struct libxl_map *map, int n_elems)
  {
-    libxl_cpumap exclude_cpumap;
-    uint32_t cpuida, cpuidb;
+    struct libxl_map exclude_map;
+    uint32_t stra, strb;
      char *endptr, *toka, *tokb, *saveptr = NULL;
-    int i, rc = 0, rmcpu;
-
-    if (!strcmp(cpu, "all")) {
-        libxl_cpumap_set_any(cpumap);
+    int i, rc = 0, rmelem;
+
+    if (!strcmp(str, "all")) {
+        libxl_map_set_any(map);
          return 0;
      }
 
-    if (libxl_cpumap_alloc(ctx,&exclude_cpumap)) {
-        fprintf(stderr, "Error: Failed to allocate cpumap.\n");
+    if (libxl_map_alloc(ctx,&exclude_map, n_elems)) {
+        fprintf(stderr, "Error: Failed to allocate libxl_map.\n");
          return ENOMEM;
      }
 
-    for (toka = strtok_r(cpu, ",",&saveptr); toka;
+    for (toka = strtok_r(str, ",",&saveptr); toka;
           toka = strtok_r(NULL, ",",&saveptr)) {
-        rmcpu = 0;
+        rmelem = 0;
          if (*toka == '^') {
              /* This (These) Cpu(s) will be removed from the map */
              toka++;
-            rmcpu = 1;
+            rmelem = 1;
          }
          /* Extract a valid (range of) cpu(s) */
-        cpuida = cpuidb = strtoul(toka,&endptr, 10);
+        stra = strb = strtoul(toka,&endptr, 10);
          if (endptr == toka) {
              fprintf(stderr, "Error: Invalid argument.\n");
              rc = EINVAL;
-            goto vcpp_out;
+            goto afp_out;
          }
          if (*endptr == '-') {
              tokb = endptr + 1;
-            cpuidb = strtoul(tokb,&endptr, 10);
-            if (endptr == tokb || cpuida>  cpuidb) {
+            strb = strtoul(tokb,&endptr, 10);
+            if (endptr == tokb || stra>  strb) {
                  fprintf(stderr, "Error: Invalid argument.\n");
                  rc = EINVAL;
-                goto vcpp_out;
+                goto afp_out;
              }
          }
-        while (cpuida<= cpuidb) {
-            rmcpu == 0 ? libxl_cpumap_set(cpumap, cpuida) :
-                         libxl_cpumap_set(&exclude_cpumap, cpuida);
-            cpuida++;
+        while (stra<= strb) {
+            rmelem == 0 ? libxl_map_set(map, stra) :
+                          libxl_map_set(&exclude_map, stra);
+            stra++;
          }
      }
 
      /* Clear all the cpus from the removal list */
-    libxl_for_each_set_cpu(i, exclude_cpumap) {
-        libxl_cpumap_reset(cpumap, i);
-    }
-
-vcpp_out:
-    libxl_cpumap_dispose(&exclude_cpumap);
+    libxl_for_each_set_elem(i, exclude_map) {
+        libxl_map_reset(map, i);
+    }
+
+afp_out:
+    libxl_map_dispose(&exclude_map);
 
      return rc;
  }
 
+static inline int vcpupin_parse(char *cpu, libxl_cpumap *cpumap)
+{
+    return affinity_parse(cpu, cpumap, libxl_get_max_cpus(ctx));
+}
+
+static inline int nodeaffinity_parse(char *nodes, libxl_nodemap *nodemap)
+{
+    return affinity_parse(nodes, nodemap, libxl_get_max_nodes(ctx));
+}
+
  static void parse_config_data(const char *configfile_filename_report,
                                const char *configfile_data,
                                int configfile_len,

@@ -3873,6 +3883,40 @@ int main_vcpuset(int argc, char **argv)
      return 0;
  }
 
+static void nodeaffinity(const char *d, char *nodes)
+{
+    libxl_nodemap nodemap;
+
+    find_domain(d);
+
+    if (libxl_nodemap_alloc(ctx,&nodemap))
+        goto nodeaffinity_out;
+
+    if (!strcmp(nodes, "all"))
+        libxl_nodemap_set_any(&nodemap);
+    else if (nodeaffinity_parse(nodes,&nodemap))
+        goto nodeaffinity_out1;
+
+    if (libxl_set_node_affinity(ctx, domid,&nodemap) == -1)
+        fprintf(stderr, "Could not set node affinity for dom `%d'.\n", domid);
+
+  nodeaffinity_out1:
+    libxl_nodemap_dispose(&nodemap);
+  nodeaffinity_out:
+    ;
+}
+
+int main_nodeaffinity(int argc, char **argv)
+{
+    int opt;
+
+    if ((opt = def_getopt(argc, argv, "", "node-affinity", 2)) != -1)
+        return opt;
+
+    nodeaffinity(argv[optind], argv[optind+1]);
+    return 0;
+}
+
  static void output_xeninfo(void)
  {
      const libxl_version_info *info;

tools/libxl/xl_cmdtable.c +5 -0

@@ -195,6 +195,11 @@ struct cmd_spec cmd_table[] = {
        "Set the number of active VCPUs allowed for the domain",
        "<Domain>  <vCPUs>",
      },
+    { "node-affinity",
+&main_nodeaffinity, 0,
+      "Set the NUMA node affinity for the domain",
+      "<Domain>  <Nodes|all>",
+    },
      { "list-vm",
        &main_list_vm, 0,
        "List the VMs,without DOM0",

Quoted 5 lines · Dario Faggioli

Add some rationale and usage documentation for the new automatic NUMA placement feature of xl.

TODO: * Decide whether we want to have things like "Future Steps/Roadmap"
and/or "Performances/Benchmarks Results" here as well.

I think these would be better in the list archives and on the wiki respectively.

docs/misc/xl-numa-placement.txt

Quoted 6 lines

Signed-off-by Dario Faggioli <[email protected]>

docs/misc/xl-numa-placement.txt new file +0 -0

new file mode 100644

It looks like you are using something approximating markdown syntax here, so you might as well name this xl-numa-placement.markdown and get a .html version etc almost for free.

docs/misc/xl-numa-placement.txt line 35

Quoted 3 lines

docs/misc/xl-numa-placement.txt new file +3 -0

+Of course, if a domain is known to only run on a subset of the physical
+CPUs of the host, it is very easy to turn all its memory accesses into
+local ones, by just constructing it's node affinity (in Xen) basing on

^based

docs/misc/xl-numa-placement.txt line 113

Quoted 5 lines

+ * nodes = [ '0', '1' ] and cpus = "0", with CPU 0 within node 0:

docs/misc/xl-numa-placement.txt new file +4 -0

+   (i.e., cpu affinity subset of node affinity):
+     domain's vcpus can and will only run on host CPU 0. As node affinity
+     is being explicitly set to host NUMA nodes 0 and 1 --- which includes
+     CPU 0 --- all the memory access of the domain will be local;

In this case won't some of (half?) the memory come from node 1 and therefore be non-local to cpu 0?

Quoted 4 lines

docs/misc/xl-numa-placement.txt new file +4 -0

+
+ * `nodes = [ '0', '1' ]` and `cpus = "0, 4", with CPU 0 in node 0 but
+   CPU 4 in, say, node 2 (i.e., cpu affinity superset of node affinity):
+     domain's vcpus can run on host CPUs 0 and 4, with CPU 4 not being within

docs/misc/xl-numa-placement.txt line 122

Quoted 5 lines

docs/misc/xl-numa-placement.txt new file +5 -0

+     the node affinity (explicitly set to host NUMA nodes 0 and 1). The
+     (credit) scheduler will try to keep memory accesses local by scheduling
+     the domain's vcpus on CPU 0, but it may not achieve 100% success;
+
+ * `nodes = [ '0', '1' ]` and `cpus = "4"`, with CPU 4 within, say, node 2

These examples might be a little clearer if you defined up front what the nodes and cpus were and then used that for all of them?

A bunch of what follows would be good to have in the xl or xl.cfg man pages too/instead. (I started with this docs patch so I haven't actually looked at the earlier ones yet, perhaps this is already the case)

Quoted 20 lines

docs/misc/xl-numa-placement.txt new file +17 -0

+
+ * "auto": automatic placement by means of a not better specified (xl
+           implementation dependant) algorithm. It is basically for those
+           who do want automatic placement, but have no idea what policy
+           or algorithm would be better... <<Just give me a sane default!>>
+
+ * "ffit": automatic placement via the First Fit algorithm, applied checking
+           the memory requirement of the domain against the amount of free
+           memory in the various host NUMA nodes;
+
+ * "bfit": automatic placement via the Best Fit algorithm, applied checking
+           the memory requirement of the domain against the amount of free
+           memory in the various host NUMA nodes;
+
+ * "wfit": automatic placement via the Worst Fit algorithm, applied checking
+           the memory requirement of the domain against the amount of free
+           memory in the various host NUMA nodes;
 
<snip>

docs/misc/xl-numa-placement.txt

Quoted 4 lines

+ * nodes_policy="auto" (or "ffit", "bfit", "wfit") and nodes=2:

docs/misc/xl-numa-placement.txt new file +3 -0

+     xl will try fitting the domain on the host NUMA nodes by using the
+     requested policy and only the number of nodes specified in `nodes=`
+     (2 in this example).

Number of nodes rather than specifically node 2? This is different to the examples in the preceding section?

Quoted 6 lines · Dario Faggioli

docs/man/xl.cfg.pod.5 +0 -0

@@ -112,6 +112,15 @@ List of which cpus the guest is allowed
  (all vcpus will run on cpus 0,2,3,5), or `cpus=["2", "3"]` (all vcpus
  will run on cpus 2 and 3).

docs/man/xl.cfg.pod.5 List of which cpus the guest is allowed · line 123

Quoted 10 lines

docs/man/xl.cfg.pod.5 +9 -0

 
+=item B<nodes=[ NODE, NODE, ...]>
+
+List of the NUMA nodes of the host the guest should be considered
+`affine` with. Being affine to a (set of) NUMA node(s) mainly means
+the guest's memory is going to be allocated on those node(s).
+
+A list of nodes should be specified as follows: `nodes=["0", "3"]`
+for the guest to be affine with nodes 0 and 3.
+

Hmm, I think that using "affine" here is technically correct, and is what one would use if writing a research paper; but it's unusual to hear the word in more common English; it would be more common to hear someone describe a VM as "having affinity with". How about something like this:

"The list of NUMA nodes the domain is considered to have affinity with. Memory from the guest will be allocated from these nodes."

Can I ask people to:

* Trim diffs when replying to patches. There is no need to quote the whole patch. Just quote the parts which are needed to understand your comments.

* Leave a blank line between quoted text of any kind and your own added text.

That will help your comments stand out, and be read. This is particularly important if the quoted patches come to more than 80 columns, because then there are lots of breaks in the ">" down the left hand side due to wrap damage and they are hard to tell from interspersed comments.

Thanks and sorry to be picky.

Quoted 5 lines

Of course this is not going to be equally effective, as it will only affect future memory allocations without touching what's already there. However, in future we might want to change this, and use this as an interface for sort-of manual "domain node migration".

I think this is a good idea.

I think if you take my suggestion to rework the previous patch into hypervisor, libxc, and libxl components, you should also move the xl stuff out of that patch and into this one (so that the xl and libxl changes relating to node affinity are each in their own patches).

Quoted 5 lines

A valid setting will directly impact the node_affinity mask of the guest, i.e., it will change the behaviour of the low level memory allocator. On the other hand, this commit does not affect by any means how the guest's vCPUs are scheduled on the host's pCPUs.

I would probably break this into three separate patches, starting with the hypervisor, then libxc, then libxl, especially since they tend to have different maintainers and committers.

docs/man/xl.cfg.pod.5 List of which cpus the guest is allowed · line 123

Quoted 3 lines

docs/man/xl.cfg.pod.5 +3 -0

+A list of nodes should be specified as follows: `nodes=["0", "3"]`
+for the guest to be affine with nodes 0 and 3.
+

Hmm, I think that using "affine" here is technically correct, and is what one would use if writing a research paper; but it's unusual to hear the word in more common English; it would be more common to hear someone describe a VM as "having affinity with".

How about something like this:

"The list of NUMA nodes the domain is considered to have affinity with. Memory from the guest will be allocated from these nodes."

Also, is there a way to specify that the affinity to be to all nodes and/or based on the vcpu mask of the vcpus?

Quoted 6 lines

+/**
+ * This function explicitly sets the affinity of a domain with the
+ * host NUMA nodes.
+ *
+ * @parm xch a handle to an open hypervisor interface.
+ * @parm domid the domain id in which vcpus are to be created.

tools/libxc/xenctrl.h int xc_watchdog(xc_interface *xch, · line 535

Quoted 7 lines

+ * @parm node_affinity the map of the affine nodes.
+ * @return 0 on success, -1 on failure.
+ */

tools/libxc/xenctrl.h +4 -0

+int xc_domain_node_affinity(xc_interface *xch,
+                            uint32_t domind,
+                            xc_nodemap_t node_affinity);
+

Seems like it would be useful to have both a get and a set.

Quoted 6 lines

  int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_cpumap *cpumap)
  {
      GC_INIT(ctx);

tools/libxl/libxl.h +0 -0

tools/libxl/libxl.h int libxl_set_vcpuaffinity(libxl_ctx *ct · line 731

Quoted 6 lines

tools/libxl/libxl.h +2 -0

@@ -727,6 +727,8 @@ int libxl_set_vcpuaffinity(libxl_ctx *ct
                             libxl_cpumap *cpumap);
  int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid,
                                 unsigned int max_vcpus, libxl_cpumap *cpumap);
+int libxl_set_node_affinity(libxl_ctx *ctx, uint32_t domid,
+                            libxl_nodemap *nodemap);

Hmm -- is there really no libxl_get_vcpuaffinity()? That seems to be a missing component...

tools/libxl/libxl.h int libxl_set_vcpuaffinity(libxl_ctx *ct

Quoted 10 lines

tools/libxl/libxl.h +10 -0

+    }
+    else {
+        /*
+         * XXX What would a sane default be? I think doing nothing
+         *     (i.e., relying on cpu-affinity/cpupool ==>  the current
+         *     behavior) should be just fine.
+         *     That would mean we're saying to the user "if you want us
+         *     to take care of NUMA, please tell us, maybe just putting
+         *     'nodes=auto', but tell us... otherwise we do as usual".
+         */

I think that for this patch, doing nothing is fine (which means removing the whole else clause). But once we have the auto placement, I think that "nodes=auto" should be the default.

Quoted 4 lines

tools/libxl/libxl.h +4 -0

+    case XEN_DOMCTL_node_affinity:
+    {
+        domid_t dom = op->domain;
+        struct domain *d = rcu_lock_domain_by_id(dom);

tools/libxl/libxl.h

Quoted 7 lines

tools/libxl/libxl.h +7 -0

+        nodemask_t new_affinity;
+
+        ret = -ESRCH;
+        if ( d == NULL )
+            break;
+
+        /* XXX We need an xsm_* for this I guess, right? */

Yes. :-)

Quoted 7 lines

xen/include/xen/sched.h +0 -0

@@ -346,8 +346,12 @@ struct domain
      /* Various mem_events */
      struct mem_event_per_domain *mem_event;
 

xen/include/xen/sched.h struct domain · line 354

Quoted 7 lines

xen/include/xen/sched.h +5 -1

-    /* Currently computed from union of all vcpu cpu-affinity masks. */
+    /*
+     * Can be specified by the user. If that is not the case, it is
+     * computed from the union of all the vcpu cpu-affinity masks.
+     */
      nodemask_t node_affinity;
+    int has_node_affinity;

There's something that seems a bit clunky about this; but I can't really think of a better way. At every least I'd change the name of the element here to something more descriptive; perhaps "auto_node_affinity" (which would invert the meaning)?

Quoted 5 lines

Hmm, I think that using "affine" here is technically correct, and is what one would use if writing a research paper; but it's unusual to hear the word in more common English; it would be more common to hear someone describe a VM as "having affinity with". How about something like this:

Quoted 2 lines

"The list of NUMA nodes the domain is considered to have affinity with. Memory from the guest will be allocated from these nodes."

That's clunky sentence, the "considered" doesn't add anything. Or perhaps:

"The NUMA nodes preferred by the guest. Memory for the guest will be allocated on these nodes."

The need for quotes around the node numbers is odd. Can that requirement be removed?

Basic idea is: cpu-affinity tells where a vcpu can run, while node-affinity tells where (in terms of on what NUMA nodes, but that of course imply a set of CPUs) all the vcpus of the domain should/prefer to run... Problems starts when you have both of them speaking at the same time!

<snip>

XXX I'm benchmarking this just right now, and peeking at the results
they don't seem too bad. Numbers are mostly close to the case where the VM is already pinned to a node when created. I'll post the results as soon as I can, and I'll be happy to investigate any issue, if you feel like the approach could be the right one.

Besides than "auto" and "ffit", as per the previous patch, enable Best and Worst Fit placement heuristics to xl, for the user to chose them in the config file. Implementation just sits on top of the First Fit algorithm code, with just the proper reordering of the nodes and their free memory before invoking it (and after it finishes).

TODO: * As usual for this series, benchmarking and testing,
as much thorough and comprehensive as possible!

Basic idea is: cpu-affinity tells where a vcpu can run, while node-affinity tells where (in terms of on what NUMA nodes, but that of course imply a set of CPUs) all the vcpus of the domain should/prefer to run... Problems starts when you have both of them speaking at the same time!

Respecting vcpu-affinity should remain the primary concern, thus what we do here is add some two-step logic here and there in the scheduler (i.e., where vcpu-affinity related decisions are being undertaken). During the first step, we check the && of vcpu and node affinity masks, aiming at giving some precedence to the CPUs that are both suitable and preferrable for the domain. However, if that fails in finding a valid CPU, the node-affinity is just ignored and, in the second step, we just fallback to vcpu-affinity, as the original behaviour was.

Both the introduced overhead and the benefits this provides has to be investigated and compared against each other, possibly in varying conditions and running different workloads.

Finally, although still at the RFC level, efforts have been made to write the code in a flexible enough fashion so that, if we ever want to introduce further balancing steps, it shouldn't be too much of a pain.

TODO: * Better investigate and test interactions with cpupools.
* Test, verify and benchmark. Then test, verify and benchmark again, and again and again. What I know as of know is that it does not explode that easily, but whether it works properly and, more important, brings any benefit, this has to be proven (and yes, I'm out running these tests and benchs already, but do not esitate to manifest your will for helping with that :-D).
* Add some counter/stats, e.g., to serve the purpose outlined right above.

XXX I'm benchmarking this just right now, and peeking at the results
they don't seem too bad. Numbers are mostly close to the case where the VM is already pinned to a node when created. I'll post the results as soon as I can, and I'll be happy to investigate any issue, if you feel like the approach could be the right one.