Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

netedwardwu
Copy link
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
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