--- wmmon.c 2012-03-09 00:14:32.134711893 -0600 +++ ../../../wmmon.app/wmmon/wmmon.c 2012-03-09 00:10:57.168719764 -0600 @@ -106,6 +106,7 @@ int minus_buffers = 1; /* do we want to FILE *fp_meminfo; FILE *fp_stat; FILE *fp_loadavg; +FILE *fp_diskstats; /* wbk new io stats API */ /* functions */ void usage(void); @@ -231,6 +232,7 @@ void wmmon_routine(int argc, char **argv fp_meminfo = fopen("/proc/meminfo", "r"); fp_loadavg = fopen("/proc/loadavg", "r"); fp_stat = fopen("/proc/stat", "r"); + fp_diskstats = fopen("/proc/diskstats", "r"); if (fp) { fscanf(fp, "%ld", &online_time); @@ -447,7 +449,7 @@ void wmmon_routine(int argc, char **argv } } - usleep(250000L); + usleep(250000L); /* wbk - may need tweaking */ } } @@ -527,7 +529,11 @@ void update_stat_mem(stat_dev *st, stat_ void get_statistics(char *devname, long *is, long *ds, long *idle) { int i; - char temp[128]; + /*char temp[128];*/ + + static char *line = NULL; + static size_t line_size = 0; + char *p; char *tokens = " \t\n"; float f; @@ -539,9 +545,9 @@ void get_statistics(char *devname, long if (!strncmp(devname, "cpu", 3)) { fseek(fp_stat, 0, SEEK_SET); - while (fgets(temp, 128, fp_stat)) { - if (strstr(temp, "cpu")) { - p = strtok(temp, tokens); + while ((getline(&line, &line_size, fp_stat)) > 0) { + if (strstr(line, "cpu")) { + p = strtok(line, tokens); /* 1..3, 4 == idle, we don't want idle! */ for (i=0; i<3; i++) { p = strtok(NULL, tokens); @@ -551,22 +557,62 @@ void get_statistics(char *devname, long *idle = atol(p); } } - fp_loadavg = freopen("/proc/loadavg", "r", fp_loadavg); + /* wbk - fopen() might be more appropriate. */ + if ((fp_loadavg = freopen("/proc/loadavg", "r", fp_loadavg)) == NULL) + perror("ger_statistics(): freopen(proc/loadavg) failed!\n"); fscanf(fp_loadavg, "%f", &f); *is = (long) (100 * f); } if (!strncmp(devname, "i/o", 3)) { + /* TODO: Do we need an freopen() call here? Seem to be losing + * our open file, breaking graph after hibernation */ - fseek(fp_stat, 0, SEEK_SET); - while (fgets(temp, 128, fp_stat)) { - if (strstr(temp, "disk_rio") || strstr(temp, "disk_wio")) { - p = strtok(temp, tokens); - /* 1..4 */ - for (i=0; i<4; i++) { + if (fseek(fp_diskstats, 0, SEEK_SET) == -1) + perror("get_statistics() seek failed\n"); + + /*while (fgets(temp, 128, fp_diskstats)) {*/ + while ((getline(&line, &line_size, fp_diskstats)) > 0 ) { + /* wbk 20120308 WORK IN PROGRESS: This is a band-aid fix to * + * restore original functionality. I think there are some * + * fundamental performance/stability issues in this whole * + * function, partly due to reliance on /procfs string parsing * + * in what I suspect is a critical loop, partly due to lack of * + * error handling. Might benefit from a rewrite, but I can't * + * invest the time needed to test such a change at the moment. * + * * + /* These are no longer in /proc/stat. /proc/diskstats seems to * + * be the closest replacement. Under modern BSD's, /proc is now * + * deprecated, so iostat() might be the answer. */ + /* http://www.gossamer-threads.com/lists/linux/kernel/314618 * + * has good info on this being removed from kernel. Also see * + * kernel sources Documentation/iostats.txt */ + /* */ + /* do we need an freopen()? Why didn't /proc/stat/ have one? */ + /* TODO: We will end up with doubled values. We are adding the * + * aggregate to the individual partition, due to device * + * selection logic. Either grab devices' stats with numbers, or * + * without (sda OR sda[1..10]. Could use strstr() return plus * + * offset, but would have to be careful with bounds checking * + * since we're in a limited buffer. Or just divide by 2. * + * (inefficient). Shouldn't matter for graphing (we care about * + * proportions, not numbers.) */ + + /* NOTE: It seems this strstr() would occasionally fail to * + * detect its target due to buffer boundary overlap. */ + if (strstr(line, "sd") || strstr(line, "sr")) { + p = strtok(line, tokens); + for (i=1; i<=6; i++) /* skip 3 tokens, then use fields from*/ p = strtok(NULL, tokens); - *ds += atol(p); - } + *ds += atol(p); + for (i=7; i<=10; i++) /* linux/Documentation/iostats.txt */ + p = strtok(NULL, tokens); + *ds += atol(p); + /* field 11 looks tailor made for a simple load monitor. In + * practice, it doesn't show much unless the system is hammered. */ + /*for (i=1; i<14; i++) + p = strtok(NULL, tokens); + *ds += atol(p);*/ } } if (*ds > maxdiskio) maxdiskio = *ds;