[patch] patch for disk I/O stats from /proc/diskstats instead of /proc/stat.

wmmon.wbk.gentoo.patch (text/plain), 4.98 KB, created by wbk on 2012-03-09 06:56:01 UTC

--- 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;