Skip to content

Conversation

@opsiff
Copy link
Member

@opsiff opsiff commented Nov 21, 2025

stable inclusion
from stable-v6.6.115
category: bugfix

commit 10fad4012234a7dea621ae17c0c9486824f645a0 upstream.

It is reported that commit 85975da ("cpuidle: menu: Avoid discarding useful information") led to a performance regression on Intel Jasper Lake systems because it reduced the time spent by CPUs in idle state C7 which is correlated to the maximum frequency the CPUs can get to because of an average running power limit [1].

Before that commit, get_typical_interval() would have returned UINT_MAX whenever it had been unable to make a high-confidence prediction which had led to selecting the deepest available idle state too often and both power and performance had been inadequate as a result of that on some systems. However, this had not been a problem on systems with relatively aggressive average running power limits, like the Jasper Lake systems in question, because on those systems it was compensated by the ability to run CPUs faster.

It was addressed by causing get_typical_interval() to return a number based on the recent idle duration information available to it even if it could not make a high-confidence prediction, but that clearly did not take the possible correlation between idle power and available CPU capacity into account.

For this reason, revert most of the changes made by commit 85975da, except for one cosmetic cleanup, and add a comment explaining the rationale for returning UINT_MAX from get_typical_interval() when it is unable to make a high-confidence prediction.

Fixes: 85975da ("cpuidle: menu: Avoid discarding useful information")
Closes: https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/ [1]
Reported-by: Sergey Senozhatsky [email protected]
Cc: All applicable [email protected]

Link: https://patch.msgid.link/[email protected]

(cherry picked from commit 29d96bcc1495260ada5aae073b8939d7afcecc16)

Summary by Sourcery

Revert the recent cpuidle menu change to restore the original idle-state prediction fallback and prevent performance regressions on systems like Jasper Lake

Bug Fixes:

  • Restore returning UINT_MAX from get_typical_interval() when no high-confidence prediction is possible to avoid reduced C7 residency and associated performance regressions on Jasper Lake

Enhancements:

  • Keep the cosmetic cleanup and add a clarifying comment on why UINT_MAX is returned in absence of high-confidence predictions

stable inclusion
from stable-v6.6.115
category: bugfix

commit 10fad4012234a7dea621ae17c0c9486824f645a0 upstream.

It is reported that commit 85975da ("cpuidle: menu: Avoid discarding
useful information") led to a performance regression on Intel Jasper Lake
systems because it reduced the time spent by CPUs in idle state C7 which
is correlated to the maximum frequency the CPUs can get to because of an
average running power limit [1].

Before that commit, get_typical_interval() would have returned UINT_MAX
whenever it had been unable to make a high-confidence prediction which
had led to selecting the deepest available idle state too often and
both power and performance had been inadequate as a result of that on
some systems.  However, this had not been a problem on systems with
relatively aggressive average running power limits, like the Jasper Lake
systems in question, because on those systems it was compensated by the
ability to run CPUs faster.

It was addressed by causing get_typical_interval() to return a number
based on the recent idle duration information available to it even if it
could not make a high-confidence prediction, but that clearly did not
take the possible correlation between idle power and available CPU
capacity into account.

For this reason, revert most of the changes made by commit 85975da,
except for one cosmetic cleanup, and add a comment explaining the
rationale for returning UINT_MAX from get_typical_interval() when it
is unable to make a high-confidence prediction.

Fixes: 85975da ("cpuidle: menu: Avoid discarding useful information")
Closes: https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/ [1]
Reported-by: Sergey Senozhatsky <[email protected]>
Cc: All applicable <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
(cherry picked from commit 29d96bcc1495260ada5aae073b8939d7afcecc16)
Signed-off-by: Wentao Guan <[email protected]>
@sourcery-ai
Copy link

sourcery-ai bot commented Nov 21, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR reverts the upstream idle interval prediction changes introduced by commit 85975da in the cpuidle menu governor, restoring the behavior where get_typical_interval() returns UINT_MAX when confidence is low, and retains only a cosmetic cleanup and enhanced comment explaining this rationale.

Class diagram for updated get_typical_interval() logic in menu governor

classDiagram
class menu_device
class get_typical_interval
get_typical_interval : +unsigned int get_typical_interval(menu_device *data)
menu_device <|-- get_typical_interval
get_typical_interval : returns UINT_MAX when confidence is low
get_typical_interval : enhanced comment explaining rationale
Loading

Flow diagram for get_typical_interval() decision logic after revert

flowchart TD
    A["Start: get_typical_interval()"] --> B["Check: divisor * 4 <= INTERVALS * 3"]
    B -- Yes --> C["Return UINT_MAX"]
    B -- No --> D["Continue: thresh = max - 1; goto again"]
Loading

File-Level Changes

Change Details Files
Revert modified get_typical_interval logic to always return UINT_MAX on low-confidence
  • Removed conditional return of max interval when sufficient samples remain
  • Simplified control flow to return UINT_MAX directly under divisor threshold
  • Added comment clarifying why UINT_MAX is returned when prediction confidence is low
drivers/cpuidle/governors/menu.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@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

@deepin-ci-robot
Copy link

deepin pr auto review

我来分析一下这段代码的修改:

  1. 代码逻辑变更:

    • 原代码在 divisor * 4 <= INTERVALS * 3 条件下还有一个额外的判断 divisor >= INTERVALS / 2,如果满足则返回 max
    • 新代码简化了逻辑,直接返回 UINT_MAX
  2. 性能影响:

    • 简化了分支逻辑,减少了条件判断,对性能有轻微提升
    • UINT_MAX 的返回会导致选择最深的空闲状态,这可能会增加进入/退出深空闲状态的延迟
  3. 安全性考虑:

    • 修改引入了更激进的空闲状态选择策略,可能会导致:
      • 在某些场景下增加功耗(频繁进入深空闲状态)
      • 增加系统响应延迟(从深空闲状态唤醒需要更长时间)
    • 但是根据注释说明,这是为了解决特定系统上深空闲状态与最大频率相关的问题
  4. 代码质量:

    • 代码逻辑更简洁,减少了嵌套
    • 注释很好地解释了修改的原因
    • 但是缺少了对边界情况的处理说明

改进建议:

  1. 可以考虑添加一个配置选项,让系统可以根据实际需求选择是否使用这种激进的策略

  2. 建议在注释中添加更多关于性能影响的说明

  3. 可以考虑添加一些统计信息,帮助监控系统进入深空闲状态的频率和效果

  4. 建议添加对 UINT_MAX 返回值的边界检查,确保调用方能够正确处理这个值

总体来说,这个修改是为了解决特定场景下的问题,但可能会影响其他场景的性能表现。建议在实际部署前进行充分的测试和性能评估。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request reverts a previous commit (85975da) that caused performance regressions on Intel Jasper Lake systems by reducing C7 idle state residency. The revert restores the original behavior where get_typical_interval() returns UINT_MAX when unable to make a high-confidence prediction, allowing the deepest available idle state to be selected. This is important for systems with aggressive power limits where deep idle states correlate with higher maximum CPU frequencies.

Key Changes

  • Removed conditional logic that returned the current maximum value when there were sufficient data points remaining after outlier elimination
  • Restored simple UINT_MAX return when the number of remaining samples is too small (divisor * 4 <= INTERVALS * 3)
  • Added a comprehensive comment explaining the rationale for returning UINT_MAX to guide future development

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* selected because there are systems where the time spent by CPUs in
* deep idle states is correlated to the maximum frequency the CPUs
* can get to. On those systems, shallow idle states should be avoided
* unless there is a clear indication that the given CPU is most likley
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in the comment: "likley" should be "likely".

Suggested change
* unless there is a clear indication that the given CPU is most likley
* unless there is a clear indication that the given CPU is most likely

Copilot uses AI. Check for mistakes.
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