From 4161f90c6fd53bb73bebfc8b75fac171ab06332f Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Thu, 25 Dec 2008 16:36:29 +0100 Subject: [PATCH] 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. --- src/common.c | 1 + src/common.h | 2 - src/diskio.c | 171 +++++++++++++++++++++++----------------------------------- src/diskio.h | 14 ++--- 4 files changed, 77 insertions(+), 111 deletions(-) diff --git a/src/common.c b/src/common.c index 5f3320a..a9aa2d9 100644 --- a/src/common.c +++ b/src/common.c @@ -33,6 +33,7 @@ #include #include #include +#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 --- a/src/common.h +++ b/src/common.h @@ -7,7 +7,6 @@ #include 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 --- 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 --- 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_ */ -- 1.7.9.5