hardirqs: fix duplicated count for shared IRQ
authorIsm Hong <ism.hong@gmail.com>
Mon, 6 Dec 2021 03:28:34 +0000 (11:28 +0800)
committeryonghong-song <ys114321@gmail.com>
Mon, 6 Dec 2021 18:27:08 +0000 (10:27 -0800)
Currently, hardirqs will count interrupt event simply while tracepoint
irq:irq_handler_entry triggered, it's fine for system without shared IRQ
event, but it will cause wrong interrupt count result for system with
shared IRQ, because kernel will interate all irq handlers belong to this
IRQ descriptor.

Take an example for system with shared IRQ below.

root@localhost:/# cat /proc/interrupts
           CPU0
 13:     385248     GICv3  39 Level     DDOMAIN ISR, gdma
 ...
 23:      61532     GICv3  38 Level     VGIP ISR, OnlineMeasure ISR

DDOMAIN IRQ and gdma shared the IRQ 13, VGIP ISR and OnlineMeasure
shared the IRQ 23, and use 'hardirqs -C' to measure the count of these
interrupt event.

root@localhost:/# hardirqs -C 10 1
Tracing hard irq events... Hit Ctrl-C to end.

HARDIRQ                    TOTAL_count
OnlineMeasure ISR                  300
VGIP ISR                           300
gdma                              2103
DDOMAIN ISR                       2103
eth0                              6677

hardirqs reported the same interrupt count for shared IRQ
'OnlineMeasure ISR/VGIP ISR' and 'gdma/DDOMAIN ISR'.

We should check the ret field of tracepoint irq:irq_hanlder_exit is
IRQ_HANDLED or IRQ_WAKE_THREAD to make sure the current event is belong
to this interrupt handler. For simplifying, just check `args->ret !=
IRQ_NONE`.

In the meantimes, the same changes should be applied to interrupt time
measurement.

The fixed hardirqs will show below output.

(bcc)root@localhost:/# ./hardirqs -C 10 1
Tracing hard irq events... Hit Ctrl-C to end.

HARDIRQ                    TOTAL_count
OnlineMeasure ISR                    1
VGIP ISR                           294
gdma                              1168
DDOMAIN ISR                       1476
eth0                              5210

tools/hardirqs.py

index 70fffbc2d28cdcbdb4cddc6140ff025d23002be8..98f1991e32651f6771f1fffe198ac9ad22177c15 100755 (executable)
@@ -84,9 +84,35 @@ BPF_HISTOGRAM(dist, irq_key_t);
 bpf_text_count = """
 TRACEPOINT_PROBE(irq, irq_handler_entry)
 {
-    irq_key_t key = {.slot = 0 /* ignore */};
-    TP_DATA_LOC_READ_STR(&key.name, name, sizeof(key.name));
-    dist.atomic_increment(key);
+    u32 tid = bpf_get_current_pid_tgid();
+    irq_name_t name = {};
+
+    TP_DATA_LOC_READ_STR(&name.name, name, sizeof(name));
+    irqnames.update(&tid, &name);
+    return 0;
+}
+
+TRACEPOINT_PROBE(irq, irq_handler_exit)
+{
+    u32 tid = bpf_get_current_pid_tgid();
+
+    // check ret value of irq handler is not IRQ_NONE to make sure
+    // the current event belong to this irq handler
+    if (args->ret != IRQ_NONE) {
+        irq_name_t *namep;
+
+        namep = irqnames.lookup(&tid);
+        if (namep == 0) {
+            return 0; // missed irq name
+        }
+        char *name = (char *)namep->name;
+        irq_key_t key = {.slot = 0 /* ignore */};
+
+        bpf_probe_read_kernel(&key.name, sizeof(key.name), name);
+        dist.atomic_increment(key);
+    }
+
+    irqnames.delete(&tid);
     return 0;
 }
 """
@@ -110,19 +136,23 @@ TRACEPOINT_PROBE(irq, irq_handler_exit)
     irq_name_t *namep;
     u32 tid = bpf_get_current_pid_tgid();
 
-    // fetch timestamp and calculate delta
-    tsp = start.lookup(&tid);
-    namep = irqnames.lookup(&tid);
-    if (tsp == 0 || namep == 0) {
-        return 0;   // missed start
+    // check ret value of irq handler is not IRQ_NONE to make sure
+    // the current event belong to this irq handler
+    if (args->ret != IRQ_NONE) {
+        // fetch timestamp and calculate delta
+        tsp = start.lookup(&tid);
+        namep = irqnames.lookup(&tid);
+        if (tsp == 0 || namep == 0) {
+            return 0;   // missed start
+        }
+
+        char *name = (char *)namep->name;
+        delta = bpf_ktime_get_ns() - *tsp;
+
+        // store as sum or histogram
+        STORE
     }
 
-    char *name = (char *)namep->name;
-    delta = bpf_ktime_get_ns() - *tsp;
-
-    // store as sum or histogram
-    STORE
-
     start.delete(&tid);
     irqnames.delete(&tid);
     return 0;