[PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It is reported that commit 85975daeaa4d ("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 in
some cases.  This was not 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 at relatively higher frequencies.

Commit 85975daeaa4d addressed that by causing get_typical_interval() to
return a number based on the recent idle duration information available
to it in those cases, but that number is usually smaller than the
maximum idle duration observed recently which may be regarded as an
overly optimistic choice.

Namely, it may be argued that when the samples considered by
get_typical_interval() are spread too much for a high-confidence
prediction to be made, the function should fall back to returning a
number that is likely to be an upper bound for the duration of the
upcoming idle interval and that number needs to be at least equal to
the maximum recently observed idle time.  Otherwise, the governor may
miss an oportunity to reduce power without hurting performance in a
noticeable way.  Of course, it may also be argued the other way around,
but the available data indicate that get_typical_interval() should
rather tend to return larger numbers as that causes the governor to
behave more closely to its past behavior from before the problematic
commit.

Accordingly, modify get_typical_interval() to return the maximum
recently observed idle time when it is unable to make a high-
confidence prediction.

Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information")
Closes: https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/ [1]
Reported-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: All applicable <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/menu.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -116,6 +116,7 @@ static void menu_update(struct cpuidle_d
 static unsigned int get_typical_interval(struct menu_device *data)
 {
 	s64 value, min_thresh = -1, max_thresh = UINT_MAX;
+	unsigned int max_overall = 0;
 	unsigned int max, min, divisor;
 	u64 avg, variance, avg_sq;
 	int i;
@@ -151,6 +152,9 @@ again:
 	if (!max)
 		return UINT_MAX;
 
+	if (max_overall < max)
+		max_overall = max;
+
 	if (divisor == INTERVALS) {
 		avg >>= INTERVAL_SHIFT;
 		variance >>= INTERVAL_SHIFT;
@@ -198,7 +202,7 @@ again:
 		 * maximum, so return the latter in that case.
 		 */
 		if (divisor >= INTERVALS / 2)
-			return max;
+			return max_overall;
 
 		return UINT_MAX;
 	}

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Christian Loehle]

On 10/16/25 17:25, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> It is reported that commit 85975daeaa4d ("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 in
> some cases.  This was not 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 at relatively higher frequencies.
> 
> Commit 85975daeaa4d addressed that by causing get_typical_interval() to
> return a number based on the recent idle duration information available
> to it in those cases, but that number is usually smaller than the
> maximum idle duration observed recently which may be regarded as an
> overly optimistic choice.
> 
> Namely, it may be argued that when the samples considered by
> get_typical_interval() are spread too much for a high-confidence
> prediction to be made, the function should fall back to returning a
> number that is likely to be an upper bound for the duration of the
> upcoming idle interval and that number needs to be at least equal to
> the maximum recently observed idle time.  Otherwise, the governor may
> miss an oportunity to reduce power without hurting performance in a
> noticeable way.  Of course, it may also be argued the other way around,
> but the available data indicate that get_typical_interval() should
> rather tend to return larger numbers as that causes the governor to
> behave more closely to its past behavior from before the problematic
> commit.
> 
> Accordingly, modify get_typical_interval() to return the maximum
> recently observed idle time when it is unable to make a high-
> confidence prediction.
> 
> Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information")
> Closes: https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/ [1]
> Reported-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: All applicable <stable@vger.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpuidle/governors/menu.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -116,6 +116,7 @@ static void menu_update(struct cpuidle_d
>  static unsigned int get_typical_interval(struct menu_device *data)
>  {
>  	s64 value, min_thresh = -1, max_thresh = UINT_MAX;
> +	unsigned int max_overall = 0;

nit: for reverse xmas this should be one further down?
Maybe s/max_overall/max_first_pass/?

>  	unsigned int max, min, divisor;
>  	u64 avg, variance, avg_sq;
>  	int i;
> @@ -151,6 +152,9 @@ again:
>  	if (!max)
>  		return UINT_MAX;
>  

Or alternatively a comment:
/* Save the max before we discard any intervals */
or something.

> +	if (max_overall < max)
> +		max_overall = max;
> +>  	if (divisor == INTERVALS) {
>  		avg >>= INTERVAL_SHIFT;
>  		variance >>= INTERVAL_SHIFT;
> @@ -198,7 +202,7 @@ again:
>  		 * maximum, so return the latter in that case.
>  		 */
>  		if (divisor >= INTERVALS / 2)
> -			return max;
> +			return max_overall;
>  
>  		return UINT_MAX;
>  	}
> 

Anyway, the patch makes sense, let me run some tests and get back.

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Rafael J. Wysocki]

On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 10/16/25 17:25, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > It is reported that commit 85975daeaa4d ("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 in
> > some cases.  This was not 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 at relatively higher frequencies.
> >
> > Commit 85975daeaa4d addressed that by causing get_typical_interval() to
> > return a number based on the recent idle duration information available
> > to it in those cases, but that number is usually smaller than the
> > maximum idle duration observed recently which may be regarded as an
> > overly optimistic choice.
> >
> > Namely, it may be argued that when the samples considered by
> > get_typical_interval() are spread too much for a high-confidence
> > prediction to be made, the function should fall back to returning a
> > number that is likely to be an upper bound for the duration of the
> > upcoming idle interval and that number needs to be at least equal to
> > the maximum recently observed idle time.  Otherwise, the governor may
> > miss an oportunity to reduce power without hurting performance in a

Ah, a typo.

> > noticeable way.  Of course, it may also be argued the other way around,
> > but the available data indicate that get_typical_interval() should
> > rather tend to return larger numbers as that causes the governor to
> > behave more closely to its past behavior from before the problematic
> > commit.
> >
> > Accordingly, modify get_typical_interval() to return the maximum
> > recently observed idle time when it is unable to make a high-
> > confidence prediction.
> >
> > Fixes: 85975daeaa4d ("cpuidle: menu: Avoid discarding useful information")
> > Closes: https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/ [1]
> > Reported-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > Cc: All applicable <stable@vger.kernel.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/cpuidle/governors/menu.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > --- a/drivers/cpuidle/governors/menu.c
> > +++ b/drivers/cpuidle/governors/menu.c
> > @@ -116,6 +116,7 @@ static void menu_update(struct cpuidle_d
> >  static unsigned int get_typical_interval(struct menu_device *data)
> >  {
> >       s64 value, min_thresh = -1, max_thresh = UINT_MAX;
> > +     unsigned int max_overall = 0;
>
> nit: for reverse xmas this should be one further down?

Actually, I can combine this with the min and max definition.

> Maybe s/max_overall/max_first_pass/?

I can call it grand_max.

> >       unsigned int max, min, divisor;
> >       u64 avg, variance, avg_sq;
> >       int i;
> > @@ -151,6 +152,9 @@ again:
> >       if (!max)
> >               return UINT_MAX;
> >
>
> Or alternatively a comment:
> /* Save the max before we discard any intervals */
> or something.
>
> > +     if (max_overall < max)
> > +             max_overall = max;
> > +>    if (divisor == INTERVALS) {
> >               avg >>= INTERVAL_SHIFT;
> >               variance >>= INTERVAL_SHIFT;
> > @@ -198,7 +202,7 @@ again:
> >                * maximum, so return the latter in that case.
> >                */
> >               if (divisor >= INTERVALS / 2)
> > -                     return max;
> > +                     return max_overall;
> >
> >               return UINT_MAX;
> >       }
> >
>
> Anyway, the patch makes sense, let me run some tests and get back.

Thanks!

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Christian Loehle]

On 10/17/25 10:39, Rafael J. Wysocki wrote:
> On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle
> <christian.loehle@arm.com> wrote:
>>
>> On 10/16/25 17:25, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> It is reported that commit 85975daeaa4d ("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].
>>> [snip]
>> [snip]
>> Anyway, the patch makes sense, let me run some tests and get back.
> 
> Thanks!

Unfortunately this patch regresses my tests about as much as a revert of
85975daeaa4d would.
(menu-1 is $SUBJECT, menu-m current mainline, menu-r mainline with
85975daeaa4d reverted):


device	 gov	 iter	 iops	 idles	 idle_misses	 idle_miss_ratio	 belows	 aboves	
mmcblk1 	menu-1 	0 	1523 	402522 	119988 	0.298 	98552 	21436
mmcblk1 	menu-1 	1 	1481 	395766 	118596 	0.300 	97640 	20956
mmcblk1 	menu-1 	2 	1503 	396560 	117876 	0.297 	97506 	20370
mmcblk1 	menu-m 	0 	2355 	703732 	22275 	0.032 	2628 	19647
mmcblk1 	menu-m 	1 	2359 	637522 	24815 	0.039 	4075 	20740
mmcblk1 	menu-m 	2 	2356 	706980 	23836 	0.034 	3208 	20628
mmcblk1 	menu-r 	0 	1490 	388180 	118294 	0.305 	97871 	20423
mmcblk1 	menu-r 	1 	1498 	393402 	119984 	0.305 	99187 	20797
mmcblk1 	menu-r 	2 	1462 	388597 	119504 	0.308 	98640 	20864
mmcblk2 	menu-1 	0 	3303 	503938 	170251 	0.338 	150276 	19975
mmcblk2 	menu-1 	1 	3310 	480508 	132132 	0.275 	114398 	17734
mmcblk2 	menu-1 	2 	3554 	466884 	113659 	0.243 	95841 	17818
mmcblk2 	menu-m 	0 	5746 	706262 	24618 	0.035 	3802 	20816
mmcblk2 	menu-m 	1 	5741 	727174 	24152 	0.033 	3737 	20415
mmcblk2 	menu-m 	2 	5777 	836940 	12424 	0.015 	335 	12089
mmcblk2 	menu-r 	0 	3241 	463112 	133052 	0.287 	114616 	18436
mmcblk2 	menu-r 	1 	3551 	422006 	100494 	0.238 	82425 	18069
mmcblk2 	menu-r 	2 	3523 	508542 	140085 	0.275 	122880 	17205
nvme0n1 	menu-1 	0 	5407 	436834 	74314 	0.170 	54133 	20181
nvme0n1 	menu-1 	1 	5763 	459510 	72673 	0.158 	51530 	21143
nvme0n1 	menu-1 	2 	6266 	489570 	78651 	0.161 	58609 	20042
nvme0n1 	menu-m 	0 	10786 	767740 	23840 	0.031 	2855 	20985
nvme0n1 	menu-m 	1 	10586 	757540 	23612 	0.031 	2933 	20679
nvme0n1 	menu-m 	2 	11805 	834012 	23528 	0.028 	2768 	20760
nvme0n1 	menu-r 	0 	5323 	431906 	77426 	0.179 	56166 	21260
nvme0n1 	menu-r 	1 	5484 	438142 	76033 	0.174 	55956 	20077
nvme0n1 	menu-r 	2 	5353 	428826 	77024 	0.180 	57016 	20008
sda 	menu-1 	0 	972 	444116 	149643 	0.337 	129023 	20620
sda 	menu-1 	1 	954 	557068 	176479 	0.317 	159092 	17387
sda 	menu-1 	2 	878 	540360 	196405 	0.363 	176792 	19613
sda 	menu-m 	0 	1634 	1017918 	29614 	0.029 	8587 	21027
sda 	menu-m 	1 	1622 	878140 	25323 	0.029 	8238 	17085
sda 	menu-m 	2 	1632 	1027167 	28798 	0.028 	8428 	20370
sda 	menu-r 	0 	918 	531112 	188314 	0.355 	168375 	19939
sda 	menu-r 	1 	924 	521378 	185727 	0.356 	165327 	20400
sda 	menu-r 	2 	880 	529146 	196391 	0.371 	176908 	19483
nullb0 	menu-1 	0 	101419 	88988 	23923 	0.269 	3080 	20843
nullb0 	menu-1 	1 	101610 	88484 	23678 	0.268 	2821 	20857
nullb0 	menu-1 	2 	101369 	89336 	23711 	0.265 	2795 	20916
nullb0 	menu-m 	0 	101696 	88698 	23860 	0.269 	2910 	20950
nullb0 	menu-m 	1 	101103 	88120 	23294 	0.264 	3295 	19999
nullb0 	menu-m 	2 	101880 	86676 	22730 	0.262 	2709 	20021
nullb0 	menu-r 	0 	101856 	87742 	23493 	0.268 	3204 	20289
nullb0 	menu-r 	1 	101514 	89070 	23653 	0.266 	2848 	20805
nullb0 	menu-r 	2 	101754 	86318 	23163 	0.268 	3229 	19934
mtdblock3 	menu-1 	0 	163 	350284 	115149 	0.329 	97166 	17983
mtdblock3 	menu-1 	1 	179 	315948 	99038 	0.313 	78243 	20795
mtdblock3 	menu-1 	2 	134 	481584 	160754 	0.334 	144150 	16604
mtdblock3 	menu-m 	0 	215 	410034 	70261 	0.171 	55445 	14816
mtdblock3 	menu-m 	1 	205 	570150 	109273 	0.192 	90189 	19084
mtdblock3 	menu-m 	2 	252 	866616 	23492 	0.027 	9717 	13775
mtdblock3 	menu-r 	0 	132 	467365 	161835 	0.346 	144056 	17779
mtdblock3 	menu-r 	1 	164 	348682 	117704 	0.338 	97859 	19845
mtdblock3 	menu-r 	2 	132 	483300 	165179 	0.342 	147164 	18015

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Rafael J. Wysocki]

On Fri, Oct 17, 2025 at 8:37 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 10/17/25 10:39, Rafael J. Wysocki wrote:
> > On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle
> > <christian.loehle@arm.com> wrote:
> >>
> >> On 10/16/25 17:25, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>
> >>> It is reported that commit 85975daeaa4d ("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].
> >>> [snip]
> >> [snip]
> >> Anyway, the patch makes sense, let me run some tests and get back.
> >
> > Thanks!
>
> Unfortunately this patch regresses my tests about as much as a revert of
> 85975daeaa4d would.
> (menu-1 is $SUBJECT, menu-m current mainline, menu-r mainline with
> 85975daeaa4d reverted):
>
>
> device   gov     iter    iops    idles   idle_misses     idle_miss_ratio         belows  aboves
> mmcblk1         menu-1  0       1523    402522  119988  0.298   98552   21436
> mmcblk1         menu-1  1       1481    395766  118596  0.300   97640   20956
> mmcblk1         menu-1  2       1503    396560  117876  0.297   97506   20370
> mmcblk1         menu-m  0       2355    703732  22275   0.032   2628    19647
> mmcblk1         menu-m  1       2359    637522  24815   0.039   4075    20740
> mmcblk1         menu-m  2       2356    706980  23836   0.034   3208    20628
> mmcblk1         menu-r  0       1490    388180  118294  0.305   97871   20423
> mmcblk1         menu-r  1       1498    393402  119984  0.305   99187   20797
> mmcblk1         menu-r  2       1462    388597  119504  0.308   98640   20864
> mmcblk2         menu-1  0       3303    503938  170251  0.338   150276  19975
> mmcblk2         menu-1  1       3310    480508  132132  0.275   114398  17734
> mmcblk2         menu-1  2       3554    466884  113659  0.243   95841   17818
> mmcblk2         menu-m  0       5746    706262  24618   0.035   3802    20816
> mmcblk2         menu-m  1       5741    727174  24152   0.033   3737    20415
> mmcblk2         menu-m  2       5777    836940  12424   0.015   335     12089
> mmcblk2         menu-r  0       3241    463112  133052  0.287   114616  18436
> mmcblk2         menu-r  1       3551    422006  100494  0.238   82425   18069
> mmcblk2         menu-r  2       3523    508542  140085  0.275   122880  17205
> nvme0n1         menu-1  0       5407    436834  74314   0.170   54133   20181
> nvme0n1         menu-1  1       5763    459510  72673   0.158   51530   21143
> nvme0n1         menu-1  2       6266    489570  78651   0.161   58609   20042
> nvme0n1         menu-m  0       10786   767740  23840   0.031   2855    20985
> nvme0n1         menu-m  1       10586   757540  23612   0.031   2933    20679
> nvme0n1         menu-m  2       11805   834012  23528   0.028   2768    20760
> nvme0n1         menu-r  0       5323    431906  77426   0.179   56166   21260
> nvme0n1         menu-r  1       5484    438142  76033   0.174   55956   20077
> nvme0n1         menu-r  2       5353    428826  77024   0.180   57016   20008
> sda     menu-1  0       972     444116  149643  0.337   129023  20620
> sda     menu-1  1       954     557068  176479  0.317   159092  17387
> sda     menu-1  2       878     540360  196405  0.363   176792  19613
> sda     menu-m  0       1634    1017918         29614   0.029   8587    21027
> sda     menu-m  1       1622    878140  25323   0.029   8238    17085
> sda     menu-m  2       1632    1027167         28798   0.028   8428    20370
> sda     menu-r  0       918     531112  188314  0.355   168375  19939
> sda     menu-r  1       924     521378  185727  0.356   165327  20400
> sda     menu-r  2       880     529146  196391  0.371   176908  19483
> nullb0  menu-1  0       101419  88988   23923   0.269   3080    20843
> nullb0  menu-1  1       101610  88484   23678   0.268   2821    20857
> nullb0  menu-1  2       101369  89336   23711   0.265   2795    20916
> nullb0  menu-m  0       101696  88698   23860   0.269   2910    20950
> nullb0  menu-m  1       101103  88120   23294   0.264   3295    19999
> nullb0  menu-m  2       101880  86676   22730   0.262   2709    20021
> nullb0  menu-r  0       101856  87742   23493   0.268   3204    20289
> nullb0  menu-r  1       101514  89070   23653   0.266   2848    20805
> nullb0  menu-r  2       101754  86318   23163   0.268   3229    19934
> mtdblock3       menu-1  0       163     350284  115149  0.329   97166   17983
> mtdblock3       menu-1  1       179     315948  99038   0.313   78243   20795
> mtdblock3       menu-1  2       134     481584  160754  0.334   144150  16604
> mtdblock3       menu-m  0       215     410034  70261   0.171   55445   14816
> mtdblock3       menu-m  1       205     570150  109273  0.192   90189   19084
> mtdblock3       menu-m  2       252     866616  23492   0.027   9717    13775
> mtdblock3       menu-r  0       132     467365  161835  0.346   144056  17779
> mtdblock3       menu-r  1       164     348682  117704  0.338   97859   19845
> mtdblock3       menu-r  2       132     483300  165179  0.342   147164  18015

Well, this means that in the majority of cases the maximum sample idle
time is so large that UINT_MAX may as well be returned instead.

The possible correlation between idle power and the max OPP a CPU can
get to has not been taken into account in cpuidle directly so far, but
it clearly isn't true that using shallow idle states more often always
improves performance.  It may hurt performance too.

Actually, this possible correlation appears to have a broader impact,
as it may affect CAS and EAS at least in principle, so it may be
advisable to allocate some time for discussing it during upcoming
conferences.

At this point I'm inclined to revert commit 85975daeaa4d because
anything else would be clearly artificial and likely ineffective at
least in some cases.

The systems that enjoyed better performance after that commit can
switch over to teo and continue to enjoy it and everybody else still
using menu should be able to continue to do so.

RE: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Doug Smythies]

[-- Attachment #1: Type: text/plain, Size: 5364 bytes --]

Hi all,

I have been following and testing these menu.c changes over the last months,
but never reported back on this email list because:
1.) I never found anything significant to report.
2.) I always seemed to be a week or more behind the conversations.

On 2025.10.18 04:47 Rafael wrote:
> On Fri, Oct 17, 2025 at 8:37 PM Christian Loehle wrote:
>> On 10/17/25 10:39, Rafael J. Wysocki wrote:
>>> On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle wrote:
>>>> On 10/16/25 17:25, Rafael J. Wysocki wrote:
>>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>>
>>>>> It is reported that commit 85975daeaa4d ("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].

I would like to understand Sergey's benchmark test better, and even try
to repeat the results on my test system. I would also like to try to 
separate the variables in an attempt to isolate potential contributors.

To eliminate the PL1 effect, limit the CPU frequency to 2300 MHz and repeat
the test. To eliminate potential CPU frequency scaling contributions, use the
performance CPU frequency scaling governor. Both changes at once would
be an acceptable first step.

Sergey: Would you be willing to do that test?
Sergey: Could you provide more details about your test?

From the turbostat data of the other day, it seems that the system was
only power throttled for about 25 seconds for each test. What we don't know
is how long the test took overall or the magnitude of any contributions from
the power limit throttling.

Extracted PL1 area from the turbostat data:

cpu0: PKG Limit #1: ENabled (6.000000 Watts, 28.000000 sec, clamp ENabled)

	MHz		Power
Sample	revert	base	revert	base
17	1150	3039	2.15	9.49	<<< base over power limit
18	2898	3086	6.26	8.63	<<< revert over power limit
19	2968	3017	8.67	9.07
20	3054	2642	8.22	7.47
21	2910	2510	9	5.57	<<< base throttled
22	2950	2438	8.62	5.74
23	2300	2571	5.61	5.67	<<< revert throttled
24	2423	2667	5.81	6.01
25	2560	1827	5.65	2.81	<<< base not throttled
26	2478	829	5.5	1.84 
27	1552	992	2.36	1.86	<<< revert not throttled

From my testing of kernels 6.17-rc1, rc2,rc3 in August and September:

779b1a1cb13a cpuidle: governors: menu: Avoid selecting states with too much latency - v6.17-rc3
fa3fa55de0d6 cpuidle: governors: menu: Avoid using invalid recent intervals data - v6.17-rc2
baseline reference: v6.17-rc1
d4a7882f93bf cpuidle: menu: Optimize bucket assignment when next_timer_ns equals KTIME_MAX - v6.16-rc1

there was an area of about 11% regression in the 2 core ping-pong sweep test.
After modifying the PL1 limits on my test system so that it would engage for the entire test,
I re-ran the test with kernel 6.18-rc1 (ref) and also with this patch (rjw).
The results were identical for each test.
Some supporting graphs are attached.

>>>>> [snip]
>>>> [snip]
>>>> Anyway, the patch makes sense, let me run some tests and get back.
>>>
>>> Thanks!
>>
>> Unfortunately this patch regresses my tests about as much as a revert of
>> 85975daeaa4d would.
>>(menu-1 is $SUBJECT, menu-m current mainline, menu-r mainline with
>> 85975daeaa4d reverted):

So I could better understand the magnitudes of things,
Christain's test results averaged and restated:

Averages:					% regression
mmcblk1	menu-1		1502.3		36.3%
		menu-m	2356.7	
		menu-r		1483.3		37.1%

mmcblk2	menu-1		3389.0		41.1%
		menu-m	5754.7	
		menu-r		3438.3		40.3%

nvme0n1	menu-1		5812.0		47.4%
		menu-m	11059.0	
		menu-r		5386.7		51.3%

sda		menu-1		934.7		42.6%
		menu-m	1629.3	
		menu-r		907.3		44.3%

nullb0		menu-1		101466.0	0.1%
		menu-m	101559.7	
		menu-r		101708.0	-0.1%

mtdblock3	menu-1		158.7		29.2%
		menu-m	224.0	
		menu-r		142.7		36.3%

So, except for nulb0 pretty significant.

Whereas Sergey's results are the other way around by similar magnitudes.

6.1-base:	84.5	+42.0%
6.1-base-fixup:	76.5	+28.5%
6.1-revert:	59.5	
backport:	78.5	+31.9%

> Well, this means that in the majority of cases the maximum sample idle
> time is so large that UINT_MAX may as well be returned instead.
>
> The possible correlation between idle power and the max OPP a CPU can
> get to has not been taken into account in cpuidle directly so far, but
> it clearly isn't true that using shallow idle states more often always
> improves performance.  It may hurt performance too.
>
> Actually, this possible correlation appears to have a broader impact,
> as it may affect CAS and EAS at least in principle, so it may be
> advisable to allocate some time for discussing it during upcoming
> conferences.

My suggestion is to better understand Sergey's benchmark test
for potential inputs to those discussions.

> At this point I'm inclined to revert commit 85975daeaa4d because
> anything else would be clearly artificial and likely ineffective at
> least in some cases.
>
> The systems that enjoyed better performance after that commit can
> switch over to teo and continue to enjoy it and everybody else still
> using menu should be able to continue to do so.

... Doug


[-- Attachment #2: loop-times.png --]
[-- Type: image/png, Size: 41753 bytes --]

[-- Attachment #3: loop-times-relative.png --]
[-- Type: image/png, Size: 52369 bytes --]

[-- Attachment #4: loop-times-pl.png --]
[-- Type: image/png, Size: 37632 bytes --]

[-- Attachment #5: loop-times-pl-relative.png --]
[-- Type: image/png, Size: 49173 bytes --]

Re: RE: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Christian Loehle]

On 10/18/25 16:10, Doug Smythies wrote:
> Hi all,
> 
> I have been following and testing these menu.c changes over the last months,
> but never reported back on this email list because:
> 1.) I never found anything significant to report.
> 2.) I always seemed to be a week or more behind the conversations.

Your input is always appreciated!

> 
> On 2025.10.18 04:47 Rafael wrote:
>> On Fri, Oct 17, 2025 at 8:37 PM Christian Loehle wrote:
>>> On 10/17/25 10:39, Rafael J. Wysocki wrote:
>>>> On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle wrote:
>>>>> On 10/16/25 17:25, Rafael J. Wysocki wrote:
>>>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>>>
>>>>>> It is reported that commit 85975daeaa4d ("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].
> 
> I would like to understand Sergey's benchmark test better, and even try
> to repeat the results on my test system. I would also like to try to 
> separate the variables in an attempt to isolate potential contributors.
> 
> To eliminate the PL1 effect, limit the CPU frequency to 2300 MHz and repeat
> the test. To eliminate potential CPU frequency scaling contributions, use the
> performance CPU frequency scaling governor. Both changes at once would
> be an acceptable first step.
> 
> Sergey: Would you be willing to do that test?
> Sergey: Could you provide more details about your test?

+1
Depending on what the actual test does maybe offlining CPUs and comparing would
be interesting too (if this means that we never reach throttling on this system).

> 
>>From the turbostat data of the other day, it seems that the system was
> only power throttled for about 25 seconds for each test. What we don't know
> is how long the test took overall or the magnitude of any contributions from
> the power limit throttling.

If I didn't mess up it should be >800s, at least from the sum of idle time
Sergey provided. (excludes active time)
That makes the powerthrottling story less plausible IMO.

> 
> Extracted PL1 area from the turbostat data:
> 
> cpu0: PKG Limit #1: ENabled (6.000000 Watts, 28.000000 sec, clamp ENabled)
> 
> 	MHz		Power
> Sample	revert	base	revert	base
> 17	1150	3039	2.15	9.49	<<< base over power limit
> 18	2898	3086	6.26	8.63	<<< revert over power limit
> 19	2968	3017	8.67	9.07
> 20	3054	2642	8.22	7.47
> 21	2910	2510	9	5.57	<<< base throttled
> 22	2950	2438	8.62	5.74
> 23	2300	2571	5.61	5.67	<<< revert throttled
> 24	2423	2667	5.81	6.01
> 25	2560	1827	5.65	2.81	<<< base not throttled
> 26	2478	829	5.5	1.84 
> 27	1552	992	2.36	1.86	<<< revert not throttled
> 
>>From my testing of kernels 6.17-rc1, rc2,rc3 in August and September:
> 
> 779b1a1cb13a cpuidle: governors: menu: Avoid selecting states with too much latency - v6.17-rc3
> fa3fa55de0d6 cpuidle: governors: menu: Avoid using invalid recent intervals data - v6.17-rc2
> baseline reference: v6.17-rc1
> d4a7882f93bf cpuidle: menu: Optimize bucket assignment when next_timer_ns equals KTIME_MAX - v6.16-rc1
> 
> there was an area of about 11% regression in the 2 core ping-pong sweep test.
> After modifying the PL1 limits on my test system so that it would engage for the entire test,
> I re-ran the test with kernel 6.18-rc1 (ref) and also with this patch (rjw).
> The results were identical for each test.
> Some supporting graphs are attached.
> 
>>>>>> [snip]
>>>>> [snip]
>>>>> Anyway, the patch makes sense, let me run some tests and get back.
>>>>
>>>> Thanks!
>>>
>>> Unfortunately this patch regresses my tests about as much as a revert of
>>> 85975daeaa4d would.
>>> (menu-1 is $SUBJECT, menu-m current mainline, menu-r mainline with
>>> 85975daeaa4d reverted):
> 
> So I could better understand the magnitudes of things,
> Christain's test results averaged and restated:
> 
> Averages:					% regression
> mmcblk1	menu-1		1502.3		36.3%
> 		menu-m	2356.7	
> 		menu-r		1483.3		37.1%
> 
> mmcblk2	menu-1		3389.0		41.1%
> 		menu-m	5754.7	
> 		menu-r		3438.3		40.3%
> 
> nvme0n1	menu-1		5812.0		47.4%
> 		menu-m	11059.0	
> 		menu-r		5386.7		51.3%
> 
> sda		menu-1		934.7		42.6%
> 		menu-m	1629.3	
> 		menu-r		907.3		44.3%
> 
> nullb0		menu-1		101466.0	0.1%
> 		menu-m	101559.7	
> 		menu-r		101708.0	-0.1%
> 
> mtdblock3	menu-1		158.7		29.2%
> 		menu-m	224.0	
> 		menu-r		142.7		36.3%
> 
> So, except for nulb0 pretty significant.

(nullb0 is just some sanity check, cpuidle shouldn't really ever
play a role here FWIW)
Thanks for summarising, next time it'll come with such a summary as well.

> 
> Whereas Sergey's results are the other way around by similar magnitudes.
> 
> 6.1-base:	84.5	+42.0%
> 6.1-base-fixup:	76.5	+28.5%
> 6.1-revert:	59.5	
> backport:	78.5	+31.9%
> 

Re: RE: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Rafael J. Wysocki]

On Sun, Oct 19, 2025 at 4:45 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 10/18/25 16:10, Doug Smythies wrote:
> > Hi all,
> >
> > I have been following and testing these menu.c changes over the last months,
> > but never reported back on this email list because:
> > 1.) I never found anything significant to report.
> > 2.) I always seemed to be a week or more behind the conversations.
>
> Your input is always appreciated!

Indeed.

> >
> > On 2025.10.18 04:47 Rafael wrote:
> >> On Fri, Oct 17, 2025 at 8:37 PM Christian Loehle wrote:
> >>> On 10/17/25 10:39, Rafael J. Wysocki wrote:
> >>>> On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle wrote:
> >>>>> On 10/16/25 17:25, Rafael J. Wysocki wrote:
> >>>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>>>>
> >>>>>> It is reported that commit 85975daeaa4d ("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].
> >
> > I would like to understand Sergey's benchmark test better, and even try
> > to repeat the results on my test system. I would also like to try to
> > separate the variables in an attempt to isolate potential contributors.
> >
> > To eliminate the PL1 effect, limit the CPU frequency to 2300 MHz and repeat
> > the test. To eliminate potential CPU frequency scaling contributions, use the
> > performance CPU frequency scaling governor. Both changes at once would
> > be an acceptable first step.
> >
> > Sergey: Would you be willing to do that test?
> > Sergey: Could you provide more details about your test?
>
> +1
> Depending on what the actual test does maybe offlining CPUs and comparing would
> be interesting too (if this means that we never reach throttling on this system).

While it would be kind of interesting to know the test details, I
don't think that this is just one test.

Sergey mentioned several different symptoms in his initial message:

https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/

which kind of indicates several different tests regressing, so this
appears to be a whole-platform issue.

> >>From the turbostat data of the other day, it seems that the system was
> > only power throttled for about 25 seconds for each test. What we don't know
> > is how long the test took overall or the magnitude of any contributions from
> > the power limit throttling.
>
> If I didn't mess up it should be >800s, at least from the sum of idle time
> Sergey provided. (excludes active time)
> That makes the powerthrottling story less plausible IMO.

Quite evidently, there is a correlation between the max CPU ("busy")
frequency and the time spent in core C7 on that system.

The only explanation that I can offer is a firmware mechanism turning
spare power into a CPU boost.

RAPL is such a mechanism and it doesn't throttle strictly speaking,
but it prevents the CPU package (in the case of PL1) from using more
energy than it is allowed to use over a given time frame.  One way to
achieve that is to allow CPUs to run fast at the beginning of the
measurement window and then throttle them below a certain power level,
but it is not the only way and it is not likely to be used.  Moreover,
it is unlikely that the time spent in C7 will affect that because that
time is not known when the measurement window starts.

Another approach is to keep the package power on a "trajectory" to
meet the goal and adjust periodically given what all of the CPUs are
doing.  In that case, it will throttle sometimes when the direction of
changes is mispredicted, but overall it will set OPPs with certain
expectation regarding the trend.

Also, on some platforms high-turgo OPPs are "locked" when deep core
idle states (typically C6 and above) are not utilized, but I'm not
aware of that being done on Jasper Lake.

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Sergey Senozhatsky]

On (25/10/18 08:10), Doug Smythies wrote:
> On 2025.10.18 04:47 Rafael wrote:
> > On Fri, Oct 17, 2025 at 8:37 PM Christian Loehle wrote:
> >> On 10/17/25 10:39, Rafael J. Wysocki wrote:
> >>> On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle wrote:
> >>>> On 10/16/25 17:25, Rafael J. Wysocki wrote:
> >>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>>>
> >>>>> It is reported that commit 85975daeaa4d ("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].
> 
> I would like to understand Sergey's benchmark test better, and even try
> to repeat the results on my test system. I would also like to try to 
> separate the variables in an attempt to isolate potential contributors.
> 
> To eliminate the PL1 effect, limit the CPU frequency to 2300 MHz and repeat
> the test. To eliminate potential CPU frequency scaling contributions, use the
> performance CPU frequency scaling governor. Both changes at once would
> be an acceptable first step.
> 
> Sergey: Would you be willing to do that test?

Apologies for the delay.

Sure, I can give it a try sometime this week, am dealing with a bunch
of other stable regressions right now (will report separately).

Can you please help me with the configuration steps?  CPU freq limiting,
etc.

> Sergey: Could you provide more details about your test?

We track regressions in a number of tests.  The one I'm running more
often than others is a Google Docs test (our tests replicate real use
cases).  The test in question creates new google docs (in chrome, of
course) and inputs some text in them (with various words-per-minute
settings - 60, 90, 120 wpm) in English, Japanese, Korean and other
languages; different font faces, different styles (bold, italic),
text highlighting/selection, windows switching, and so on.  The test
measures input latency, the number of dropped frames during scrolling,
CPU usage, power consumption, etc.

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Sergey Senozhatsky]

On (25/10/20 15:18), Rafael J. Wysocki wrote:
> While it would be kind of interesting to know the test details, I
> don't think that this is just one test.
> 
> Sergey mentioned several different symptoms in his initial message:
> 
> https://lore.kernel.org/linux-pm/36iykr223vmcfsoysexug6s274nq2oimcu55ybn6ww4il3g3cv@cohflgdbpnq7/
> 
> which kind of indicates several different tests regressing, so this
> appears to be a whole-platform issue.

Correct, different test scenarios:

GoogleDocs, VideoPlayback, VideoCall, and perhaps some others.

RE: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Doug Smythies]

On 2025.10.20 20:43 Sergey Senozhatsky wrote:
> On (25/10/18 08:10), Doug Smythies wrote:
>> On 2025.10.18 04:47 Rafael wrote:
>>> On Fri, Oct 17, 2025 at 8:37 PM Christian Loehle wrote:
>>>> On 10/17/25 10:39, Rafael J. Wysocki wrote:
>>>>> On Fri, Oct 17, 2025 at 10:22 AM Christian Loehle wrote:
>>>>>> On 10/16/25 17:25, Rafael J. Wysocki wrote:
>>>>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>>>>
>>>>>>> It is reported that commit 85975daeaa4d ("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].
>> 
>> I would like to understand Sergey's benchmark test better, and even try
>> to repeat the results on my test system. I would also like to try to 
>> separate the variables in an attempt to isolate potential contributors.
>> 
>> To eliminate the PL1 effect, limit the CPU frequency to 2300 MHz and repeat
>> the test. To eliminate potential CPU frequency scaling contributions, use the
>> performance CPU frequency scaling governor. Both changes at once would
>> be an acceptable first step.
>> 
>> Sergey: Would you be willing to do that test?
>
> Apologies for the delay.
>
> Sure, I can give it a try sometime this week, am dealing with a bunch
> of other stable regressions right now (will report separately).
>
> Can you please help me with the configuration steps?  CPU freq limiting,
> etc.

For your system booted with "base" and "revert" do:

echo performance | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
echo 2300000 | sudo tee /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq

then do your test.

>
>> Sergey: Could you provide more details about your test?
>
> We track regressions in a number of tests.  The one I'm running more
> often than others is a Google Docs test (our tests replicate real use
> cases).  The test in question creates new google docs (in chrome, of
> course) and inputs some text in them (with various words-per-minute
> settings - 60, 90, 120 wpm) in English, Japanese, Korean and other
> languages; different font faces, different styles (bold, italic),
> text highlighting/selection, windows switching, and so on.  The test
> measures input latency, the number of dropped frames during scrolling,
> CPU usage, power consumption, etc.

Okay, Thanks. So not a test I can repeat on my test computer.

... Doug

Re: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Sergey Senozhatsky]

On (25/10/21 07:30), Doug Smythies wrote:
> For your system booted with "base" and "revert" do:
> 
> echo performance | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
> echo 2300000 | sudo tee /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq

Alright, here are the results:

~ # echo performance | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
performance
~ # echo 2300000 | sudo tee /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq
2300000

Base:		52.5
Revert:		45.5

RE: [PATCH v1] cpuidle: governors: menu: Predict longer idle time when in doubt

[Doug Smythies]

On 2025.10.22 01:01 Sergey Senozhatsky wrote:
> On (25/10/21 07:30), Doug Smythies wrote:
>> For your system booted with "base" and "revert" do:
>> 
>> echo performance | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
>> echo 2300000 | sudo tee /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq
>
> Alright, here are the results:
> 
> ~ # echo performance | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
> performance
> ~ # echo 2300000 | sudo tee /sys/devices/system/cpu/cpufreq/policy*/scaling_max_freq
> 2300000
>
> Base:		52.5
> Revert:	45.5

So, still 15.4 %.

Thank you very much.
I wish I could create a test that would show such results on my test computer and believe me I have tried.
I haven't been able to.

... Doug