[sigar] Get CPU counts from /proc/cpuinfo
Zane Bitter
zaneb at fedoraproject.org
Wed Sep 7 16:37:05 UTC 2011
commit 956474638056578aabf7bfc4bed50c5de3c992f5
Author: Zane Bitter <zbitter at redhat.com>
Date: Mon Aug 29 18:03:27 2011 +0200
Get CPU counts from /proc/cpuinfo
bz714249-1-cpu-count.patch | 197 ++++++++++++++++++++++++++++++++++++++++++++
sigar.spec | 10 ++-
2 files changed, 206 insertions(+), 1 deletions(-)
---
diff --git a/bz714249-1-cpu-count.patch b/bz714249-1-cpu-count.patch
new file mode 100644
index 0000000..0d9a1e0
--- /dev/null
+++ b/bz714249-1-cpu-count.patch
@@ -0,0 +1,197 @@
+commit cae27197baf656ae0d7734913d8714995a787ee4
+Author: Zane Bitter <zbitter at redhat.com>
+Date: Wed Aug 17 15:35:15 2011 +0200
+
+ Correct reporting of number of CPU cores on Linux
+
+ There are a number of problems with the implementation of
+ sigar_cpu_info_list_get() on Linux:
+
+ * The call sysconf(_SC_NPROCESSORS_CONF) returns the number of processors
+ in the system, not the number of cores (i.e. in a hyperthreaded
+ system, sigar_cpu_total_count() gives the number of threads, not cores).
+ * It turns out that the CPUINFO instruction on x86 is not a reliable way of
+ getting the number of cores, or even the number of threads (which is what
+ sigar_cpu_core_count() is actually trying to read).
+ * core_rollup ignores all but one processor per socket, on the assumption
+ that entries in /proc/cpuinfo appear grouped by socket. This assumption
+ is not correct.
+ * The get_cpuinfo_max_freq() function retrieves data corresponding to the
+ wrong processor in the presence of the core_rollup option (although the
+ practical effect is cancelled out by the previous issue, purely by
+ coincidence).
+
+ The solution is to obtain all information required about the processors
+ from the kernel via the /proc/cpuinfo file. This allows us to return the
+ actual number of cores, rather than threads, in the cores_per_socket and
+ total_cores variables. It also allows us to return the correct number of
+ sockets in the system, which was broken by a combination of the first two
+ issues above. Theoretically, all of this would work even in an asymmetric
+ system with different numbers of cores in each socket. It should work even
+ if the processors listed in /proc/cpuinfo are in arbitrary order, even in
+ the presence of the core_rollup option.
+
+ Signed-off-by: Zane Bitter <zbitter at redhat.com>
+
+diff --git a/src/os/linux/linux_sigar.c b/src/os/linux/linux_sigar.c
+index 0e4283f..e65abd6 100644
+--- a/src/os/linux/linux_sigar.c
++++ b/src/os/linux/linux_sigar.c
+@@ -46,6 +46,13 @@
+ #define PROC_PARTITIONS PROC_FS_ROOT "partitions"
+ #define PROC_DISKSTATS PROC_FS_ROOT "diskstats"
+
++typedef struct {
++ sigar_cpu_info_t cpu;
++ int socket;
++ int core;
++ int processor;
++} processor_info_t;
++
+ /*
+ * /proc/self/stat fields:
+ * 1 - pid
+@@ -1499,16 +1506,15 @@ static SIGAR_INLINE void cpu_info_strcpy(char *ptr, char *buf, int len)
+ }
+ }
+
+-static int get_cpu_info(sigar_t *sigar, sigar_cpu_info_t *info,
++static int get_cpu_info(sigar_t *sigar, processor_info_t *proc,
+ FILE *fp)
+ {
+ char buffer[BUFSIZ], *ptr;
++ sigar_cpu_info_t *info = &proc->cpu;
+
+ int found = 0;
+
+- /* UML vm wont have "cpu MHz" or "cache size" fields */
+- info->mhz = 0;
+- info->cache_size = 0;
++ memset(proc, 0, sizeof(*proc));
+
+ #ifdef __powerpc64__
+ SIGAR_SSTRCPY(info->vendor, "IBM");
+@@ -1520,6 +1526,10 @@ static int get_cpu_info(sigar_t *sigar, sigar_cpu_info_t *info,
+ if (strnEQ(ptr, "processor", 9)) {
+ found = 1;
+ }
++ else if (strnEQ(ptr, "physical id", 11)) {
++ ptr = cpu_info_strval(ptr);
++ proc->socket = sigar_strtoul(ptr);
++ }
+ break;
+ case 'v':
+ /* "vendor_id" or "vendor" */
+@@ -1555,6 +1565,14 @@ static int get_cpu_info(sigar_t *sigar, sigar_cpu_info_t *info,
+ ptr = cpu_info_strval(ptr);
+ info->cache_size = sigar_strtoul(ptr);
+ }
++ else if (strnEQ(ptr, "core id", 7)) {
++ ptr = cpu_info_strval(ptr);
++ proc->core = sigar_strtoul(ptr);
++ }
++ else if (strnEQ(ptr, "cpu cores", 9)) {
++ ptr = cpu_info_strval(ptr);
++ info->cores_per_socket = sigar_strtoul(ptr);
++ }
+ #ifdef __powerpc64__
+ /* each /proc/cpuinfo entry looks like so:
+ * processor : 0
+@@ -1618,11 +1636,55 @@ static void get_cpuinfo_min_freq(sigar_cpu_info_t *cpu_info, int num)
+ }
+ }
+
++typedef struct {
++ size_t size;
++ sigar_uint32_t *data;
++} bitmap_t;
++
++static int bitmap_check(sigar_t *sigar, bitmap_t *bitmap, unsigned int index)
++{
++ int ret = 0;
++ size_t i = index / 32;
++ if (i >= bitmap->size) {
++ void *new_data;
++ size_t old_size = bitmap->size;
++
++ bitmap->size *= 2;
++ if (i >= bitmap->size) {
++ bitmap->size = i + 1;
++ }
++
++ new_data = realloc(bitmap->data,
++ bitmap->size * sizeof(sigar_uint32_t));
++
++ if (new_data) {
++ bitmap->data = new_data;
++ memset(&bitmap->data[old_size], 0,
++ (bitmap->size - old_size) * sizeof(sigar_uint32_t));
++ } else {
++ // Out of memory. This could result in counting a single socket
++ // multiple times if the physical id is huge.
++ sigar_log_printf(sigar, SIGAR_LOG_WARN,
++ "Cannot allocate CPU socket bitmap");
++ return 0;
++ }
++ } else {
++ ret = (bitmap->data[i] & (1 << (index % 32))) != 0;
++ }
++
++ bitmap->data[i] |= 1 << (index % 32);
++
++ return ret;
++}
++
+ int sigar_cpu_info_list_get(sigar_t *sigar,
+ sigar_cpu_info_list_t *cpu_infos)
+ {
+ FILE *fp;
+ int core_rollup = sigar_cpu_core_rollup(sigar), i=0;
++ processor_info_t processor;
++ int total_sockets = 0, total_cores = 0;
++ bitmap_t sockets_found = {};
+
+ if (!(fp = fopen(PROC_FS_ROOT "cpuinfo", "r"))) {
+ return errno;
+@@ -1631,26 +1693,33 @@ int sigar_cpu_info_list_get(sigar_t *sigar,
+ (void)sigar_cpu_total_count(sigar);
+ sigar_cpu_info_list_create(cpu_infos);
+
+- while (get_cpu_info(sigar, &cpu_infos->data[cpu_infos->number], fp)) {
++ while (get_cpu_info(sigar, &processor, fp)) {
+ sigar_cpu_info_t *info;
+
+- if (core_rollup && (i++ % sigar->lcpu)) {
+- continue; /* fold logical processors */
++ if (!bitmap_check(sigar, &sockets_found, processor.socket)) {
++ total_sockets++;
++ total_cores += processor.cpu.cores_per_socket;
++ } else if (core_rollup) {
++ continue;
+ }
+
+ info = &cpu_infos->data[cpu_infos->number];
+- get_cpuinfo_max_freq(info, cpu_infos->number);
+- get_cpuinfo_min_freq(info, cpu_infos->number);
+
+- info->total_cores = sigar->ncpu;
+- info->cores_per_socket = sigar->lcpu;
+- info->total_sockets = sigar_cpu_socket_count(sigar);
++ memcpy(info, &processor.cpu, sizeof(*info));
++ get_cpuinfo_max_freq(info, processor.processor);
++ get_cpuinfo_min_freq(info, processor.processor);
+
+ ++cpu_infos->number;
+ SIGAR_CPU_INFO_LIST_GROW(cpu_infos);
+ }
+
+ fclose(fp);
++ free(sockets_found.data);
++
++ for (i = 0; i < cpu_infos->number; i++) {
++ cpu_infos->data[i].total_cores = total_cores;
++ cpu_infos->data[i].total_sockets = total_sockets;
++ }
+
+ return SIGAR_OK;
+ }
diff --git a/sigar.spec b/sigar.spec
index aab799e..fac0c32 100644
--- a/sigar.spec
+++ b/sigar.spec
@@ -1,6 +1,6 @@
Name: sigar
Version: 1.6.5
-Release: 0.3.git58097d9%{?dist}
+Release: 0.4.git58097d9%{?dist}
Summary: System Information Gatherer And Reporter
%global sigar_suffix 0-g4b67f57
@@ -25,6 +25,8 @@ BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
BuildRequires: gcc cmake
+Patch100: bz714249-1-cpu-count.patch
+
%description
The Sigar API provides a portable interface for gathering system
information such as:
@@ -56,6 +58,8 @@ Header files for developing against the Sigar API
# setup -q -n hyperic-{name}-{sigar_hash}
%setup -q -n %{name}-%{version}
+%patch100 -p1 -b .bz714249
+
%build
# Fix lib directory
@@ -92,6 +96,10 @@ rm -rf $RPM_BUILD_ROOT
%doc LICENSE NOTICE AUTHORS
%changelog
+* Mon Aug 29 2011 Zane Bitter <zbitter at redhat.com> - 1.6.5-0.4.git833ca18
+- Get CPU counts from /proc/cpuinfo
+ Resolves: #714249
+
* Wed Feb 09 2011 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 1.6.5-0.2.git833ca18
- Rebuilt for https://fedoraproject.org/wiki/Fedora_15_Mass_Rebuild
More information about the scm-commits
mailing list