handle dynamically added/removed interrupts or softirqs

Also:
- store RRDDIM pointers to avoid rrddim_find overhead
- fixed some inverted likely/unlikely

Signed-off-by: Rémi Lefèvre <remi.lefevre@parrot.com>

diff --git a/src/proc_interrupts.c b/src/proc_interrupts.c
index 14215de68a24f497f208f72694ddffb60fe692cf..145d72a20fb917b533b46c570a08ab577b243771 100644 (file)
--- a/src/proc_interrupts.c
+++ b/src/proc_interrupts.c
@@ -2,17 +2,23 @@
 
 #define MAX_INTERRUPT_NAME 50
 
+struct cpu_interrupt {
+    unsigned long long value;
+    RRDDIM *rd;
+};
+
 struct interrupt {
     int used;
     char *id;
     char name[MAX_INTERRUPT_NAME + 1];
+    RRDDIM *rd;
     unsigned long long total;
-    unsigned long long value[];
+    struct cpu_interrupt cpu[];
 };
 
 // since each interrupt is variable in size
 // we use this to calculate its record size
-#define recordsize(cpus) (sizeof(struct interrupt) + (cpus * sizeof(unsigned long long)))
+#define recordsize(cpus) (sizeof(struct interrupt) + (cpus * sizeof(struct cpu_interrupt)))
 
 // given a base, get a pointer to each record
 #define irrindex(base, line, cpus) ((struct interrupt *)&((char *)(base))[line * recordsize(cpus)])
@@ -21,8 +27,21 @@ static inline struct interrupt *get_interrupts_array(int lines, int cpus) {
     static struct interrupt *irrs = NULL;
     static int allocated = 0;
 
-    if(unlikely(lines > allocated)) {
+    if(unlikely(lines != allocated)) {
+        uint32_t l;
+        int c;
+
         irrs = (struct interrupt *)reallocz(irrs, lines * recordsize(cpus));
+
+        // reset all interrupt RRDDIM pointers as any line could have shifted
+        for(l = 0; l < lines ;l++) {
+            struct interrupt *irr = irrindex(irrs, l, cpus);
+            irr->rd = NULL;
+            irr->name[0] = '\0';
+            for(c = 0; c < cpus ;c++)
+                irr->cpu[c].rd = NULL;
+        }
+
         allocated = lines;
     }
 
@@ -31,7 +50,6 @@ static inline struct interrupt *get_interrupts_array(int lines, int cpus) {
 
 int do_proc_interrupts(int update_every, unsigned long long dt) {
     (void)dt;
-
     static procfile *ff = NULL;
     static int cpus = -1, do_per_core = -1;
     struct interrupt *irrs = NULL;
@@ -64,7 +82,7 @@ int do_proc_interrupts(int update_every, unsigned long long dt) {
         uint32_t w;
         cpus = 0;
         for(w = 0; w < words ; w++) {
-            if(unlikely(strncmp(procfile_lineword(ff, 0, w), "CPU", 3) == 0))
+            if(likely(strncmp(procfile_lineword(ff, 0, w), "CPU", 3) == 0))
                 cpus++;
         }
     }
@@ -96,12 +114,12 @@ int do_proc_interrupts(int update_every, unsigned long long dt) {
 
         int c;
         for(c = 0; c < cpus ;c++) {
-            if(unlikely((c + 1) < (int)words))
-                irr->value[c] = strtoull(procfile_lineword(ff, l, (uint32_t)(c + 1)), NULL, 10);
+            if(likely((c + 1) < (int)words))
+                irr->cpu[c].value = strtoull(procfile_lineword(ff, l, (uint32_t)(c + 1)), NULL, 10);
             else
-                irr->value[c] = 0;
+                irr->cpu[c].value = 0;
 
-            irr->total += irr->value[c];
+            irr->total += irr->cpu[c].value;
         }
 
         if(unlikely(isdigit(irr->id[0]) && (uint32_t)(cpus + 2) < words)) {
@@ -124,28 +142,37 @@ int do_proc_interrupts(int update_every, unsigned long long dt) {
     // --------------------------------------------------------------------
 
     st = rrdset_find_bytype("system", "interrupts");
-    if(unlikely(!st)) {
-        st = rrdset_create("system", "interrupts", NULL, "interrupts", NULL, "System interrupts", "interrupts/s", 1000, update_every, RRDSET_TYPE_STACKED);
-
-        for(l = 0; l < lines ;l++) {
-            struct interrupt *irr = irrindex(irrs, l, cpus);
-            if(unlikely(!irr->used)) continue;
-            rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
-        }
-    }
+    if(unlikely(!st)) st = rrdset_create("system", "interrupts", NULL, "interrupts", NULL, "System interrupts", "interrupts/s", 1000, update_every, RRDSET_TYPE_STACKED);
     else rrdset_next(st);
 
     for(l = 0; l < lines ;l++) {
         struct interrupt *irr = irrindex(irrs, l, cpus);
         if(unlikely(!irr->used)) continue;
-        rrddim_set(st, irr->id, irr->total);
+        // some interrupt may have changed without changing the total number of lines
+        // if the same number of interrupts have been added and removed between two
+        // calls of this function.
+        if(unlikely(!irr->rd || strncmp(irr->rd->name, irr->name, MAX_INTERRUPT_NAME) != 0)) {
+            irr->rd = rrddim_find(st, irr->id);
+            if(unlikely(!irr->rd))
+                irr->rd = rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
+            else
+                rrddim_set_name(st, irr->rd, irr->name);
+
+            // also reset per cpu RRDDIMs to avoid repeating strncmp() in the per core loop
+            if(likely(do_per_core)) {
+                int c;
+                for (c = 0; c < cpus ;c++)
+                    irr->cpu[c].rd = NULL;
+            }
+        }
+        rrddim_set_by_pointer(st, irr->rd, irr->total);
     }
     rrdset_done(st);
 
     if(likely(do_per_core)) {
         int c;
 
-        for(c = 0; c < cpus ; c++) {
+        for(c = 0; c < cpus ;c++) {
             char id[50+1];
             snprintfz(id, 50, "cpu%d_interrupts", c);
 
@@ -154,19 +181,20 @@ int do_proc_interrupts(int update_every, unsigned long long dt) {
                 char title[100+1];
                 snprintfz(title, 100, "CPU%d Interrupts", c);
                 st = rrdset_create("cpu", id, NULL, "interrupts", "cpu.interrupts", title, "interrupts/s", 1100 + c, update_every, RRDSET_TYPE_STACKED);
-
-                for(l = 0; l < lines ;l++) {
-                    struct interrupt *irr = irrindex(irrs, l, cpus);
-                    if(unlikely(!irr->used)) continue;
-                    rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
-                }
             }
             else rrdset_next(st);
 
             for(l = 0; l < lines ;l++) {
                 struct interrupt *irr = irrindex(irrs, l, cpus);
                 if(unlikely(!irr->used)) continue;
-                rrddim_set(st, irr->id, irr->value[c]);
+                if(unlikely(!irr->cpu[c].rd)) {
+                    irr->cpu[c].rd = rrddim_find(st, irr->id);
+                    if(unlikely(!irr->cpu[c].rd))
+                        irr->cpu[c].rd = rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
+                    else
+                        rrddim_set_name(st, irr->cpu[c].rd, irr->name);
+                }
+                rrddim_set_by_pointer(st, irr->cpu[c].rd, irr->cpu[c].value);
             }
             rrdset_done(st);
         }

diff --git a/src/proc_softirqs.c b/src/proc_softirqs.c
index 818a6a7098294c950ce1517e9215ae632b4ddaac..069e83ce199c37838718f8f15ff1ccec039574f6 100644 (file)
--- a/src/proc_softirqs.c
+++ b/src/proc_softirqs.c
@@ -2,17 +2,23 @@
 
 #define MAX_INTERRUPT_NAME 50
 
+struct cpu_interrupt {
+    unsigned long long value;
+    RRDDIM *rd;
+};
+
 struct interrupt {
     int used;
     char *id;
     char name[MAX_INTERRUPT_NAME + 1];
+    RRDDIM *rd;
     unsigned long long total;
-    unsigned long long value[];
+    struct cpu_interrupt cpu[];
 };
 
 // since each interrupt is variable in size
 // we use this to calculate its record size
-#define recordsize(cpus) (sizeof(struct interrupt) + (cpus * sizeof(unsigned long long)))
+#define recordsize(cpus) (sizeof(struct interrupt) + (cpus * sizeof(struct cpu_interrupt)))
 
 // given a base, get a pointer to each record
 #define irrindex(base, line, cpus) ((struct interrupt *)&((char *)(base))[line * recordsize(cpus)])
@@ -21,8 +27,21 @@ static inline struct interrupt *get_interrupts_array(int lines, int cpus) {
     static struct interrupt *irrs = NULL;
     static int allocated = 0;
 
-    if(unlikely(lines > allocated)) {
+    if(unlikely(lines != allocated)) {
+        uint32_t l;
+        int c;
+
         irrs = (struct interrupt *)reallocz(irrs, lines * recordsize(cpus));
+
+        // reset all interrupt RRDDIM pointers as any line could have shifted
+        for(l = 0; l < lines ;l++) {
+            struct interrupt *irr = irrindex(irrs, l, cpus);
+            irr->rd = NULL;
+            irr->name[0] = '\0';
+            for(c = 0; c < cpus ;c++)
+                irr->cpu[c].rd = NULL;
+        }
+
         allocated = lines;
     }
 
@@ -31,10 +50,8 @@ static inline struct interrupt *get_interrupts_array(int lines, int cpus) {
 
 int do_proc_softirqs(int update_every, unsigned long long dt) {
     (void)dt;
-
     static procfile *ff = NULL;
     static int cpus = -1, do_per_core = -1;
-
     struct interrupt *irrs = NULL;
 
     if(unlikely(do_per_core == -1)) do_per_core = config_get_boolean("plugin:proc:/proc/softirqs", "interrupts per core", 1);
@@ -62,7 +79,7 @@ int do_proc_softirqs(int update_every, unsigned long long dt) {
         uint32_t w;
         cpus = 0;
         for(w = 0; w < words ; w++) {
-            if(unlikely(strncmp(procfile_lineword(ff, 0, w), "CPU", 3) == 0))
+            if(likely(strncmp(procfile_lineword(ff, 0, w), "CPU", 3) == 0))
                 cpus++;
         }
     }
@@ -94,12 +111,12 @@ int do_proc_softirqs(int update_every, unsigned long long dt) {
 
         int c;
         for(c = 0; c < cpus ;c++) {
-            if(unlikely((c + 1) < (int)words))
-                irr->value[c] = strtoull(procfile_lineword(ff, l, (uint32_t)(c + 1)), NULL, 10);
+            if(likely((c + 1) < (int)words))
+                irr->cpu[c].value = strtoull(procfile_lineword(ff, l, (uint32_t)(c + 1)), NULL, 10);
             else
-                irr->value[c] = 0;
+                irr->cpu[c].value = 0;
 
-            irr->total += irr->value[c];
+            irr->total += irr->cpu[c].value;
         }
 
         strncpyz(irr->name, irr->id, MAX_INTERRUPT_NAME);
@@ -112,21 +129,30 @@ int do_proc_softirqs(int update_every, unsigned long long dt) {
     // --------------------------------------------------------------------
 
     st = rrdset_find_bytype("system", "softirqs");
-    if(unlikely(!st)) {
-        st = rrdset_create("system", "softirqs", NULL, "softirqs", NULL, "System softirqs", "softirqs/s", 950, update_every, RRDSET_TYPE_STACKED);
-
-        for(l = 0; l < lines ;l++) {
-            struct interrupt *irr = irrindex(irrs, l, cpus);
-            if(unlikely(!irr->used)) continue;
-            rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
-        }
-    }
+    if(unlikely(!st)) st = rrdset_create("system", "softirqs", NULL, "softirqs", NULL, "System softirqs", "softirqs/s", 950, update_every, RRDSET_TYPE_STACKED);
     else rrdset_next(st);
 
     for(l = 0; l < lines ;l++) {
         struct interrupt *irr = irrindex(irrs, l, cpus);
         if(unlikely(!irr->used)) continue;
-        rrddim_set(st, irr->id, irr->total);
+        // some interrupt may have changed without changing the total number of lines
+        // if the same number of interrupts have been added and removed between two
+        // calls of this function.
+        if(unlikely(!irr->rd || strncmp(irr->name, irr->rd->name, MAX_INTERRUPT_NAME) != 0)) {
+            irr->rd = rrddim_find(st, irr->id);
+            if(unlikely(!irr->rd))
+                irr->rd = rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
+            else
+                rrddim_set_name(st, irr->rd, irr->name);
+
+            // also reset per cpu RRDDIMs to avoid repeating strncmp() in the per core loop
+            if(likely(do_per_core)) {
+                int c;
+                for (c = 0; c < cpus ;c++)
+                    irr->cpu[c].rd = NULL;
+            }
+        }
+        rrddim_set_by_pointer(st, irr->rd, irr->total);
     }
     rrdset_done(st);
 
@@ -144,26 +170,27 @@ int do_proc_softirqs(int update_every, unsigned long long dt) {
                 for(l = 0; l < lines ;l++) {
                     struct interrupt *irr = irrindex(irrs, l, cpus);
                     if(unlikely(!irr->used)) continue;
-                    core_sum += irr->value[c];
+                    core_sum += irr->cpu[c].value;
                 }
                 if(unlikely(core_sum == 0)) continue; // try next core
 
                 char title[100+1];
                 snprintfz(title, 100, "CPU%d softirqs", c);
                 st = rrdset_create("cpu", id, NULL, "softirqs", "cpu.softirqs", title, "softirqs/s", 3000 + c, update_every, RRDSET_TYPE_STACKED);
-
-                for(l = 0; l < lines ;l++) {
-                    struct interrupt *irr = irrindex(irrs, l, cpus);
-                    if(unlikely(!irr->used)) continue;
-                    rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
-                }
             }
             else rrdset_next(st);
 
             for(l = 0; l < lines ;l++) {
                 struct interrupt *irr = irrindex(irrs, l, cpus);
                 if(unlikely(!irr->used)) continue;
-                rrddim_set(st, irr->id, irr->value[c]);
+                if(unlikely(!irr->cpu[c].rd)) {
+                    irr->cpu[c].rd = rrddim_find(st, irr->id);
+                    if(unlikely(!irr->cpu[c].rd))
+                        irr->cpu[c].rd = rrddim_add(st, irr->id, irr->name, 1, 1, RRDDIM_INCREMENTAL);
+                    else
+                        rrddim_set_name(st, irr->cpu[c].rd, irr->name);
+                }
+                rrddim_set_by_pointer(st, irr->cpu[c].rd, irr->cpu[c].value);
             }
             rrdset_done(st);
         }