11:15 a.m.
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
11:15 a.m.
New subject: [libvirt] [PATCH] * src/nodeinfo.c: Read nodeinfo->mhz from cpufreq device file, if detected.
---
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
10:02 a.m.
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5408
days inactive
5410
days old
2 comments
2 participants
participants (2)
-
Daniel Veillard -
Thomas Treutner