[libvirt] [PATCH] Make nodeGetInfo report the correct number of NUMA nodes.
The nodeGetInfo code was always assuming that machine had a single NUMA node, which is not correct. The good news is that libnuma gives us this information pretty easily, so let's properly report it. NOTE: With recent hardware starting to support CPU hot-add and hot-remove, both this code and the nodeCapsInitNUMA() code are quickly going to become obsolete. We'll have to think of a more dynamic solution for dealing with NUMA nodes and CPUs that can come and go at will. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/nodeinfo.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0748602..8d7e055 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -159,7 +159,11 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, nodeinfo->cpus = 0; nodeinfo->mhz = 0; - nodeinfo->nodes = nodeinfo->cores = 1; + nodeinfo->cores = 1; + if (numa_available() < 0) + nodeinfo->nodes = 1; + else + nodeinfo->nodes = numa_max_node() + 1; /* NB: It is impossible to fill our nodes, since cpuinfo * has no knowledge of NUMA nodes */ -- 1.6.6.1
On Thu, Mar 11, 2010 at 06:00:56PM -0500, Chris Lalancette wrote:
The nodeGetInfo code was always assuming that machine had a single NUMA node, which is not correct. The good news is that libnuma gives us this information pretty easily, so let's properly report it.
okay
NOTE: With recent hardware starting to support CPU hot-add and hot-remove, both this code and the nodeCapsInitNUMA() code are quickly going to become obsolete. We'll have to think of a more dynamic solution for dealing with NUMA nodes and CPUs that can come and go at will.
well it makes little sense to refresh all the time, I would expect some kind of signal we can hook on to detect change in topology or capacities, though I don't see anything on signal.h for this
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/nodeinfo.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0748602..8d7e055 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -159,7 +159,11 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo,
nodeinfo->cpus = 0; nodeinfo->mhz = 0; - nodeinfo->nodes = nodeinfo->cores = 1; + nodeinfo->cores = 1; + if (numa_available() < 0) + nodeinfo->nodes = 1; + else + nodeinfo->nodes = numa_max_node() + 1;
/* NB: It is impossible to fill our nodes, since cpuinfo * has no knowledge of NUMA nodes */
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 03/12/2010 06:32 AM, Daniel Veillard wrote:
On Thu, Mar 11, 2010 at 06:00:56PM -0500, Chris Lalancette wrote:
The nodeGetInfo code was always assuming that machine had a single NUMA node, which is not correct. The good news is that libnuma gives us this information pretty easily, so let's properly report it.
okay
NOTE: With recent hardware starting to support CPU hot-add and hot-remove, both this code and the nodeCapsInitNUMA() code are quickly going to become obsolete. We'll have to think of a more dynamic solution for dealing with NUMA nodes and CPUs that can come and go at will.
well it makes little sense to refresh all the time, I would expect some kind of signal we can hook on to detect change in topology or capacities, though I don't see anything on signal.h for this
Well, it's actually not horrible to refresh all of the time. The way we get this information is all in memory (from /proc or /sys), so it's a very fast operation. That being said, if we can get a signal when things change, and just react to that, that's probably the best way to go. This stuff is all very new, so things are still being put into place; we'll just have to figure out how to be more dynamic in the future.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/nodeinfo.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0748602..8d7e055 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -159,7 +159,11 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo,
nodeinfo->cpus = 0; nodeinfo->mhz = 0; - nodeinfo->nodes = nodeinfo->cores = 1; + nodeinfo->cores = 1; + if (numa_available() < 0) + nodeinfo->nodes = 1; + else + nodeinfo->nodes = numa_max_node() + 1;
/* NB: It is impossible to fill our nodes, since cpuinfo * has no knowledge of NUMA nodes */
ACK,
Thanks, I've pushed this now. -- Chris Lalancette
On Fri, Mar 12, 2010 at 08:45:23AM -0500, Chris Lalancette wrote:
On 03/12/2010 06:32 AM, Daniel Veillard wrote:
On Thu, Mar 11, 2010 at 06:00:56PM -0500, Chris Lalancette wrote:
The nodeGetInfo code was always assuming that machine had a single NUMA node, which is not correct. The good news is that libnuma gives us this information pretty easily, so let's properly report it.
okay
NOTE: With recent hardware starting to support CPU hot-add and hot-remove, both this code and the nodeCapsInitNUMA() code are quickly going to become obsolete. We'll have to think of a more dynamic solution for dealing with NUMA nodes and CPUs that can come and go at will.
well it makes little sense to refresh all the time, I would expect some kind of signal we can hook on to detect change in topology or capacities, though I don't see anything on signal.h for this
Well, it's actually not horrible to refresh all of the time. The way we get this information is all in memory (from /proc or /sys), so it's a very fast operation. That being said, if we can get a signal when things change, and just react to that, that's probably the best way to go. This stuff is all very new, so things are still being put into place; we'll just have to figure out how to be more dynamic in the future.
Jes Sorensen suggested that the hotplug daemon could somehow be tweaked to advertize when things changed. But this is all suppositions at this point, and might be hard to make portable accros distros :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi Chris, On Thu, 11 Mar 2010 18:00:56 -0500 Chris Lalancette <clalance@redhat.com> wrote:
The nodeGetInfo code was always assuming that machine had a single NUMA node, which is not correct. The good news is that libnuma gives us this information pretty easily, so let's properly report it.
NOTE: With recent hardware starting to support CPU hot-add and hot-remove, both this code and the nodeCapsInitNUMA() code are quickly going to become obsolete. We'll have to think of a more dynamic solution for dealing with NUMA nodes and CPUs that can come and go at will.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/nodeinfo.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0748602..8d7e055 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -159,7 +159,11 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo,
nodeinfo->cpus = 0; nodeinfo->mhz = 0; - nodeinfo->nodes = nodeinfo->cores = 1; + nodeinfo->cores = 1; + if (numa_available() < 0) + nodeinfo->nodes = 1; + else + nodeinfo->nodes = numa_max_node() + 1;
/* NB: It is impossible to fill our nodes, since cpuinfo * has no knowledge of NUMA nodes */
This commit breaks the build for me with the following message: """ cc1: warnings being treated as errors nodeinfo.c: In function 'linuxNodeInfoCPUPopulate': nodeinfo.c:163: error: implicit declaration of function 'numa_available' nodeinfo.c:163: error: nested extern declaration of 'numa_available' nodeinfo.c:166: error: implicit declaration of function 'numa_max_node' nodeinfo.c:166: error: nested extern declaration of 'numa_max_node' make[3]: *** [libvirt_driver_la-nodeinfo.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... ... """ The code in this file that uses the same functions seems to be protected by the HAVE_NUMACTL macro.
On 03/12/2010 10:10 AM, Luiz Capitulino wrote:
Hi Chris,
On Thu, 11 Mar 2010 18:00:56 -0500 Chris Lalancette <clalance@redhat.com> wrote:
The nodeGetInfo code was always assuming that machine had a single NUMA node, which is not correct. The good news is that libnuma gives us this information pretty easily, so let's properly report it.
NOTE: With recent hardware starting to support CPU hot-add and hot-remove, both this code and the nodeCapsInitNUMA() code are quickly going to become obsolete. We'll have to think of a more dynamic solution for dealing with NUMA nodes and CPUs that can come and go at will.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/nodeinfo.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0748602..8d7e055 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -159,7 +159,11 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo,
nodeinfo->cpus = 0; nodeinfo->mhz = 0; - nodeinfo->nodes = nodeinfo->cores = 1; + nodeinfo->cores = 1; + if (numa_available() < 0) + nodeinfo->nodes = 1; + else + nodeinfo->nodes = numa_max_node() + 1;
/* NB: It is impossible to fill our nodes, since cpuinfo * has no knowledge of NUMA nodes */
This commit breaks the build for me with the following message:
""" cc1: warnings being treated as errors nodeinfo.c: In function 'linuxNodeInfoCPUPopulate': nodeinfo.c:163: error: implicit declaration of function 'numa_available' nodeinfo.c:163: error: nested extern declaration of 'numa_available' nodeinfo.c:166: error: implicit declaration of function 'numa_max_node' nodeinfo.c:166: error: nested extern declaration of 'numa_max_node' make[3]: *** [libvirt_driver_la-nodeinfo.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... ... """
The code in this file that uses the same functions seems to be protected by the HAVE_NUMACTL macro.
Yeah, you are right. Sorry about that. Followup patch coming. -- Chris Lalancette
participants (3)
-
Chris Lalancette -
Daniel Veillard -
Luiz Capitulino