Re: [PATCH v2 0/6] cpuidle: speed up do_idle() by caching the governor latency QoS constraint

[Yaxiong Tian]

On 2026/9/18 03:34, Rafael J. Wysocki (Intel) wrote:
> On Wed, Jul 29, 2026 at 8:16 AM Yaxiong Tian <tianyaxiong@xxxxxxxxxx> wrote:
>> cpuidle_governor_latency_req() is evaluated on every idle-state
>> selection.  It aggregates the per-CPU resume latency with the global
>> CPU latency and wakeup latency QoS limits
> True.
>
>> by repeatedly calling
>> get_cpu_device() and pm_qos_read_value() cpu_latency_qos_limit()
>> cpu_wakeup_latency_qos_limit().
> I would say "every time" rather than "repeatedly", but fair enough.
>
> However, those functions don't do anything expensive except for
> defensive checks that can be omitted.  Have you tried to omit them?
I haven't tried that, but I found that function_graph may introduce measurement errors, especially for very short functions.
>
>> Use ftrace's function_graph, we can see:
>> parent:          do_idle
>> parent_total_ns: 36671010505
>> parent_count:    5994
>>
>> SYMBOL                                                      TIME_NS    %ROOT  %PARENT      COUNT
>> --------------------------------------------------------------------------------------------------
>> do_idle                                                 36671010505  100.00%  100.00%       5994
>>   cpuidle_idle_call                                     35566528731   96.99%   96.99%       8570
>>     call_cpuidle                                        35476606844   96.74%   99.75%       8561
>>       cpuidle_enter                                     35472932468   96.73%   99.99%       8526
>>     cpuidle_select                                         52097031    0.14%    0.15%       8580
>>       menu_select                                          49555181    0.14%   95.12%       8580
>>         tick_nohz_get_sleep_length                         28843887    0.08%   58.21%       8570
>>         cpuidle_governor_latency_req                        9567488    0.03%   19.31%       8580
>>         tick_nohz_tick_stopped                              2057031    0.01%    4.15%      15695
>>     cpuidle_reflect                                        11427201    0.03%    0.03%       8561
>>       menu_reflect                                          6579506    0.02%   57.58%       8526
>>         tick_nohz_idle_got_tick                             2231559    0.01%   33.92%       8526
>>       __sysvec_apic_timer_interrupt                          105520    0.00%    0.92%          3
>>     tick_nohz_idle_stop_tick                                8279641    0.02%    0.02%       1475
>>     ---- skip
>>
>> The majority of the time spent in cpuidle_enter for CPUs entering
>> idle state has already been charged to the idle path. Among the
>> remaining contributors, cpuidle_governor_latency_req() accounts
>> for a non-negligible portion of the overall latency.
>>
>> Under the menu governor this shows up hot: ftrace data shows,
>>  cpuidle_governor_latency_req() accounts for about 19.9% of
>> menu_select() time (~1.9 us/call).  After caching the aggregated
>> value per CPU and invalidating via QoS notifiers, that share drops to
>> about 4.2% (~0.3 us/call), roughly a 6x reduction on this path.
> Is there any real-world workload in which that difference is actually visible?
Sorry, I only tested it under no load.

>
>> The ftrace data before and after the optimization is shown below:
>> 1) original
>> parent:          menu_select
>> parent_total_ns: 160492937
>> parent_count:    16718
>>
>> SYMBOL                                                      TIME_NS    %ROOT  %PARENT      COUNT
>> --------------------------------------------------------------------------------------------------
>> menu_select                                               160492937  100.00%  100.00%      16718
> Where did you lose menu_update()?
Because menu_update() was optimized away by the compiler, it can't be seen in /proc/kallsyms, so it won't be traced by ftrace either.
>
>>   tick_nohz_get_sleep_length                              100262940   62.47%   62.47%      16698
>>     tick_nohz_next_event                                   67891299   42.30%   67.71%      16689
>>       rcu_needs_cpu                                         2825649    1.76%    4.16%      16689
>>       timekeeping_max_deferment                             2380377    1.48%    3.51%      15296
>>     hrtimer_next_event_without                             17865162   11.13%   17.82%      15296
>>       hrtimer_bases_next_event_without                      2707631    1.69%   15.16%      15296
>>       _raw_spin_lock_irqsave                                2461132    1.53%   13.78%      15296
>>         native_queued_spin_lock_slowpath                        177    0.00%    0.01%          1
>>       _raw_spin_unlock_irqrestore                           2364647    1.47%   13.24%      15296
>>     can_stop_idle_tick                                      2906072    1.81%    2.90%      16698
>>   cpuidle_governor_latency_req                             31988150   19.93%   19.93%      16718
>>     get_cpu_device                                          4122502    2.57%   12.89%      16718
>>     pm_qos_read_value                                       3427318    2.14%   10.71%      16718
>>     cpu_latency_qos_limit                                   3005804    1.87%    9.40%      16718
>>     cpu_wakeup_latency_qos_limit                            3005475    1.87%    9.40%      16718
>>   tick_nohz_tick_stopped                                    4551981    2.84%    2.84%      29496
>>
>> %ROOT = share of menu_select; %PARENT = share of immediate caller (inclusive)
>>
>> 2) post-optimized
>> parent:          menu_select
>> parent_total_ns: 55428604
>> parent_count:    7626
>>
>> SYMBOL                                                      TIME_NS    %ROOT  %PARENT      COUNT
>> --------------------------------------------------------------------------------------------------
>> menu_select                                                55428604  100.00%  100.00%       7626
> And here?
>
>>   tick_nohz_get_sleep_length                               37464607   67.59%   67.59%       7544
>>     tick_nohz_next_event                                   24076913   43.44%   64.27%       7522
>>       get_next_timer_interrupt                             16332381   29.47%   67.83%       5854
>>       rcu_needs_cpu                                         1489633    2.69%    6.19%       7522
>>       timekeeping_max_deferment                              870851    1.57%    3.62%       5586
>>     hrtimer_next_event_without                              6140153   11.08%   16.39%       5586
>>       hrtimer_bases_next_event_without                       979000    1.77%   15.94%       5586
>>       _raw_spin_lock_irqsave                                 808786    1.46%   13.17%       5586
>>       _raw_spin_unlock_irqrestore                            785518    1.42%   12.79%       5586
>>     can_stop_idle_tick                                      1583137    2.86%    4.23%       7544
>>   cpuidle_governor_latency_req                              2321119    4.19%    4.19%       7626
>>   tick_nohz_tick_stopped                                    1863015    3.36%    3.36%      12751
>>
>> %ROOT = share of menu_select; %PARENT = share of immediate caller (inclusive)
>>
>> This series:
>> - patch1~pathc3: Add QoS notifiers-related infrastructure and updates to
>> latency_req_gen.
>> - patch4: Implement QoS constraint for cache-aggregated governor latency
>> - patch5: add latency_req QoS idle-state selection test
>> - patch6: add idle-state disable selftest
> So this adds two per-CPU variables and a ton of extra complexity to
> optimize the case in which the majority of the governor computations
> aren't actually done.
>
> I'm totally unconvinced.
I agree, it doesn't seem worth the effort.

>
> Thanks!