On 06/28/2013 08:32 AM, Dario Faggioli wrote:
Starting from Xen 4.2, libxl has all the bits and pieces are in
s/are in/in/
place for retrieving an adequate amount of information about the host NUMA topology. It is therefore possible, after a bit of shuffling, to arrange those information in the way libvirt wants to present them to the outside world.
Therefore, with this patch, the <topology> section of the host capabilities is properly populated, when running on Xen, so that we can figure out whether or not we're running on a NUMA host, and what its characteristics are.
[raistlin@Zhaman ~]$ sudo virsh --connect xen:/// capabilities <capabilities> <host> <cpu> .... <topology> <cells num='2'> <cell id='0'> <memory unit='KiB'>6291456</memory> <cpus num='8'> <cpu id='0' socket_id='1' core_id='0' siblings='0-1'/> <cpu id='1' socket_id='1' core_id='0' siblings='0-1'/> <cpu id='2' socket_id='1' core_id='1' siblings='2-3'/> <cpu id='3' socket_id='1' core_id='1' siblings='2-3'/> <cpu id='4' socket_id='1' core_id='9' siblings='4-5'/> <cpu id='5' socket_id='1' core_id='9' siblings='4-5'/> <cpu id='6' socket_id='1' core_id='10' siblings='6-7'/> <cpu id='7' socket_id='1' core_id='10' siblings='6-7'/> </cpus> </cell> <cell id='1'> <memory unit='KiB'>6881280</memory> <cpus num='8'> <cpu id='8' socket_id='0' core_id='0' siblings='8-9'/> <cpu id='9' socket_id='0' core_id='0' siblings='8-9'/> <cpu id='10' socket_id='0' core_id='1' siblings='10-11'/> <cpu id='11' socket_id='0' core_id='1' siblings='10-11'/> <cpu id='12' socket_id='0' core_id='9' siblings='12-13'/> <cpu id='13' socket_id='0' core_id='9' siblings='12-13'/> <cpu id='14' socket_id='0' core_id='10' siblings='14-15'/> <cpu id='15' socket_id='0' core_id='10' siblings='14-15'/> </cpus> </cell> </cells> </topology> </host> ....
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> --- src/libxl/libxl_conf.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index e170357..2a9bcf0 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -163,6 +163,8 @@ libxlBuildCapabilities(virArch hostarch, static virCapsPtr libxlMakeCapabilitiesInternal(virArch hostarch, libxl_physinfo *phy_info, + libxl_numainfo *numa_info, int nr_nodes, + libxl_cputopology *cpu_topo, int nr_cpus,
The most prominent pattern in libvirt is one param per line after the first, if they all don't fit in 80 columns. E.g. libxlMakeCapabilitiesInternal(virArch hostarch, libxl_physinfo *phy_info, libxl_numainfo *numa_info, int nr_nodes, libxl_cputopology *cpu_topo, int nr_cpus, ...)
char *capabilities) { char *str, *token; @@ -173,6 +175,12 @@ libxlMakeCapabilitiesInternal(virArch hostarch, int host_pae = 0; struct guest_arch guest_archs[32]; int nr_guest_archs = 0; + + /* For building NUMA capabilities */ + virCapsHostNUMACellCPUPtr *cpus = NULL; + int *nr_cpus_node = NULL; + bool numa_failed = false; +
No need for the extra whitespace between these local variables.
virCapsPtr caps = NULL;
memset(guest_archs, 0, sizeof(guest_archs)); @@ -280,6 +288,100 @@ libxlMakeCapabilitiesInternal(virArch hostarch, nr_guest_archs)) == NULL) goto no_memory;
+ /* What about NUMA? */
What about it? I think the comment should just say "Include NUMA information if available" or similar :).
+ if (!numa_info || !cpu_topo) + return caps; + + if (VIR_ALLOC_N(cpus, nr_nodes)) + goto no_memory; + memset(cpus, 0, sizeof(cpus) * nr_nodes);
VIR_ALLOC_N will already zero-fill the memory. From its' documentation in viralloc.h * Allocate an array of 'count' elements, each sizeof(*ptr) * bytes long and store the address of allocated memory in * 'ptr'. Fill the newly allocated memory with zeros.
+ + if (VIR_ALLOC_N(nr_cpus_node, nr_nodes)) { + VIR_FREE(cpus); + goto no_memory; + } + memset(nr_cpus_node, 0, sizeof(nr_cpus_node) * nr_nodes);
Same here.
+ + /* For each node, prepare a list of CPUs belonging to that node */ + for (i = 0; i < nr_cpus; i++) { + int node = cpu_topo[i].node; + + if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY) + continue; + + nr_cpus_node[node]++; + + if (nr_cpus_node[node] == 1) { + if (VIR_ALLOC(cpus[node]) < 0) { + numa_failed = true; + goto cleanup; + } + } + else { + if (VIR_REALLOC_N(cpus[node], nr_cpus_node[node]) < 0) {
virReportOOMError() ?
+ numa_failed = true; + goto cleanup; + } + } + + /* Mapping between what libxl tells and what libvirt wants */ + cpus[node][nr_cpus_node[node]-1].id = i; + cpus[node][nr_cpus_node[node]-1].socket_id = cpu_topo[i].socket; + cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core; + cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus); + + if (!cpus[node][nr_cpus_node[node]-1].siblings) { + virReportOOMError(); + numa_failed = true; + goto cleanup; + } + } + + /* Let's now populate the siblings bitmaps */ + for (i = 0; i < nr_cpus; i++) { + int j, node = cpu_topo[i].node; + + if (cpu_topo[i].core == LIBXL_CPUTOPOLOGY_INVALID_ENTRY) + continue; + + for (j = 0; j < nr_cpus_node[node]; j++) { + if (cpus[node][j].core_id == cpu_topo[i].core) + ignore_value(virBitmapSetBit(cpus[node][j].siblings, i)); + } + } + + for (i = 0; i < nr_nodes; i++) { + if (numa_info[i].size == LIBXL_NUMAINFO_INVALID_ENTRY) + continue; + + if (virCapabilitiesAddHostNUMACell(caps, i, nr_cpus_node[i], + numa_info[i].size / 1024, + cpus[i]) < 0) {
On my non-NUMA test machine I have the cell memory reported as <memory unit='KiB'>9175040</memory> The machine has 8G of memory, running xen 4.3 rc6, with dom0_mem=1024M. 'xl info --numa' says total_memory : 8190 ... numa_info : node: memsize memfree distances 0: 8960 7116 10 Why is the node memsize > total_memory?
+ virCapabilitiesClearHostNUMACellCPUTopology(cpus[i], + nr_cpus_node[i]); + numa_failed = true; + goto cleanup; + } + + /* This is safe, as the CPU list is now stored in the NUMA cell */ + cpus[i] = NULL; + } + +cleanup: + if (numa_failed) { + /* Looks like something went wrong. Well, that's bad, but probably + * not enough to break the whole driver, so we log and carry on */ + for (i = 0; i < nr_nodes; i++) { + VIR_FREE(cpus[i]); + } + VIR_WARN("Failed to retrieve and build host NUMA topology properly,\n" + "disabling NUMA capabilities"); + virCapabilitiesFreeNUMAInfo(caps); + } + + VIR_FREE(cpus); + VIR_FREE(nr_cpus_node);
Hmm, I'm beginning to think the numa additions to libxlMakeCapabilitiesInternal() should be in a helper function, e.g. libxlMakeNumaCapabilities(), and called when numa_info and cpu_topo are provided. Regards, Jim
+ return caps;
no_memory: @@ -772,7 +874,11 @@ libxlMakeCapabilities(libxl_ctx *ctx) { int err; libxl_physinfo phy_info; + libxl_numainfo *numa_info = NULL; + libxl_cputopology *cpu_topo = NULL; const libxl_version_info *ver_info; + int nr_nodes = 0, nr_cpus = 0; + virCapsPtr caps;
err = regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED); if (err != 0) { @@ -796,9 +902,29 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; }
- return libxlMakeCapabilitiesInternal(virArchFromHost(), + /* Let's try to fetch NUMA info, but it is not critical if we fail */ + numa_info = libxl_get_numainfo(ctx, &nr_nodes); + if (numa_info == NULL) + VIR_WARN("libxl_get_numainfo failed to retrieve NUMA data"); + else { + /* If the above failed, we'd have no NUMa caps anyway! */ + cpu_topo = libxl_get_cpu_topology(ctx, &nr_cpus); + if (cpu_topo == NULL) { + VIR_WARN("libxl_get_cpu_topology failed to retrieve topology"); + libxl_numainfo_list_free(numa_info, nr_nodes); + } + } + + caps = libxlMakeCapabilitiesInternal(virArchFromHost(), &phy_info, + numa_info, nr_nodes, + cpu_topo, nr_cpus, ver_info->capabilities); + + libxl_cputopology_list_free(cpu_topo, nr_cpus); + libxl_numainfo_list_free(numa_info, nr_nodes); + + return caps; }
int