hardirqs: fix issue if irq is triggered while idle task (tid=0)
authorIsm Hong <ism.hong@gmail.com>
Wed, 8 Dec 2021 02:17:20 +0000 (10:17 +0800)
committeryonghong-song <ys114321@gmail.com>
Thu, 9 Dec 2021 06:45:57 +0000 (22:45 -0800)
Currently, hardirqs use tid as key to store information while tracepoint
irq_handler_entry. It works fine if irq is triggered while normal task
running, but there is a chance causing overwrite issue while irq is
triggered while idle task (a.k.a swapper/x, tid=0) running on multi-core
system.

Let's say there are two irq event trigger simultaneously on both CPU
core, irq A @ core #0, irq B @ core #1, and system load is pretty light,
so BPF program will get tid=0 since current task is swapper/x for both cpu
core. In this case, the information of first irq event stored in map could
be overwritten by incoming second irq event.

Use tid and cpu_id together to make sure the key is unique for each
event in this corner case.

Please check more detail at merge request #2804, #3733.

tools/hardirqs.py

index 98f1991e32651f6771f1fffe198ac9ad22177c15..0eeddddc086ee8e40e0479d04a2551d8e729871a 100755 (executable)
@@ -67,6 +67,14 @@ bpf_text = """
 #include <linux/irqdesc.h>
 #include <linux/interrupt.h>
 
+// Add cpu_id as part of key for irq entry event to handle the case which irq
+// is triggered while idle thread(swapper/x, tid=0) for each cpu core.
+// Please see more detail at pull request #2804, #3733.
+typedef struct entry_key {
+    u32 tid;
+    u32 cpu_id;
+} entry_key_t;
+
 typedef struct irq_key {
     char name[32];
     u64 slot;
@@ -76,32 +84,38 @@ typedef struct irq_name {
     char name[32];
 } irq_name_t;
 
-BPF_HASH(start, u32);
-BPF_HASH(irqnames, u32, irq_name_t);
+BPF_HASH(start, entry_key_t);
+BPF_HASH(irqnames, entry_key_t, irq_name_t);
 BPF_HISTOGRAM(dist, irq_key_t);
 """
 
 bpf_text_count = """
 TRACEPOINT_PROBE(irq, irq_handler_entry)
 {
-    u32 tid = bpf_get_current_pid_tgid();
+    struct entry_key key = {};
     irq_name_t name = {};
 
+    key.tid = bpf_get_current_pid_tgid();
+    key.cpu_id = bpf_get_smp_processor_id();
+
     TP_DATA_LOC_READ_STR(&name.name, name, sizeof(name));
-    irqnames.update(&tid, &name);
+    irqnames.update(&key, &name);
     return 0;
 }
 
 TRACEPOINT_PROBE(irq, irq_handler_exit)
 {
-    u32 tid = bpf_get_current_pid_tgid();
+    struct entry_key key = {};
+
+    key.tid = bpf_get_current_pid_tgid();
+    key.cpu_id = bpf_get_smp_processor_id();
 
     // 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);
+        namep = irqnames.lookup(&key);
         if (namep == 0) {
             return 0; // missed irq name
         }
@@ -112,7 +126,7 @@ TRACEPOINT_PROBE(irq, irq_handler_exit)
         dist.atomic_increment(key);
     }
 
-    irqnames.delete(&tid);
+    irqnames.delete(&key);
     return 0;
 }
 """
@@ -120,13 +134,16 @@ TRACEPOINT_PROBE(irq, irq_handler_exit)
 bpf_text_time = """
 TRACEPOINT_PROBE(irq, irq_handler_entry)
 {
-    u32 tid = bpf_get_current_pid_tgid();
     u64 ts = bpf_ktime_get_ns();
     irq_name_t name = {};
+    struct entry_key key = {};
+
+    key.tid = bpf_get_current_pid_tgid();
+    key.cpu_id = bpf_get_smp_processor_id();
 
     TP_DATA_LOC_READ_STR(&name.name, name, sizeof(name));
-    irqnames.update(&tid, &name);
-    start.update(&tid, &ts);
+    irqnames.update(&key, &name);
+    start.update(&key, &ts);
     return 0;
 }
 
@@ -134,14 +151,17 @@ TRACEPOINT_PROBE(irq, irq_handler_exit)
 {
     u64 *tsp, delta;
     irq_name_t *namep;
-    u32 tid = bpf_get_current_pid_tgid();
+    struct entry_key key = {};
+
+    key.tid = bpf_get_current_pid_tgid();
+    key.cpu_id = bpf_get_smp_processor_id();
 
     // 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);
+        tsp = start.lookup(&key);
+        namep = irqnames.lookup(&key);
         if (tsp == 0 || namep == 0) {
             return 0;   // missed start
         }
@@ -153,8 +173,8 @@ TRACEPOINT_PROBE(irq, irq_handler_exit)
         STORE
     }
 
-    start.delete(&tid);
-    irqnames.delete(&tid);
+    start.delete(&key);
+    irqnames.delete(&key);
     return 0;
 }
 """