Skip to content

softirqs: Combined CPU as part of the key is necessary to avoid amiss…#2804

Merged
yonghong-song merged 1 commit into
iovisor:masterfrom
netedwardwu:master
Mar 9, 2020
Merged

softirqs: Combined CPU as part of the key is necessary to avoid amiss…#2804
yonghong-song merged 1 commit into
iovisor:masterfrom
netedwardwu:master

Conversation

@netedwardwu

Copy link
Copy Markdown
Contributor

… value.

In the environment of massive software interrupts.
-0 [003] ..s1 106.421020: softirq_entry: vec=6 [action=TASKLET]
-0 [000] ..s1 106.421063: softirq_entry: vec=3 [action=NET_RX]
-0 [003] ..s1 106.421083: softirq_exit: vec=6 [action=TASKLET]

Follow the above ftrace logs, we know the correct vec-6 start timestamp is replaced with incorrect vec-3.
Because PID is idle-0. It will produce the wrong result after calculating delta.

… value.

In the environment of massive software interrupts.
<idle>-0     [003] ..s1   106.421020: softirq_entry: vec=6 [action=TASKLET]
<idle>-0     [000] ..s1   106.421063: softirq_entry: vec=3 [action=NET_RX]
<idle>-0     [003] ..s1   106.421083: softirq_exit: vec=6 [action=TASKLET]

Follow the above ftrace logs, we know the correct vec-6 start timestamp is replaced with incorrect vec-3.
Because PID is idle-0. It will produce the wrong result after calculating delta.
@yonghong-song

Copy link
Copy Markdown
Collaborator

[buildbot, test this please]

@yonghong-song
yonghong-song merged commit 57e7af7 into iovisor:master Mar 9, 2020
ismhong added a commit to ismhong/bcc that referenced this pull request Dec 8, 2021
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 iovisor#2804, iovisor#3733.
yonghong-song pushed a commit that referenced this pull request Dec 9, 2021
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.
CrackerCat pushed a commit to CrackerCat/bcc that referenced this pull request Jul 31, 2024
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 iovisor#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 iovisor#2804, iovisor#3733.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants