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

Re: [libvirt] [PATCH] nodeinfo: Don't fail on non-contiguous NUMA topologies



On Thu, Jul 11, 2013 at 04:09:47PM +0200, Peter Krempa wrote:
> From: hejia hejia <jiakernel gmail com>
> 
> nodeGetFreeMemory and nodeGetCellsFreeMemory assumed that the NUMA nodes
> are contiguous and starting from 0. Unfortunately there are machines
> that don't match this assumption:
> 
> available: 1 nodes (1)
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
> node 1 size: 16340 MB
> node 1 free: 11065 MB
> 
> Before this patch:
> error: internal error Failed to query NUMA free memory
> error: internal error Failed to query NUMA free memory for node: 0
> 
> After this patch:
> Total: 15772580 KiB
> 0: 0 KiB
> 
> Signed-off-by: Peter Krempa <pkrempa redhat com>
> ---
>  src/nodeinfo.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index a2a2f73..4df4851 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -1686,12 +1686,9 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems,
> 
>      for (numCells = 0, n = startCell; n <= lastCell; n++) {
>          long long mem;
> -        if (numa_node_size64(n, &mem) < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("Failed to query NUMA free memory for node: %d"),
> -                           n);
> -            goto cleanup;
> -        }
> +        if (numa_node_size64(n, &mem) < 0)
> +            mem = 0;
> +
>          freeMems[numCells++] = mem;
>      }
>      ret = numCells;
> @@ -1712,15 +1709,12 @@ nodeGetFreeMemory(void)
> 
>      for (n = 0; n <= numa_max_node(); n++) {
>          long long mem;
> -        if (numa_node_size64(n, &mem) < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           "%s", _("Failed to query NUMA free memory"));
> -            goto cleanup;
> -        }
> +        if (numa_node_size64(n, &mem) < 0)
> +            continue;
> +
>          freeMem += mem;
>      }

ACK, looking at the libnuma code, the only reasons why numa_node_size64
would return -1, is if the NUMA node does not exist, or the sysfs file
was not parsable as an integer. The latter is basically not going to
happen, so it is reasonable to skip this error reporting unconditionally.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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