[libvirt] [PATCH 1/4] libxl: implement NUMA capabilities reporting
Jim Fehlig
jfehlig at suse.com
Mon Jul 1 22:47:23 UTC 2013
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 at 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 at 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
>
>
>
>
More information about the libvir-list
mailing list