[PATCH 00 of 10 v3] Automatic NUMA placement for xl

[PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Dario Faggioli]

Hello,

Third version of the NUMA placement series Xen 4.2.

All the comments received during v2's review have been addressed (more details
in single changelogs).

The most notable changes are the following:
 - the libxl_cpumap --> libxl_bitmap renaming has been rebased on top of the
   recent patches that allows us to allocate bitmaps of different sizes;
 - the heuristics for deciding which NUMA placement is the best one has been
   redesigned, so that it now provides total ordering.

Here it is what this posting contains (* = acked during previous round):

 * [PATCH 01 of 10 v3] libxl: add a new Array type to the IDL
   [PATCH 02 of 10 v3] libxl,libxc: introduce libxl_get_numainfo()
 * [PATCH 03 of 10 v3] xl: add more NUMA information to `xl info -n'
   [PATCH 04 of 10 v3] libxl: rename libxl_cpumap to libxl_bitmap
   [PATCH 05 of 10 v3] libxl: expand the libxl_bitmap API a bit
 * [PATCH 06 of 10 v3] libxl: introduce some node map helpers
   [PATCH 07 of 10 v3] libxl: explicitly check for libmath in autoconf
 
Is where data structures, utility functions and infrastructure are introduced.

 * [PATCH 08 of 10 v3] libxl: enable automatic placement of guests on NUMA nodes
 * [PATCH 09 of 10 v3] libxl: have NUMA placement deal with cpupools

Host the core of the mechanism.

 * [PATCH 10 of 10 v3] Some automatic NUMA placement documentation

For some more documentation.

Thanks a lot and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[PATCH 01 of 10 v3] libxl: add a new Array type to the IDL

[Dario Faggioli]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1341416322 -7200
# Node ID 8e367818e194c212cd1470aad663f3243ff53bdb
# Parent  42f76d536b116d2ebad1b6705ae51ecd171d2581
libxl: add a new Array type to the IDL

And make all the required infrastructure updates to enable this.


Since there are currently no uses of this type there is no change to
the generated code.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Tested-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

---
Changes from v2:
 * v2's patch replaced by what Ian Campbell posted as
   <03b641aa89f979a1670b.1340791844@cosworth.uk.xensource.com>, as
   agreed during the review process of that patch.

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -27,6 +27,18 @@ def gen_rand_init(ty, v, indent = "    "
     s = ""
     if isinstance(ty, idl.Enumeration):
         s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), randomize_enum(ty))
+    elif isinstance(ty, idl.Array):
+        if parent is None:
+            raise Exception("Array type must have a parent")
+        s += "%s = rand()%%8;\n" % (parent + ty.lenvar.name)
+        s += "%s = calloc(%s, sizeof(*%s));\n" % \
+            (v, parent + ty.lenvar.name, v)
+        s += "{\n"
+        s += "    int i;\n"
+        s += "    for (i=0; i<%s; i++)\n" % (parent + ty.lenvar.name)
+        s += gen_rand_init(ty.elem_type, v+"[i]",
+                           indent + "        ", parent)
+        s += "}\n"
     elif isinstance(ty, idl.KeyedUnion):
         if parent is None:
             raise Exception("KeyedUnion type must have a parent")
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -11,8 +11,12 @@ def libxl_C_instance_of(ty, instancename
             return libxl_C_type_define(ty)
         else:
             return libxl_C_type_define(ty) + " " + instancename
-    else:
-        return ty.typename + " " + instancename
+
+    s = ""
+    if isinstance(ty, idl.Array):
+        s += libxl_C_instance_of(ty.lenvar.type, ty.lenvar.name) + ";\n"
+
+    return s + ty.typename + " " + instancename
 
 def libxl_C_type_define(ty, indent = ""):
     s = ""
@@ -66,6 +70,21 @@ def libxl_C_type_dispose(ty, v, indent =
             s += libxl_C_type_dispose(f.type, fexpr, indent + "    ", nparent)
             s += "    break;\n"
         s += "}\n"
+    elif isinstance(ty, idl.Array):
+        if parent is None:
+            raise Exception("Array type must have a parent")
+        if ty.elem_type.dispose_fn is not None:
+            s += "{\n"
+            s += "    int i;\n"
+            s += "    for (i=0; i<%s; i++)\n" % (parent + ty.lenvar.name)
+            s += libxl_C_type_dispose(ty.elem_type, v+"[i]",
+                                      indent + "        ", parent)
+        if ty.dispose_fn is not None:
+            if ty.elem_type.dispose_fn is not None:
+                s += "    "
+            s += "%s(%s);\n" % (ty.dispose_fn, ty.pass_arg(v, parent is None))
+        if ty.elem_type.dispose_fn is not None:
+            s += "}\n"
     elif isinstance(ty, idl.Struct) and (parent is None or ty.dispose_fn is None):
         for f in [f for f in ty.fields if not f.const]:
             (nparent,fexpr) = ty.member(v, f, parent is None)
@@ -164,7 +183,24 @@ def libxl_C_type_gen_json(ty, v, indent 
     s = ""
     if parent is None:
         s += "yajl_gen_status s;\n"
-    if isinstance(ty, idl.Enumeration):
+
+    if isinstance(ty, idl.Array):
+        if parent is None:
+            raise Exception("Array type must have a parent")
+        s += "{\n"
+        s += "    int i;\n"
+        s += "    s = yajl_gen_array_open(hand);\n"
+        s += "    if (s != yajl_gen_status_ok)\n"
+        s += "        goto out;\n"
+        s += "    for (i=0; i<%s; i++) {\n" % (parent + ty.lenvar.name)
+        s += libxl_C_type_gen_json(ty.elem_type, v+"[i]",
+                                   indent + "        ", parent)
+        s += "    }\n"
+        s += "    s = yajl_gen_array_close(hand);\n"
+        s += "    if (s != yajl_gen_status_ok)\n"
+        s += "        goto out;\n"
+        s += "}\n"
+    elif isinstance(ty, idl.Enumeration):
         s += "s = libxl__yajl_gen_enum(hand, %s_to_string(%s));\n" % (ty.typename, ty.pass_arg(v, parent is None))
         s += "if (s != yajl_gen_status_ok)\n"
         s += "    goto out;\n"
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -266,6 +266,17 @@ string = Builtin("char *", namespace = N
                  json_fn = "libxl__string_gen_json",
                  autogenerate_json = False)
 
+class Array(Type):
+    """An array of the same type"""
+    def __init__(self, elem_type, lenvar_name, **kwargs):
+        kwargs.setdefault('dispose_fn', 'free')
+        Type.__init__(self, namespace=elem_type.namespace, typename=elem_type.rawname + " *", **kwargs)
+
+        lv_kwargs = dict([(x.lstrip('lenvar_'),y) for (x,y) in kwargs.items() if x.startswith('lenvar_')])
+
+        self.lenvar = Field(integer, lenvar_name, **lv_kwargs)
+        self.elem_type = elem_type
+
 class OrderedDict(dict):
     """A dictionary which remembers insertion order.
 
diff --git a/tools/libxl/idl.txt b/tools/libxl/idl.txt
--- a/tools/libxl/idl.txt
+++ b/tools/libxl/idl.txt
@@ -145,11 +145,24 @@ idl.KeyedUnion
 
  A subclass of idl.Aggregate which represents the C union type
  where the currently valid member of the union can be determined based
- upon another member in the containing type.
+ upon another member in the containing type. An idl.KeyedUnion must
+ always be a member of a containing idl.Aggregate type.
 
- The KeyedUnion.keyvar contains an idl.type the member of the
- containing type which determines the valid member of the union. The
- must be an instance of the Enumeration type.
+ The KeyedUnion.keyvar contains an idl.Field, this is the member of
+ the containing type which determines the valid member of the
+ union. The idl.Field.type of the keyvar must be an Enumeration type.
+
+idl.Array
+
+  A class representing an array of similar elements. An idl.Array must
+  always be an idl.Field of a containing idl.Aggregate.
+
+  idl.Array.elem_type contains an idl.Type which is the type of each
+  element of the array.
+
+  idl.Array.len_var contains an idl.Field which is added to the parent
+  idl.Aggregate and will contain the length of the array. The field
+  MUST be named num_ARRAYNAME.
 
 Standard Types
 --------------
diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -55,7 +55,8 @@ def ocaml_type_of(ty):
             return "int%d" % ty.width
         else:
             return "int"
-
+    elif isinstance(ty,idl.Array):
+        return "%s array" % ocaml_type_of(ty.elem_type)
     elif isinstance(ty,idl.Builtin):
         if not builtins.has_key(ty.typename):
             raise NotImplementedError("Unknown Builtin %s (%s)" % (ty.typename, type(ty)))
@@ -138,6 +139,8 @@ def c_val(ty, c, o, indent="", parent = 
         if not fn:
             raise NotImplementedError("No c_val fn for Builtin %s (%s)" % (ty.typename, type(ty)))
         s += "%s;" % (fn % { "o": o, "c": c })
+    elif isinstance (ty,idl.Array):
+        raise("Cannot handle Array type\n")
     elif isinstance(ty,idl.Enumeration) and (parent is None):
         n = 0
         s += "switch(Int_val(%s)) {\n" % o
@@ -195,6 +198,16 @@ def ocaml_Val(ty, o, c, indent="", paren
         if not fn:
             raise NotImplementedError("No ocaml Val fn for Builtin %s (%s)" % (ty.typename, type(ty)))
         s += "%s = %s;" % (o, fn % { "c": c })
+    elif isinstance(ty, idl.Array):
+        s += "{\n"
+        s += "\t    int i;\n"
+        s += "\t    value array_elem;\n"
+        s += "\t    %s = caml_alloc(%s,0);\n" % (o, parent + ty.lenvar.name)
+        s += "\t    for(i=0; i<%s; i++) {\n" % (parent + ty.lenvar.name)
+        s += "\t        %s\n" % ocaml_Val(ty.elem_type, "array_elem", c + "[i]", "")
+        s += "\t        Store_field(%s, i, array_elem);\n" % o
+        s += "\t    }\n"
+        s += "\t}"
     elif isinstance(ty,idl.Enumeration) and (parent is None):
         n = 0
         s += "switch(%s) {\n" % c
diff --git a/tools/python/genwrap.py b/tools/python/genwrap.py
--- a/tools/python/genwrap.py
+++ b/tools/python/genwrap.py
@@ -4,7 +4,7 @@ import sys,os
 
 import idl
 
-(TYPE_DEFBOOL, TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_AGGREGATE) = range(6)
+(TYPE_DEFBOOL, TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_ARRAY, TYPE_AGGREGATE) = range(7)
 
 def py_type(ty):
     if ty == idl.bool:
@@ -18,6 +18,8 @@ def py_type(ty):
             return TYPE_INT
         else:
             return TYPE_UINT
+    if isinstance(ty, idl.Array):
+        return TYPE_ARRAY
     if isinstance(ty, idl.Aggregate):
         return TYPE_AGGREGATE
     if ty == idl.string:
@@ -74,7 +76,7 @@ def py_attrib_get(ty, f):
         l.append('    return genwrap__ull_get(self->obj.%s);'%f.name)
     elif t == TYPE_STRING:
         l.append('    return genwrap__string_get(&self->obj.%s);'%f.name)
-    elif t == TYPE_AGGREGATE:
+    elif t == TYPE_AGGREGATE or t == TYPE_ARRAY:
         l.append('    PyErr_SetString(PyExc_NotImplementedError, "Getting %s");'%ty.typename)
         l.append('    return NULL;')
     else:
@@ -105,7 +107,7 @@ def py_attrib_set(ty, f):
         l.append('    return ret;')
     elif t == TYPE_STRING:
         l.append('    return genwrap__string_set(v, &self->obj.%s);'%f.name)
-    elif t == TYPE_AGGREGATE:
+    elif t == TYPE_AGGREGATE or t == TYPE_ARRAY:
         l.append('    PyErr_SetString(PyExc_NotImplementedError, "Setting %s");'%ty.typename)
         l.append('    return -1;')
     else:

[PATCH 02 of 10 v3] libxl, libxc: introduce libxl_get_numainfo()

[Dario Faggioli]

# HG changeset patch
# User Dario Faggioli <raistlin@linux.it>
# Date 1341416323 -7200
# Node ID 0ca91a203fc95d3d18bb436ecdc7106b0b2ff22f
# Parent  8e367818e194c212cd1470aad663f3243ff53bdb
libxl,libxc: introduce libxl_get_numainfo()

Make some NUMA node information available to the toolstack. Achieve
this by means of xc_numainfo(), which exposes memory size and amount
of free memory of each node, as well as the relative distances of
each node to all the others.

For properly exposing distances we need the IDL to support arrays.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

---
Changes from v2:
 * converted libxl__zalloc(NULL, ...) to libxl_calloc(NOGC, ...).
 * Fixed the comment about memory ownership of libxl_get_numainfo().
 * Added a comment for libxl_numainfo in libxl_types.idl.

Changes from v1:
 * malloc converted to libxl__zalloc(NOGC, ...).
 * The patch also accommodates some bits of what was in "libxc,
   libxl: introduce xc_nodemap_t and libxl_nodemap" which was
   removed as well, as full support for node maps at libxc
   level is not needed (yet!).

diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -35,6 +35,20 @@ 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;
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -329,6 +329,12 @@ int xc_get_cpumap_size(xc_interface *xch
 /* allocate a cpumap */
 xc_cpumap_t xc_cpumap_alloc(xc_interface *xch);
 
+ /*
+ * NODEMAP handling
+ */
+/* return maximum number of NUMA nodes the hypervisor supports */
+int xc_get_max_nodes(xc_interface *xch);
+
 /*
  * DOMAIN DEBUGGING FUNCTIONS
  */
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3298,6 +3298,75 @@ fail:
     return ret;
 }
 
+libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr)
+{
+    GC_INIT(ctx);
+    xc_numainfo_t ninfo;
+    DECLARE_HYPERCALL_BUFFER(xc_node_to_memsize_t, memsize);
+    DECLARE_HYPERCALL_BUFFER(xc_node_to_memfree_t, memfree);
+    DECLARE_HYPERCALL_BUFFER(uint32_t, node_dists);
+    libxl_numainfo *ret = NULL;
+    int i, j, max_nodes;
+
+    max_nodes = libxl_get_max_nodes(ctx);
+    if (max_nodes == 0)
+    {
+        LIBXL__LOG(ctx, XTL_ERROR, "Unable to determine number of NODES");
+        ret = NULL;
+        goto out;
+    }
+
+    memsize = xc_hypercall_buffer_alloc
+        (ctx->xch, memsize, sizeof(*memsize) * max_nodes);
+    memfree = xc_hypercall_buffer_alloc
+        (ctx->xch, memfree, sizeof(*memfree) * max_nodes);
+    node_dists = xc_hypercall_buffer_alloc
+        (ctx->xch, node_dists, sizeof(*node_dists) * max_nodes * max_nodes);
+    if ((memsize == NULL) || (memfree == NULL) || (node_dists == NULL)) {
+        LIBXL__LOG_ERRNOVAL(ctx, XTL_ERROR, ENOMEM,
+                            "Unable to allocate hypercall arguments");
+        goto fail;
+    }
+
+    set_xen_guest_handle(ninfo.node_to_memsize, memsize);
+    set_xen_guest_handle(ninfo.node_to_memfree, memfree);
+    set_xen_guest_handle(ninfo.node_to_node_distance, node_dists);
+    ninfo.max_node_index = max_nodes - 1;
+    if (xc_numainfo(ctx->xch, &ninfo) != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting numainfo");
+        goto fail;
+    }
+
+    if (ninfo.max_node_index < max_nodes - 1)
+        max_nodes = ninfo.max_node_index + 1;
+
+    *nr = max_nodes;
+
+    ret = libxl__zalloc(NOGC, sizeof(libxl_numainfo) * max_nodes);
+    for (i = 0; i < max_nodes; i++)
+        ret[i].dists = libxl__calloc(NOGC, max_nodes, sizeof(*node_dists));
+
+    for (i = 0; i < max_nodes; i++) {
+#define V(mem, i) (mem[i] == INVALID_NUMAINFO_ID) ? \
+    LIBXL_NUMAINFO_INVALID_ENTRY : mem[i]
+        ret[i].size = V(memsize, i);
+        ret[i].free = V(memfree, i);
+        ret[i].num_dists = max_nodes;
+        for (j = 0; j < ret[i].num_dists; j++)
+            ret[i].dists[j] = V(node_dists, i * max_nodes + j);
+#undef V
+    }
+
+ fail:
+    xc_hypercall_buffer_free(ctx->xch, memsize);
+    xc_hypercall_buffer_free(ctx->xch, memfree);
+    xc_hypercall_buffer_free(ctx->xch, node_dists);
+
+ out:
+    GC_FREE;
+    return ret;
+}
+
 const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
 {
     union {
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -532,6 +532,9 @@ int libxl_domain_preserve(libxl_ctx *ctx
 /* get max. number of cpus supported by hypervisor */
 int libxl_get_max_cpus(libxl_ctx *ctx);
 
+/* get max. number of NUMA nodes supported by hypervisor */
+int libxl_get_max_nodes(libxl_ctx *ctx);
+
 int libxl_domain_rename(libxl_ctx *ctx, uint32_t domid,
                         const char *old_name, const char *new_name);
 
@@ -604,6 +607,10 @@ void libxl_vminfo_list_free(libxl_vminfo
 libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx, int *nb_cpu_out);
 void libxl_cputopology_list_free(libxl_cputopology *, int nb_cpu);
 
+#define LIBXL_NUMAINFO_INVALID_ENTRY (~(uint32_t)0)
+libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr);
+void libxl_numainfo_list_free(libxl_numainfo *, int nr);
+
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
                                 int *nb_vcpu, int *nr_vcpus_out);
 void libxl_vcpuinfo_list_free(libxl_vcpuinfo *, int nr_vcpus);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -433,6 +433,15 @@ libxl_physinfo = Struct("physinfo", [
     ("cap_hvm_directio", bool),
     ], dir=DIR_OUT)
 
+# NUMA node characteristics: size and free are how much memory it has, and how
+# much of it is free, respectively. dists is an array of distances from this
+# node to each other node.
+libxl_numainfo = Struct("numainfo", [
+    ("size", uint64),
+    ("free", uint64),
+    ("dists", Array(uint32, "num_dists")),
+    ], dir=DIR_OUT)
+
 libxl_cputopology = Struct("cputopology", [
     ("core", uint32),
     ("socket", uint32),
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -572,6 +572,11 @@ 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)
 {
@@ -594,6 +599,14 @@ void libxl_cputopology_list_free(libxl_c
     free(list);
 }
 
+void libxl_numainfo_list_free(libxl_numainfo *list, int nr)
+{
+    int i;
+    for (i = 0; i < nr; i++)
+        libxl_numainfo_dispose(&list[i]);
+    free(list);
+}
+
 void libxl_vcpuinfo_list_free(libxl_vcpuinfo *list, int nr)
 {
     int i;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -484,6 +484,7 @@ typedef struct xen_sysctl_topologyinfo x
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_topologyinfo_t);
 
 /* XEN_SYSCTL_numainfo */
+#define INVALID_NUMAINFO_ID (~0U)
 struct xen_sysctl_numainfo {
     /*
      * IN: maximum addressable entry in the caller-provided arrays.

[PATCH 03 of 10 v3] xl: add more NUMA information to `xl info -n'

[Dario Faggioli]

# HG changeset patch
# User Dario Faggioli <raistlin@linux.it>
# Date 1341416323 -7200
# Node ID f1227d5a82e56d10e302aec4c3717d281718a349
# Parent  0ca91a203fc95d3d18bb436ecdc7106b0b2ff22f
xl: add more NUMA information to `xl info -n'

So that the user knows how much memory there is on each node and
how far they are from each others.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
Changes from v1:
 * integer division replaced by right shift.

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4249,6 +4249,36 @@ static void output_physinfo(void)
     return;
 }
 
+static void output_numainfo(void)
+{
+    libxl_numainfo *info;
+    int i, j, nr;
+
+    info = libxl_get_numainfo(ctx, &nr);
+    if (info == NULL) {
+        fprintf(stderr, "libxl_get_numainfo failed.\n");
+        return;
+    }
+
+    printf("numa_info              :\n");
+    printf("node:    memsize    memfree    distances\n");
+
+    for (i = 0; i < nr; i++) {
+        if (info[i].size != LIBXL_NUMAINFO_INVALID_ENTRY) {
+            printf("%4d:    %6"PRIu64"     %6"PRIu64"      %d", i,
+                   info[i].size >> 20, info[i].free >> 20,
+                   info[i].dists[0]);
+            for (j = 1; j < info[i].num_dists; j++)
+                printf(",%d", info[i].dists[j]);
+            printf("\n");
+        }
+    }
+
+    libxl_numainfo_list_free(info, nr);
+
+    return;
+}
+
 static void output_topologyinfo(void)
 {
     libxl_cputopology *info;
@@ -4271,8 +4301,6 @@ static void output_topologyinfo(void)
 
     libxl_cputopology_list_free(info, nr);
 
-    printf("numa_info              : none\n");
-
     return;
 }
 
@@ -4282,8 +4310,10 @@ static void info(int numa)
 
     output_physinfo();
 
-    if (numa)
+    if (numa) {
         output_topologyinfo();
+        output_numainfo();
+    }
 
     output_xeninfo();

[PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Dario Faggioli]

Hello,

Third version of the NUMA placement series Xen 4.2.

All the comments received during v2's review have been addressed (more details
in single changelogs).

The most notable changes are the following:
 - the libxl_cpumap --> libxl_bitmap renaming has been rebased on top of the
   recent patches that allows us to allocate bitmaps of different sizes;
 - the heuristics for deciding which NUMA placement is the best one has been
   redesigned, so that it now provides total ordering.

Here it is what this posting contains (* = acked during previous round):

 * [PATCH 01 of 10 v3] libxl: add a new Array type to the IDL
   [PATCH 02 of 10 v3] libxl,libxc: introduce libxl_get_numainfo()
 * [PATCH 03 of 10 v3] xl: add more NUMA information to `xl info -n'
   [PATCH 04 of 10 v3] libxl: rename libxl_cpumap to libxl_bitmap
   [PATCH 05 of 10 v3] libxl: expand the libxl_bitmap API a bit
 * [PATCH 06 of 10 v3] libxl: introduce some node map helpers
   [PATCH 07 of 10 v3] libxl: explicitly check for libmath in autoconf
 
Is where data structures, utility functions and infrastructure are introduced.

 * [PATCH 08 of 10 v3] libxl: enable automatic placement of guests on NUMA nodes
 * [PATCH 09 of 10 v3] libxl: have NUMA placement deal with cpupools

Host the core of the mechanism.

 * [PATCH 10 of 10 v3] Some automatic NUMA placement documentation

For some more documentation.

Thanks a lot and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Dario Faggioli]

On Wed, 2012-07-04 at 18:02 +0200, Dario Faggioli wrote:
> Hello,
> 
Sorry for this! I don't have the slightest idea of why it `hg email'
hanged after sending only these 3 mails. :-(

Anyway, my second attempt seems to have succeeded, so please, consider
that for the full series (i.e., the thread where the msg-id of the
introductory mail is <patchbomb.1341418679@Solace>), and sorry again for
the noise.

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Ian Campbell]

On Wed, 2012-07-04 at 17:17 +0100, Dario Faggioli wrote:
> Hello,
> 
> Third version of the NUMA placement series Xen 4.2.

I'm afraid I get a segfault with this:

        quartz:~# xl dest d32-1 ; gdb --args xl cr /etc/xen/debian-x86_32p-1
        [...]
        (gdb) r
        Starting program: /usr/sbin/xl cr /etc/xen/debian-x86_32p-1
        [Thread debugging using libthread_db enabled]
        Parsing config from /etc/xen/debian-x86_32p-1
        
        Program received signal SIGSEGV, Segmentation fault.
        *__GI___libc_free (mem=0x1) at malloc.c:3710
        3710	malloc.c: No such file or directory.
        	in malloc.c
        (gdb) bt
        #0  *__GI___libc_free (mem=0x1) at malloc.c:3710
        #1  0xb7fa8b78 in libxl_bitmap_dispose (map=0xbffff170) at libxl_utils.c:510
        #2  0xb7fadbde in libxl__get_numa_candidates (gc=0x806b7a0, min_free_memkb=141312, min_cpus=4, min_nodes=0, max_nodes=0, suitable_cpumap=0xbffff1f4, cndts=0xbffff208, nr_cndts=0xbffff20c) at libxl_numa.c:397
        #3  0xb7fa45ec in numa_place_domain (gc=0x806b7a0, domid=8, info=0xbffff5dc, state=0x806a954) at libxl_dom.c:169
        #4  libxl__build_pre (gc=0x806b7a0, domid=8, info=0xbffff5dc, state=0x806a954) at libxl_dom.c:232
        #5  0xb7f98234 in libxl__domain_build (gc=0x806b7a0, info=0xbffff5dc, domid=8, state=0x806a954) at libxl_create.c:320
        #6  0xb7f9859f in domcreate_bootloader_done (egc=0xbffff43c, bl=0x806a998, rc=0) at libxl_create.c:695
        #7  0xb7fb5e50 in bootloader_callback (egc=<value optimized out>, bl=0x806a998, rc=0) at libxl_bootloader.c:256
        #8  0xb7fb7482 in libxl__bootloader_run (egc=0xbffff43c, bl=0x806a998) at libxl_bootloader.c:394
        #9  0xb7f99535 in initiate_domain_create (ctx=<value optimized out>, d_config=<value optimized out>, domid=0x8068354, restore_fd=-1, ao_how=0x0, aop_console_how=0x0) at libxl_create.c:635
        #10 do_domain_create (ctx=<value optimized out>, d_config=<value optimized out>, domid=0x8068354, restore_fd=-1, ao_how=0x0, aop_console_how=0x0) at libxl_create.c:1039
        #11 0xb7f9966f in libxl_domain_create_new (ctx=0x8069030, d_config=0xbffff5ac, domid=0x8068354, ao_how=0x0, aop_console_how=0x0) at libxl_create.c:1062
        #12 0x0805c479 in create_domain (dom_info=<value optimized out>) at xl_cmdimpl.c:1809
        #13 0x0805dd13 in main_create (argc=2, argv=0xbffffd28) at xl_cmdimpl.c:3774
        #14 0x0804d1d6 in main (argc=3, argv=0xbffffd24) at xl.c:263
        (gdb) frame 1
        #1  0xb7fa8b78 in libxl_bitmap_dispose (map=0xbffff170) at libxl_utils.c:510
        510	libxl_utils.c: No such file or directory.
        	in libxl_utils.c
        (gdb) print *map
        $2 = {size = 3221221764, map = 0x1 <Address 0x1 out of bounds>}
        (gdb) frame 2
        #2  0xb7fadbde in libxl__get_numa_candidates (gc=0x806b7a0, min_free_memkb=141312, min_cpus=4, min_nodes=0, max_nodes=0, suitable_cpumap=0xbffff1f4, cndts=0xbffff208, nr_cndts=0xbffff20c) at libxl_numa.c:397
        397	libxl_numa.c: No such file or directory.
        	in libxl_numa.c
        (gdb) print suitable_nodemap
        $3 = {size = 3221221764, map = 0x1 <Address 0x1 out of bounds>}
        (gdb) print nodemap
        $4 = {size = 0, map = 0x0}
        
So it looks like suitable_nodemap wasn't initialised?

There are a few "goto out"s before initialising that variable, but none
of them log (really they should) and I didn't investigate which one it
was yet.

Ian.

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Ian Campbell]

On Fri, 2012-07-06 at 12:16 +0100, Ian Campbell wrote:
> There are a few "goto out"s before initialising that variable, but none
> of them log (really they should) and I didn't investigate which one it
> was yet.

It seems to be:
   /* If we don't have at least 2 nodes, it is useless to proceed */
    if (nr_nodes < 2) {
        LOG(DEBUG, "only %d node. no placement required", nr_nodes);
        rc = 0;
        goto out;
    }

(LOG is mine...). The other exit paths look like the log further down
the stack but not this one so it is worth adding.

You probably want another libxl_bitmap_init near the top of the
function.

Ian.

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Ian Campbell]

On Fri, 2012-07-06 at 12:20 +0100, Ian Campbell wrote:

> You probably want another libxl_bitmap_init near the top of the
> function.

Works for me:

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1341573735 -3600
# Node ID 4f964e2446c935838f54b9cae48a6c62fd8de3d0
# Parent  124ddd91c8de38204e94d3125013a40aaa326774
[mq]: libxl-numa-place-segfault.patch

diff -r 124ddd91c8de -r 4f964e2446c9 tools/libxl/libxl_numa.c
--- a/tools/libxl/libxl_numa.c	Wed Jul 04 17:38:44 2012 +0200
+++ b/tools/libxl/libxl_numa.c	Fri Jul 06 12:22:15 2012 +0100
@@ -258,6 +258,7 @@ int libxl__get_numa_candidates(libxl__gc
     libxl_bitmap suitable_nodemap, nodemap;
     int array_size, rc;
 
+    libxl_bitmap_init(&suitable_nodemap);
     libxl_bitmap_init(&nodemap);
 
     /* Get platform info and prepare the map for testing the combinations */
@@ -266,6 +267,7 @@ int libxl__get_numa_candidates(libxl__gc
         return ERROR_FAIL;
     /* If we don't have at least 2 nodes, it is useless to proceed */
     if (nr_nodes < 2) {
+        LOG(DEBUG, "only %d node. no placement required", nr_nodes);
         rc = 0;
         goto out;
     }

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Ian Campbell]

On Wed, 2012-07-04 at 17:17 +0100, Dario Faggioli wrote:
> Hello,
> 
> Third version of the NUMA placement series Xen 4.2.
> 
> All the comments received during v2's review have been addressed (more details
> in single changelogs).
> 
> The most notable changes are the following:
>  - the libxl_cpumap --> libxl_bitmap renaming has been rebased on top of the
>    recent patches that allows us to allocate bitmaps of different sizes;
>  - the heuristics for deciding which NUMA placement is the best one has been
>    redesigned, so that it now provides total ordering.
> 
> Here it is what this posting contains (* = acked during previous round):
> 
>  * [PATCH 01 of 10 v3] libxl: add a new Array type to the IDL
>    [PATCH 02 of 10 v3] libxl,libxc: introduce libxl_get_numainfo()
>  * [PATCH 03 of 10 v3] xl: add more NUMA information to `xl info -n'
>    [PATCH 04 of 10 v3] libxl: rename libxl_cpumap to libxl_bitmap
>    [PATCH 05 of 10 v3] libxl: expand the libxl_bitmap API a bit
>  * [PATCH 06 of 10 v3] libxl: introduce some node map helpers
>    [PATCH 07 of 10 v3] libxl: explicitly check for libmath in autoconf

These are now sufficiently acked that I have committed them.
 
> Is where data structures, utility functions and infrastructure are introduced.
> 
>  * [PATCH 08 of 10 v3] libxl: enable automatic placement of guests on NUMA nodes

As discussed this one has a few issues so I stopped before committing
this one.

Ian.

>  * [PATCH 09 of 10 v3] libxl: have NUMA placement deal with cpupools
> 
> Host the core of the mechanism.
> 
>  * [PATCH 10 of 10 v3] Some automatic NUMA placement documentation
> 
> For some more documentation.
> 
> Thanks a lot and Regards,
> Dario
> 

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Dario Faggioli]

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

On Fri, 2012-07-06 at 12:22 +0100, Ian Campbell wrote: 
> On Fri, 2012-07-06 at 12:20 +0100, Ian Campbell wrote:
> 
> > You probably want another libxl_bitmap_init near the top of the
> > function.
> 
Damn. I tested with two, with (fake) 4 and (fake) 8, but I guess I
forgot testing with just one node this time. Sorry for that. :-(

> Works for me:
> 
Good. Thank you,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



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

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

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

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Ian Campbell]

(resend as I don't think I had SMTP setup properly on my laptop, sorry if you get this twice!)

On Wed, 2012-07-04 at 12:17 -0400, Dario Faggioli wrote:
> Hello,
> 
> Third version of the NUMA placement series Xen 4.2.

Starting an HVM guest (although I don't suppose it is HVM specific) on a
none NUMA system I get this:

        libxl: debug: libxl_numa.c:270:libxl__get_numa_candidates: only 1 node. no placement required
        libxl: detail: libxl_dom.c:175:numa_place_domain: 0 NUMA placement candidates found
        libxl: notice: libxl_dom.c:180:numa_place_domain: NUMA placement failed, performance might be affected
        
this being a non-NUMA system I suppose it is not the end of the world.
It'd be nice to avoid the warning though -- perhaps
libxl__get_numa_candidates should either not special case single node
systems or it should manually return the trivial candidate set?

> All the comments received during v2's review have been addressed (more details
> in single changelogs).
> 
> The most notable changes are the following:
>  - the libxl_cpumap --> libxl_bitmap renaming has been rebased on top of the
>    recent patches that allows us to allocate bitmaps of different sizes;
>  - the heuristics for deciding which NUMA placement is the best one has been
>    redesigned, so that it now provides total ordering.
> 
> Here it is what this posting contains (* = acked during previous round):
> 
>  * [PATCH 01 of 10 v3] libxl: add a new Array type to the IDL
>    [PATCH 02 of 10 v3] libxl,libxc: introduce libxl_get_numainfo()
>  * [PATCH 03 of 10 v3] xl: add more NUMA information to `xl info -n'
>    [PATCH 04 of 10 v3] libxl: rename libxl_cpumap to libxl_bitmap
>    [PATCH 05 of 10 v3] libxl: expand the libxl_bitmap API a bit
>  * [PATCH 06 of 10 v3] libxl: introduce some node map helpers
>    [PATCH 07 of 10 v3] libxl: explicitly check for libmath in autoconf
>  
> Is where data structures, utility functions and infrastructure are introduced.
> 
>  * [PATCH 08 of 10 v3] libxl: enable automatic placement of guests on NUMA nodes
>  * [PATCH 09 of 10 v3] libxl: have NUMA placement deal with cpupools
> 
> Host the core of the mechanism.
> 
>  * [PATCH 10 of 10 v3] Some automatic NUMA placement documentation
> 
> For some more documentation.
> 
> Thanks a lot and Regards,
> Dario
> 

Re: [PATCH 00 of 10 v3] Automatic NUMA placement for xl

[Dario Faggioli]

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

On Sun, 2012-07-08 at 12:32 -0600, Ian Campbell wrote: 
> On Wed, 2012-07-04 at 12:17 -0400, Dario Faggioli wrote:
> > Hello,
> > 
> > Third version of the NUMA placement series Xen 4.2.
> 
> Starting an HVM guest (although I don't suppose it is HVM specific) on a
> none NUMA system I get this:
> 
>         libxl: debug: libxl_numa.c:270:libxl__get_numa_candidates: only 1 node. no placement required
>         libxl: detail: libxl_dom.c:175:numa_place_domain: 0 NUMA placement candidates found
>         libxl: notice: libxl_dom.c:180:numa_place_domain: NUMA placement failed, performance might be affected
>         
Mmm... I see. No, it's not HVM specific and you're right, it does not
make much sense.

> this being a non-NUMA system I suppose it is not the end of the world.
> It'd be nice to avoid the warning though -- perhaps
> libxl__get_numa_candidates should either not special case single node
> systems or it should manually return the trivial candidate set?
> 
Yep, I can definitely do that, along with the other fix you suggested
and the wording of the doc patch and resend the last three.

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



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

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

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