[libvirt] Fix wrong nodeinfo->mhz when cpufreq is enabled
Hi, I've written a little patch to fix wrong nodeinfo->mhz when the Linux kernel module cpufreq and a typical governor like ondemand are loaded. nodeinfo->mhz is then too low as libvirt just reads /proc/cpuinfo, entry "cpu MHz". This patch reads /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq if existing and corrects nodeinfo->mhz if the value found is higher. Using cpu0 is a crude hack but as discussed on IRC, considering different governors/maximum frequencies per cpu/core would require changing the nodeinfo struct, which would break the API. Some more info: https://bugzilla.redhat.com/show_bug.cgi?id=559762 Please have a careful look at my patch, my last C is quite some time ago. I tried to stick to linuxNodeInfoCPUPopulate, but f.e., checking for "&& *p == '\0'" after virStrToLong_ui doesn't work and I'm wondering why scaling_max_freq may not be terminated with by NUL? I also hope the file handling is correct, but please have a look. There is still some todo: Some logging would be helpful, maybe one of you could point out which function best to use? This is my first patch, so hopefully it follows in the next mail ;-) kr, tom
--- src/nodeinfo.c | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 7d26b2b..15877ed 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -55,11 +55,14 @@ #ifdef __linux__ #define CPUINFO_PATH "/proc/cpuinfo" +#define SCALINGMAXFREQ_PATH "/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq" /* NB, these are not static as we need to call them from testsuite */ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr nodeinfo); +int linuxNodeInfoConsiderCPUScaling(FILE *scalingmaxfreq, virNodeInfoPtr nodeinfo); + int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr nodeinfo) { char line[1024]; @@ -132,6 +135,27 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr n return 0; } +int linuxNodeInfoConsiderCPUScaling(FILE *scalingmaxfreq, virNodeInfoPtr nodeinfo) { + char line[1024]; + + while (fgets(line, sizeof(line), scalingmaxfreq) != NULL) { + char *buf = line; + char *p; + unsigned int khz; + + if (virStrToLong_ui(buf, &p, 10, &khz) == 0) + { + if(khz/1000 > nodeinfo->mhz) + { + nodeinfo->mhz = khz/1000; + return 0; + } + } + } + return 1; +} + + #endif int nodeGetInfo(virConnectPtr conn, @@ -164,6 +188,18 @@ int nodeGetInfo(virConnectPtr conn, if (ret < 0) return -1; + FILE *scalingmaxfreq = fopen(SCALINGMAXFREQ_PATH, "r"); + if (scalingmaxfreq != NULL) { + // TODO: some logging information that cpufreq was detected? + ret = linuxNodeInfoConsiderCPUScaling(scalingmaxfreq, nodeinfo); + if(ret == 0) { + // TODO: logging: nodeinfo->mhz was updated + } else if(ret == 1) { + // TODO: logging: cpufreq was detected, but information available didn't make sense + } + fclose(scalingmaxfreq); + } + /* Convert to KB. */ nodeinfo->memory = physmem_total () / 1024; -- 1.5.4.3
On Tue, Feb 02, 2010 at 06:15:50PM +0100, Thomas Treutner wrote:
Hi,
I've written a little patch to fix wrong nodeinfo->mhz when the Linux kernel module cpufreq and a typical governor like ondemand are loaded. nodeinfo->mhz is then too low as libvirt just reads /proc/cpuinfo, entry "cpu MHz". This patch reads
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
if existing and corrects nodeinfo->mhz if the value found is higher. Using cpu0 is a crude hack but as discussed on IRC, considering different governors/maximum frequencies per cpu/core would require changing the nodeinfo struct, which would break the API.
Right, just checked on my laptop (idle) and got cpu MHz : 800.000 while /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq indicates a 2.2GHz model, we really need to fix this
Some more info: https://bugzilla.redhat.com/show_bug.cgi?id=559762
Please have a careful look at my patch, my last C is quite some time ago. I tried to stick to linuxNodeInfoCPUPopulate, but f.e., checking for "&& *p == '\0'" after virStrToLong_ui doesn't work and I'm wondering why scaling_max_freq may not be terminated with by NUL? I also hope the file handling is correct, but please have a look.
There is still some todo: Some logging would be helpful, maybe one of you could point out which function best to use?
This is my first patch, so hopefully it follows in the next mail ;-)
Okay, we will review but this clearly need to be fixed :-) thanks ! 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/
participants (2)
-
Daniel Veillard -
Thomas Treutner