On Tue, Nov 29, 2011 at 08:26:57PM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
Date: Mon, 3 Oct 2011 05:45:30 -0700
Subject: [PATCH 1/5] Use sysfs to gather host topology, in place of
/proc/cpuinfo
Libvirt at present depends on /proc/cpuinfo to gather host
details such as CPUs, cores, threads, etc. This is an architecture-
dependent approach. An alternative is to use 'Sysfs', which provides
a platform-agnostic interface to parse host CPU topology.
Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com>
---
src/nodeinfo.c | 114 +++++++++++++++++++-------------------------------------
1 files changed, 39 insertions(+), 75 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 6448b79..f70654a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -30,6 +30,7 @@
#include <errno.h>
#include <dirent.h>
#include <sys/utsname.h>
+#include <sched.h>
#if HAVE_NUMACTL
# define NUMA_VERSION1_COMPATIBILITY 1
@@ -67,8 +68,7 @@
/* NB, this is not static as we need to call it from the testsuite */
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- virNodeInfoPtr nodeinfo,
- bool need_hyperthreads);
+ virNodeInfoPtr nodeinfo);
static int linuxNodeGetCPUStats(FILE *procstat,
int cpuNum,
@@ -191,23 +191,26 @@ static int parse_socket(unsigned int cpu)
return ret;
}
+static int parse_core(unsigned int cpu)
+{
+ return get_cpu_value(cpu, "topology/core_id", false);
+}
+
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- virNodeInfoPtr nodeinfo,
- bool need_hyperthreads)
+ virNodeInfoPtr nodeinfo)
{
char line[1024];
DIR *cpudir = NULL;
struct dirent *cpudirent = NULL;
unsigned int cpu;
- unsigned long cur_threads;
- int socket;
- unsigned long long socket_mask = 0;
- unsigned int remaining;
+ unsigned long core, socket, cur_threads;
+ cpu_set_t core_mask;
+ cpu_set_t socket_mask;
int online;
nodeinfo->cpus = 0;
nodeinfo->mhz = 0;
- nodeinfo->cores = 1;
+ nodeinfo->cores = 0;
nodeinfo->nodes = 1;
# if HAVE_NUMACTL
@@ -221,20 +224,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* NOTE: hyperthreads are ignored here; they are parsed out of /sys */
while (fgets(line, sizeof(line), cpuinfo) != NULL) {
char *buf = line;
- if (STRPREFIX(buf, "processor")) { /* aka a single logical CPU */
- buf += 9;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':') {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("parsing cpuinfo
processor"));
- return -1;
- }
- nodeinfo->cpus++;
# if defined(__x86_64__) || \
defined(__amd64__) || \
defined(__i386__)
- } else if (STRPREFIX(buf, "cpu MHz")) {
+ if (STRPREFIX(buf, "cpu MHz")) {
char *p;
unsigned int ui;
buf += 9;
@@ -249,24 +242,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo->mhz = ui;
- } else if (STRPREFIX(buf, "cpu cores")) { /* aka cores */
- char *p;
- unsigned int id;
- buf += 9;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':' || !buf[1]) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("parsing cpuinfo cpu cores %c"), *buf);
- return -1;
- }
- if (virStrToLong_ui(buf+1, &p, 10, &id) == 0
- && (*p == '\0' || c_isspace(*p))
- && id > nodeinfo->cores)
- nodeinfo->cores = id;
# elif defined(__powerpc__) || \
defined(__powerpc64__)
- } else if (STRPREFIX(buf, "clock")) {
+ if (STRPREFIX(buf, "clock")) {
char *p;
unsigned int ui;
buf += 5;
@@ -281,53 +259,30 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || c_isspace(*p)))
nodeinfo->mhz = ui;
-# elif defined(__s390__) || \
- defined(__s390x__)
- } else if (STRPREFIX(buf, "# processors")) {
- char *p;
- unsigned int ui;
- buf += 12;
- while (*buf && c_isspace(*buf))
- buf++;
- if (*buf != ':' || !buf[1]) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("parsing number of processors %c"), *buf);
- return -1;
- }
- if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0
- && (*p == '\0' || c_isspace(*p)))
- nodeinfo->cpus = ui;
/* No other interesting infos are available in /proc/cpuinfo.
* However, there is a line identifying processor's version,
* identification and machine, but we don't want it to be caught
* and parsed in next iteration, because it is not in expected
* format and thus lead to error. */
- break;
# else
# warning Parser for /proc/cpuinfo needs to be adapted for your architecture
# endif
}
}
- if (!nodeinfo->cpus) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("no cpus found"));
- return -1;
- }
-
- if (!need_hyperthreads)
- return 0;
-
- /* OK, we've parsed what we can out of /proc/cpuinfo. Get the socket
- * and thread information from /sys
+ /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the core, socket
+ * thread and topology information from /sys
*/
- remaining = nodeinfo->cpus;
cpudir = opendir(CPU_SYS_PATH);
if (cpudir == NULL) {
virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
return -1;
}
- while ((errno = 0), remaining && (cpudirent = readdir(cpudir))) {
+
+ CPU_ZERO(&core_mask);
+ CPU_ZERO(&socket_mask);
+
+ while ((cpudirent = readdir(cpudir))) {
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
@@ -338,15 +293,19 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
}
if (!online)
continue;
- remaining--;
+ nodeinfo->cpus++;
- socket = parse_socket(cpu);
- if (socket < 0) {
- closedir(cpudir);
- return -1;
+ /* Parse core */
+ core = parse_core(cpu);
+ if (!CPU_ISSET(core, &core_mask)) {
+ CPU_SET(core, &core_mask);
+ nodeinfo->cores++;
}
- if (!(socket_mask & (1 << socket))) {
- socket_mask |= (1 << socket);
+
+ /* Parse socket */
+ socket = parse_socket(cpu);
+ if (!CPU_ISSET(socket, &socket_mask)) {
+ CPU_SET(socket, &socket_mask);
nodeinfo->sockets++;
}
@@ -367,7 +326,12 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
closedir(cpudir);
- /* there should always be at least one socket and one thread */
+ /* there should always be at least one cpu, socket and one thread */
+ if (nodeinfo->cpus == 0) {
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("no CPUs found"));
+ return -1;
+ }
if (nodeinfo->sockets == 0) {
nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("no sockets found"));
@@ -617,7 +581,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
_("cannot open %s"), CPUINFO_PATH);
return -1;
}
- ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo, true);
+ ret = linuxNodeInfoCPUPopulate(cpuinfo, nodeinfo);
VIR_FORCE_FCLOSE(cpuinfo);
if (ret < 0)
return -1;
There's nothing particularly wrong with the idea here, but as Stefan
mentions this has broken the test suite, because the nodeinfotest.c
now tries to read /sys from the test machine, instead of reading
our dummy data files in tests/nodeinfodata/
I think we'd probably need to create some dummy sysfs trees under
tests/nodeinfodata/test1/sys/..../ and make sure we can pass in
a path to the linuxNodeInfoCPUPopulate API so we cna point the
test at our sysfs, instead of the real machine
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 :|