rewrite linux diskio code

Instead of using a hardcoded maximum number of slots for
stats of different disks, use a linked list. Also since the algorithm to
update each device's counters is the same for updating the totals, share
equal code, which in my eyes not only saves a bunch of LoC, but also
drastically increases readability.

diff --git a/src/common.c b/src/common.c
index 5f3320a..a9aa2d9 100644 (file)
--- a/src/common.c
+++ b/src/common.c
@@ -33,6 +33,7 @@
 #include <errno.h>
 #include <sys/time.h>
 #include <pthread.h>
+#include "diskio.h"
 
 /* check for OS and include appropriate headers */
 #if defined(__linux__)

diff --git a/src/common.h b/src/common.h
index 057cab0..9f3977c 100644 (file)
--- a/src/common.h
+++ b/src/common.h
@@ -7,7 +7,6 @@
 #include <sys/socket.h>
 
 int check_mount(char *s);
-void update_diskio(void);
 void prepare_update(void);
 void update_uptime(void);
 void update_meminfo(void);
@@ -83,7 +82,6 @@ double get_sysfs_info(int *fd, int arg, char *devtype, char *type);
 
 void get_adt746x_cpu(char *, size_t);
 void get_adt746x_fan(char *, size_t);
-unsigned int get_diskio(void);
 
 int open_acpi_temperature(const char *name);
 double get_acpi_temperature(int fd);

diff --git a/src/diskio.c b/src/diskio.c
index b805ec7..6865710 100644 (file)
--- a/src/diskio.c
+++ b/src/diskio.c
@@ -48,79 +48,91 @@
 #define NBD_MAJOR 43
 #endif
 
-static struct diskio_stat diskio_stats_[MAX_DISKIO_STATS];
-struct diskio_stat *diskio_stats = diskio_stats_;
+/* this is the root of all per disk stats,
+ * also containing the totals. */
+static struct diskio_stat stats = {
+       .next = NULL,
+       .current = 0,
+       .current_read = 0,
+       .current_write = 0,
+       .last = UINT_MAX,
+       .last_read = UINT_MAX,
+       .last_write = UINT_MAX,
+};
 
 void clear_diskio_stats(void)
 {
-       unsigned i;
-       for(i = 1; i < MAX_DISKIO_STATS; i++) {
-               if (diskio_stats[i].dev) {
-                       free(diskio_stats[i].dev);
-                       diskio_stats[i].dev = 0;
-               }
+       struct diskio_stat *cur;
+       while (stats.next) {
+               cur = stats.next;
+               stats.next = stats.next->next;
+               free(cur);
        }
 }
 
 struct diskio_stat *prepare_diskio_stat(const char *s)
 {
-       struct diskio_stat *new = 0;
-       unsigned i;
+       struct diskio_stat *cur = &stats;
 
        if (!s)
-               return &diskio_stats[0];
+               return &stats;
 
-       /* lookup existing or get new */
-       for (i = 1; i < MAX_DISKIO_STATS; i++) {
-               if (diskio_stats[i].dev) {
-                       if (strcmp(diskio_stats[i].dev, s) == 0) {
-                               return &diskio_stats[i];
-                       }
-               } else {
-                       new = &diskio_stats[i];
-                       break;
-               }
+       /* lookup existing */
+       while (cur->next) {
+               cur = cur->next;
+               if (!strcmp(cur->dev, s))
+                       return cur;
        }
-       /* new dev */
-       if (!new) {
-               ERR("too many diskio stats");
-               return 0;
-       }
-       if (new->dev) {
-               free(new->dev);
-               new->dev = 0;
+
+       /* no existing found, make a new one */
+       cur->next = malloc(sizeof(struct diskio_stat));
+       cur = cur->next;
+       memset(cur, 0, sizeof(struct diskio_stat));
+       cur->dev = strndup(s, text_buffer_size);
+       cur->last = UINT_MAX;
+       cur->last_read = UINT_MAX;
+       cur->last_write = UINT_MAX;
+       return cur;
+}
+
+static void update_diskio_values(struct diskio_stat *ds,
+               unsigned int reads, unsigned int writes)
+{
+       if (reads < ds->last_read || writes < ds->last_write) {
+               /* counter overflow or reset - rebase to sane values */
+               ds->last = 0;
+               ds->last_read = 0;
+               ds->last_write = 0;
        }
-       new->dev = strndup(s, text_buffer_size);
-       new->current = 0;
-       new->current_read = 0;
-       new ->current_write = 0;
-       new->last = UINT_MAX;
-       new->last_read = UINT_MAX;
-       new->last_write = UINT_MAX;
-       return new;
+       /* since the values in /proc/diskstats are absolute, we have to substract
+        * our last reading. The numbers stand for "sectors read", and we therefore
+        * have to divide by two to get KB */
+       ds->current_read = (reads - ds->last_read) / 2;
+       ds->current_write = (writes - ds->last_write) / 2;
+       ds->current = ds->current_read + ds->current_write;
+
+       ds->last_read = reads;
+       ds->last_write = writes;
+       ds->last = ds->last_read + ds->last_write;
 }
 
 void update_diskio(void)
 {
-       static unsigned int last = UINT_MAX;
-       static unsigned int last_read = UINT_MAX;
-       static unsigned int last_write = UINT_MAX;
        FILE *fp;
        static int rep = 0;
 
+       struct diskio_stat *cur;
        char buf[512], devbuf[64];
-       int i;
        unsigned int major, minor;
-       unsigned int current = 0;
-       unsigned int current_read = 0;
-       unsigned int current_write = 0;
-       unsigned int reads, writes = 0;
+       unsigned int reads, writes;
+       unsigned int total_reads, total_writes;
        int col_count = 0;
-       int tot, tot_read, tot_write;
 
-       if (!(fp = open_file("/proc/diskstats", &rep))) {
+       stats.current = 0;
+       stats.current_read = 0;
+       stats.current_write = 0;
 
-               diskio_stats[0].current = 0;
+       if (!(fp = open_file("/proc/diskstats", &rep))) {
                return;
        }
 
@@ -135,9 +147,8 @@ void update_diskio(void)
                 * XXX: ignore devices which are part of a SW RAID (MD_MAJOR) */
                if (col_count == 5 && major != LVM_BLK_MAJOR && major != NBD_MAJOR
                                && major != RAMDISK_MAJOR && major != LOOP_MAJOR) {
-                       current += reads + writes;
-                       current_read += reads;
-                       current_write += writes;
+                       total_reads += reads;
+                       total_writes += writes;
                } else {
                        col_count = sscanf(buf, "%u %u %s %*u %u %*u %u",
                                &major, &minor, devbuf, &reads, &writes);
@@ -145,60 +156,14 @@ void update_diskio(void)
                                continue;
                        }
                }
-               for (i = 1; i < MAX_DISKIO_STATS; i++) {
-                       if (diskio_stats[i].dev && !strcmp(devbuf, diskio_stats[i].dev)) {
-                               diskio_stats[i].current =
-                                       (reads + writes - diskio_stats[i].last) / 2;
-                               diskio_stats[i].current_read =
-                                       (reads - diskio_stats[i].last_read) / 2;
-                               diskio_stats[i].current_write =
-                                       (writes - diskio_stats[i].last_write) / 2;
-                               if (reads + writes < diskio_stats[i].last) {
-                                       diskio_stats[i].current = 0;
-                               }
-                               if (reads < diskio_stats[i].last_read) {
-                                       diskio_stats[i].current_read = 0;
-                                       diskio_stats[i].current = diskio_stats[i].current_write;
-                               }
-                               if (writes < diskio_stats[i].last_write) {
-                                       diskio_stats[i].current_write = 0;
-                                       diskio_stats[i].current = diskio_stats[i].current_read;
-                               }
-                               diskio_stats[i].last = reads + writes;
-                               diskio_stats[i].last_read = reads;
-                               diskio_stats[i].last_write = writes;
-                       }
-               }
-       }
-
-       /* since the values in /proc/diststats are absolute, we have to substract
-        * our last reading. The numbers stand for "sectors read", and we therefore
-        * have to divide by two to get KB */
-       tot = ((double) (current - last) / 2);
-       tot_read = ((double) (current_read - last_read) / 2);
-       tot_write = ((double) (current_write - last_write) / 2);
-
-       if (last_read > current_read) {
-               tot_read = 0;
-       }
-       if (last_write > current_write) {
-               tot_write = 0;
-       }
+               cur = stats.next;
+               while (cur && strcmp(devbuf, cur->dev))
+                       cur = cur->next;
 
-       if (last > current) {
-               /* we hit this either if it's the very first time we run this, or
-                * when /proc/diskstats overflows; while 0 is not correct, it's at
-                * least not way off */
-               tot = 0;
+               if (cur)
+                       update_diskio_values(cur, reads, writes);
        }
-       last = current;
-       last_read = current_read;
-       last_write = current_write;
-
-       diskio_stats[0].current = tot;
-       diskio_stats[0].current_read = tot_read;
-       diskio_stats[0].current_write = tot_write;
-
+       update_diskio_values(&stats, total_reads, total_writes);
        fclose(fp);
 }
 

diff --git a/src/diskio.h b/src/diskio.h
index 8bd5dd2..f2f8746 100644 (file)
--- a/src/diskio.h
+++ b/src/diskio.h
@@ -30,16 +30,18 @@
 #define DISKIO_H_
 
 struct diskio_stat {
+       struct diskio_stat *next;
        char *dev;
-       unsigned int current, current_read, current_write, last, last_read,
-               last_write;
+       unsigned int current;
+       unsigned int current_read;
+       unsigned int current_write;
+       unsigned int last;
+       unsigned int last_read;
+       unsigned int last_write;
 };
 
-#define MAX_DISKIO_STATS 64
-
-struct diskio_stat *diskio_stats;
-
 struct diskio_stat *prepare_diskio_stat(const char *s);
+void update_diskio(void);
 void clear_diskio_stats(void);
 
 #endif /* DISKIO_H_ */