This patch provides a rewrite of how /proc/cpuinfo is parsed in
common_jag.c, as the original code made the incorrect assumption that
cpuinfo follows a sane format across architectures ;-)
The motivation for this patch is that the original code was producing
stack smashing on a POWER7 running RHEL6.4 Red Hat adds
-fstack-protector along with a lot of other protective CFLAGS when
building RPMs. The code ran okay with -fno-stack-protector, but that is
not the best work-around.
So, the relevant /proc/cpuinfo line on an Intel (Xeon X5675) system
looks like:
cpu MHz : 3066.915
Whereas the relevant line in a POWER7 system is
clock : 3550.000000MHz
My patch replaces the assumption that the relevant number starts 11
characters into the string with another assumption: That the relevant
number starts two characters after a colon in a string that matches
(M|G)Hz.
All in all, the function has a few more calls, which may be a
performance issue if it has to be called multiple times, but since the
section I edited only gets evaluated if we don't know the cpu frequency,
getting it right will actually result in fewer string operations and
unnecessary opens of /proc/cpuinfo for systems likewise affected.
Finally, I also read the actual value into a double and multiply it up
to the size indicated by the suffix, so we end up with KHz? It was
unclear what the original code intended, since it matched both MHz and
GHz, replaced the decimal point with a zero, and read the result as an int.
--
Chandler Wilkerson
Center for Research Computing
Rice University
diff -up slurm-14.11.8/src/plugins/jobacct_gather/common/common_jag.c.orig slurm-14.11.8/src/plugins/jobacct_gather/common/common_jag.c
--- slurm-14.11.8/src/plugins/jobacct_gather/common/common_jag.c.orig 2015-07-21 10:58:26.325499406 -0500
+++ slurm-14.11.8/src/plugins/jobacct_gather/common/common_jag.c 2015-07-21 11:07:36.394333559 -0500
@@ -81,20 +81,6 @@ static uint32_t _update_weighted_freq(st
return thisfreq;
}
-static char *_skipdot (char *str)
-{
- int pntr = 0;
- while (str[pntr]) {
- if (str[pntr] == '.') {
- str[pntr] = '0';
- break;
- }
- pntr++;
- }
- str[pntr+3] = '\0';
- return str;
-}
-
static int _get_sys_interface_freq_line(uint32_t cpu, char *filename,
char * sbuf)
{
@@ -102,7 +88,8 @@ static int _get_sys_interface_freq_line(
FILE *sys_fp = NULL;
char freq_file[80];
char cpunfo_line [128];
- char cpufreq_line [10];
+ char *cpufreq_line = NULL;
+ double cpufreq_value;
if (cpunfo_frequency)
/* scaling not enabled, static freq obtained */
@@ -131,14 +118,22 @@ static int _get_sys_interface_freq_line(
if ((sys_fp = fopen(freq_file, "r")) != NULL) {
while (fgets(cpunfo_line, sizeof cpunfo_line,
sys_fp) != NULL) {
- if (strstr(cpunfo_line, "cpu MHz") ||
- strstr(cpunfo_line, "cpu GHz")) {
- break;
+ if (strstr(cpunfo_line, "MHz")) {
+ cpunfo_frequency = 1000;
+ }
+ if (strstr(cpunfo_line, "GHz")) {
+ cpunfo_frequency = 1000000;
+ }
+ if (cpunfo_frequency) {
+ cpufreq_line = strchr(cpunfo_line, ':');
+ if(sscanf(cpufreq_line+2, "%lf", &cpufreq_value)) {
+ cpunfo_frequency = (int)(cpufreq_value * cpunfo_frequency);
+ break;
+ } else {
+ cpunfo_frequency = 0;
+ }
}
}
- strncpy(cpufreq_line, cpunfo_line+11, 8);
- _skipdot(cpufreq_line);
- sscanf(cpufreq_line, "%d", &cpunfo_frequency);
debug2("cpunfo_frequency= %d",cpunfo_frequency);
fclose(sys_fp);
}
