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

[Deepin-Kernel-SIG] [Upstream] [linux 6.6-y] net: save some cycles when doing skb_attempt_defer_free() #531

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

opsiff
Copy link
Member

@opsiff opsiff commented Dec 23, 2024

mainline inclusion
from mainline-v6.10-rc1
category: performance

Normally, we don't face these two exceptions very often meanwhile we have some chance to meet the condition where the current cpu id is the same as skb->alloc_cpu.

One simple test that can help us see the frequency of this statement 'cpu == raw_smp_processor_id()':

  1. running iperf -s and iperf -c [ip] -P [MAX CPU]
  2. using BPF to capture skb_attempt_defer_free()

I can see around 4% chance that happens to satisfy the statement. So moving this statement at the beginning can save some cycles in most cases.

Reviewed-by: Alexander Lobakin [email protected]

mainline inclusion
from mainline-v6.10-rc1
category: performance

Normally, we don't face these two exceptions very often meanwhile
we have some chance to meet the condition where the current cpu id
is the same as skb->alloc_cpu.

One simple test that can help us see the frequency of this statement
'cpu == raw_smp_processor_id()':
1. running iperf -s and iperf -c [ip] -P [MAX CPU]
2. using BPF to capture skb_attempt_defer_free()

I can see around 4% chance that happens to satisfy the statement.
So moving this statement at the beginning can save some cycles in
most cases.

Signed-off-by: Jason Xing <[email protected]>
Reviewed-by: Alexander Lobakin <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Wentao Guan <[email protected]>
@deepin-ci-robot
Copy link

deepin pr auto review

在代码审查过程中,我会关注代码的语法、逻辑、质量、性能和安全性。以下是针对您提供的代码片段的审查意见:

代码逻辑

  • 代码逻辑上没有明显的错误,但是将 cpu == raw_smp_processor_id() 放在第一个检查位置可能会影响性能,因为这是一个非常频繁的检查。通常,性能敏感的代码会先检查最有可能为真的条件。

代码质量

  • 代码风格上,将 WARN_ON_ONCE(cpu >= nr_cpu_ids) 放在第一个检查位置可能会让代码的可读性降低,因为读者可能会首先注意到这个条件,而忽略了它后面的条件。建议保持原有的顺序,即先检查 cpu >= nr_cpu_ids,再检查 cpu_online(cpu),最后检查 cpu == raw_smp_processor_id()

代码性能

  • cpu == raw_smp_processor_id() 放在第一个检查位置可能会稍微影响性能,因为这是一个非常频繁的检查。如果这个检查失败的概率很低,那么这种影响可以忽略不计。但是,如果这个检查失败的概率较高,那么建议保持原有的顺序。

代码安全性

  • 代码中没有明显的安全问题,但是需要确保 cpu_online(cpu) 函数的实现是安全的,不会因为传入的 cpu 参数不正确而导致系统崩溃。

综合建议

  • 保持原有的检查顺序,即先检查 cpu >= nr_cpu_ids,再检查 cpu_online(cpu),最后检查 cpu == raw_smp_processor_id()
  • 确保 cpu_online(cpu) 函数的实现是安全的。

综上所述,代码逻辑上是正确的,但是代码风格和性能方面可以做一些优化。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@opsiff opsiff merged commit 8e7a5c3 into linux-6.6.y Dec 24, 2024
7 of 10 checks passed
@opsiff opsiff deleted the net-opt branch January 13, 2025 07:59
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.

3 participants