[PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[Hongyan Xia]

From: Hongyan Xia <hongyan.xia@transsion.com>

If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
should then add or subtract task runnable_avg, but the arithmetic below
is still with task util_avg. This mixes runnable_avg with util_avg which
is incorrect.

Fix by always doing arithmetic with runnable_avg and only take
max(runnable_avg, util_avg) at the last step.

Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>

---
Changed in v2:
- Rebase against the latest sched/core
---
 kernel/sched/fair.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f4ed841f766f..1b23e73f48b0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8968,25 +8968,32 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
 static unsigned long
 cpu_util(int cpu, struct task_struct *p, int dst_cpu, int boost)
 {
+	bool add_task = p && task_cpu(p) != cpu && dst_cpu == cpu;
+	bool sub_task = p && task_cpu(p) == cpu && dst_cpu != cpu;
 	struct cfs_rq *cfs_rq = &cpu_rq(cpu)->cfs;
 	unsigned long util = READ_ONCE(cfs_rq->avg.util_avg);
 	unsigned long runnable;
 
-	if (boost) {
-		runnable = READ_ONCE(cfs_rq->avg.runnable_avg);
-		util = max(util, runnable);
-	}
-
 	/*
 	 * If @dst_cpu is -1 or @p migrates from @cpu to @dst_cpu remove its
 	 * contribution. If @p migrates from another CPU to @cpu add its
 	 * contribution. In all the other cases @cpu is not impacted by the
 	 * migration so its util_avg is already correct.
 	 */
-	if (p && task_cpu(p) == cpu && dst_cpu != cpu)
-		lsub_positive(&util, task_util(p));
-	else if (p && task_cpu(p) != cpu && dst_cpu == cpu)
+	if (add_task)
 		util += task_util(p);
+	else if (sub_task)
+		lsub_positive(&util, task_util(p));
+
+	if (boost) {
+		runnable = READ_ONCE(cfs_rq->avg.runnable_avg);
+		if (add_task)
+			runnable += READ_ONCE(p->se.avg.runnable_avg);
+		else if (sub_task)
+			lsub_positive(&runnable,
+				      READ_ONCE(p->se.avg.runnable_avg));
+		util = max(util, runnable);
+	}
 
 	if (sched_feat(UTIL_EST)) {
 		unsigned long util_est;
-- 
2.47.3

Re: [PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[Dietmar Eggemann]

On 05.06.26 11:43, Hongyan Xia wrote:
> From: Hongyan Xia <hongyan.xia@transsion.com>
> 
> If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
> should then add or subtract task runnable_avg, but the arithmetic below
> is still with task util_avg. This mixes runnable_avg with util_avg which
> is incorrect.
> 
> Fix by always doing arithmetic with runnable_avg and only take
> max(runnable_avg, util_avg) at the last step.
> 
> Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>

Does this fix the issue in EAS energy calculation you mentioned
initially? We now add/subtract task rbl_avg from CPU rbl_avg but can we
now use this value correctly in util_avg based EAS?

How do you want to solve the power consumption regression in you
low-power use cases? Since you mentioned per-CPU tasks in those
contention scenarios (per-CPU worker vs producer *), do you plan to only
use boost in cpu_util() in case the affinity of p (worker) is not
constrained? Not sure whether the consumer (CPU affinity not
constrained) also has rbl_avg > util_avg?

*
https://lore.kernel.org/r/4adbab4d-f9e4-4354-aa1e-48f11b1fd208@transsion.com


[...]

Re: [PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[Hongyan Xia]

On 6/5/2026 6:35 PM, Dietmar Eggemann wrote:
> On 05.06.26 11:43, Hongyan Xia wrote:
>> From: Hongyan Xia <hongyan.xia@transsion.com>
>>
>> If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
>> should then add or subtract task runnable_avg, but the arithmetic below
>> is still with task util_avg. This mixes runnable_avg with util_avg which
>> is incorrect.
>>
>> Fix by always doing arithmetic with runnable_avg and only take
>> max(runnable_avg, util_avg) at the last step.
>>
>> Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
>> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
>
> Does this fix the issue in EAS energy calculation you mentioned
> initially? We now add/subtract task rbl_avg from CPU rbl_avg but can we
> now use this value correctly in util_avg based EAS?

It does improve things a bit. It used to occasionally pile up tasks on
the same CPU, which happens less after this patch. At least it now gets
the maths right so this fix should probably be in regardless.

> How do you want to solve the power consumption regression in you
> low-power use cases? Since you mentioned per-CPU tasks in those
> contention scenarios (per-CPU worker vs producer *), do you plan to only
> use boost in cpu_util() in case the affinity of p (worker) is not
> constrained? Not sure whether the consumer (CPU affinity not
> constrained) also has rbl_avg > util_avg?

This patch only gets the maths right but the energy regression is still
big because frequency hasn't changed. The producer-consumer is only the
worst offender, not the only one. Trouble is that runnable_avg is just a
big number to deal with in general, and you could easily double or
triple your frequency if you have many small threads around (which is
the case in our mobile cases).

We haven't found a good solution to solve it completely. I keep
wondering if there could be a better metric than raw runnable_avg. One
that is not so big in magnitude and does a much better job to tell the
true contention where boosting frequency helps.

> *
> https://lore.kernel.org/r/4adbab4d-f9e4-4354-aa1e-48f11b1fd208@transsion.com
>
>
> [...]

Re: [PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[Dietmar Eggemann]

On 05.06.26 15:15, Hongyan Xia wrote:
> On 6/5/2026 6:35 PM, Dietmar Eggemann wrote:
>> On 05.06.26 11:43, Hongyan Xia wrote:
>>> From: Hongyan Xia <hongyan.xia@transsion.com>
>>>
>>> If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
>>> should then add or subtract task runnable_avg, but the arithmetic below
>>> is still with task util_avg. This mixes runnable_avg with util_avg which
>>> is incorrect.
>>>
>>> Fix by always doing arithmetic with runnable_avg and only take
>>> max(runnable_avg, util_avg) at the last step.
>>>
>>> Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
>>> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
>>
>> Does this fix the issue in EAS energy calculation you mentioned
>> initially? We now add/subtract task rbl_avg from CPU rbl_avg but can we
>> now use this value correctly in util_avg based EAS?
> 
> It does improve things a bit. It used to occasionally pile up tasks on
> the same CPU, which happens less after this patch. At least it now gets
> the maths right so this fix should probably be in regardless.

Just to map this into the code:

--- EAS ---:

compute_energy(..., p, dst_cpu)

  max_util = eenv_pd_max_util(..., p, dst_cpu)

               for_each_cpu(cpu, pd_cpus)
 
                 util = cpu_util(cpu, p, dst_cpu, 1)
                                                  ^ boost

  energy = em_pd_get_efficient_state(..., max_util)

    for (i = min_ps; i <= max_ps; i++)

      if (ps->performance >= max_util)
        return i 


--- schedutil ---:

sugov_get_util(cpu)

  util =+ cpu_util_cfs_boost(cpu)                                             

            util = cpu_util(cpu, NULL, -1, 1) --> p == NULL, dst_cpu == -1
		                           ^ boost



This can help to calculate a more correct max_util value on the EAS-side,
but won't change schedutil?

I agree that it's more correct to add/subract task runnable in case of
migration but using runnable instead of util vs capacity is still not
'correct'? 

>> How do you want to solve the power consumption regression in you
>> low-power use cases? Since you mentioned per-CPU tasks in those
>> contention scenarios (per-CPU worker vs producer *), do you plan to only
>> use boost in cpu_util() in case the affinity of p (worker) is not
>> constrained? Not sure whether the consumer (CPU affinity not
>> constrained) also has rbl_avg > util_avg?
> 
> This patch only gets the maths right but the energy regression is still
> big because frequency hasn't changed. The producer-consumer is only the
> worst offender, not the only one. Trouble is that runnable_avg is just a
> big number to deal with in general, and you could easily double or
> triple your frequency if you have many small threads around (which is
> the case in our mobile cases).
> 
> We haven't found a good solution to solve it completely. I keep
> wondering if there could be a better metric than raw runnable_avg. One
> that is not so big in magnitude and does a much better job to tell the
> true contention where boosting frequency helps.
> 
>> *
>> https://lore.kernel.org/r/4adbab4d-f9e4-4354-aa1e-48f11b1fd208@transsion.com
>>
>>
>> [...]

Re: [PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[Hongyan Xia]

On 6/9/2026 3:04 PM, Dietmar Eggemann wrote:
> On 05.06.26 15:15, Hongyan Xia wrote:
>> On 6/5/2026 6:35 PM, Dietmar Eggemann wrote:
>>> On 05.06.26 11:43, Hongyan Xia wrote:
>>>> From: Hongyan Xia <hongyan.xia@transsion.com>
>>>>
>>>> If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
>>>> should then add or subtract task runnable_avg, but the arithmetic below
>>>> is still with task util_avg. This mixes runnable_avg with util_avg which
>>>> is incorrect.
>>>>
>>>> Fix by always doing arithmetic with runnable_avg and only take
>>>> max(runnable_avg, util_avg) at the last step.
>>>>
>>>> Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
>>>> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
>>>
>>> Does this fix the issue in EAS energy calculation you mentioned
>>> initially? We now add/subtract task rbl_avg from CPU rbl_avg but can we
>>> now use this value correctly in util_avg based EAS?
>>
>> It does improve things a bit. It used to occasionally pile up tasks on
>> the same CPU, which happens less after this patch. At least it now gets
>> the maths right so this fix should probably be in regardless.
>
> Just to map this into the code:
>
> --- EAS ---:
>
> compute_energy(..., p, dst_cpu)
>
>    max_util = eenv_pd_max_util(..., p, dst_cpu)
>
>                 for_each_cpu(cpu, pd_cpus)
>
>                   util = cpu_util(cpu, p, dst_cpu, 1)
>                                                    ^ boost
>
>    energy = em_pd_get_efficient_state(..., max_util)
>
>      for (i = min_ps; i <= max_ps; i++)
>
>        if (ps->performance >= max_util)
>          return i
>
>
> --- schedutil ---:
>
> sugov_get_util(cpu)
>
>    util =+ cpu_util_cfs_boost(cpu)
>
>              util = cpu_util(cpu, NULL, -1, 1) --> p == NULL, dst_cpu == -1
>                                          ^ boost
>
>
>
> This can help to calculate a more correct max_util value on the EAS-side,
> but won't change schedutil?

Yes. The CPUFreq side has p == NULL so this patch won't affect how
frequency is calculated.

> I agree that it's more correct to add/subract task runnable in case of
> migration but using runnable instead of util vs capacity is still not
> 'correct'?

I guess we have two problems.

1. Adding/subtracting util from runnable_avg in EAS.
2. Using runnable_avg for frequency.

1 is incorrect which I think all of us agree. For 2, not sure if I want
to call it 'wrong', but it's just runnable_avg can grow really big
really quickly, and we want a more sensible value.

>>> How do you want to solve the power consumption regression in you
>>> low-power use cases? Since you mentioned per-CPU tasks in those
>>> contention scenarios (per-CPU worker vs producer *), do you plan to only
>>> use boost in cpu_util() in case the affinity of p (worker) is not
>>> constrained? Not sure whether the consumer (CPU affinity not
>>> constrained) also has rbl_avg > util_avg?
>>
>> This patch only gets the maths right but the energy regression is still
>> big because frequency hasn't changed. The producer-consumer is only the
>> worst offender, not the only one. Trouble is that runnable_avg is just a
>> big number to deal with in general, and you could easily double or
>> triple your frequency if you have many small threads around (which is
>> the case in our mobile cases).
>>
>> We haven't found a good solution to solve it completely. I keep
>> wondering if there could be a better metric than raw runnable_avg. One
>> that is not so big in magnitude and does a much better job to tell the
>> true contention where boosting frequency helps.
>>
>>> *
>>> https://lore.kernel.org/r/4adbab4d-f9e4-4354-aa1e-48f11b1fd208@transsion.com
>>>
>>>
>>> [...]

Re: [PATCH v2] sched/fair: Fix cpu_util runnable_avg arithmetic

[Vincent Guittot]

On Fri, 5 Jun 2026 at 11:43, Hongyan Xia <hongyan.xia@transsion.com> wrote:
>
> From: Hongyan Xia <hongyan.xia@transsion.com>
>
> If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
> should then add or subtract task runnable_avg, but the arithmetic below
> is still with task util_avg. This mixes runnable_avg with util_avg which
> is incorrect.
>
> Fix by always doing arithmetic with runnable_avg and only take
> max(runnable_avg, util_avg) at the last step.
>
> Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

>
> ---
> Changed in v2:
> - Rebase against the latest sched/core
> ---
>  kernel/sched/fair.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index f4ed841f766f..1b23e73f48b0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8968,25 +8968,32 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  static unsigned long
>  cpu_util(int cpu, struct task_struct *p, int dst_cpu, int boost)
>  {
> +       bool add_task = p && task_cpu(p) != cpu && dst_cpu == cpu;
> +       bool sub_task = p && task_cpu(p) == cpu && dst_cpu != cpu;
>         struct cfs_rq *cfs_rq = &cpu_rq(cpu)->cfs;
>         unsigned long util = READ_ONCE(cfs_rq->avg.util_avg);
>         unsigned long runnable;
>
> -       if (boost) {
> -               runnable = READ_ONCE(cfs_rq->avg.runnable_avg);
> -               util = max(util, runnable);
> -       }
> -
>         /*
>          * If @dst_cpu is -1 or @p migrates from @cpu to @dst_cpu remove its
>          * contribution. If @p migrates from another CPU to @cpu add its
>          * contribution. In all the other cases @cpu is not impacted by the
>          * migration so its util_avg is already correct.
>          */
> -       if (p && task_cpu(p) == cpu && dst_cpu != cpu)
> -               lsub_positive(&util, task_util(p));
> -       else if (p && task_cpu(p) != cpu && dst_cpu == cpu)
> +       if (add_task)
>                 util += task_util(p);
> +       else if (sub_task)
> +               lsub_positive(&util, task_util(p));
> +
> +       if (boost) {
> +               runnable = READ_ONCE(cfs_rq->avg.runnable_avg);
> +               if (add_task)
> +                       runnable += READ_ONCE(p->se.avg.runnable_avg);
> +               else if (sub_task)
> +                       lsub_positive(&runnable,
> +                                     READ_ONCE(p->se.avg.runnable_avg));
> +               util = max(util, runnable);
> +       }
>
>         if (sched_feat(UTIL_EST)) {
>                 unsigned long util_est;
> --
> 2.47.3
>

[tip: sched/core] sched/fair: Fix cpu_util runnable_avg arithmetic

[tip-bot2 for Hongyan Xia]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     29922fdfc2a4008d66418bedd0ebf5038fc54efa
Gitweb:        https://git.kernel.org/tip/29922fdfc2a4008d66418bedd0ebf5038fc54efa
Author:        Hongyan Xia <hongyan.xia@transsion.com>
AuthorDate:    Fri, 05 Jun 2026 09:43:39 
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 09 Jun 2026 10:28:08 +02:00

sched/fair: Fix cpu_util runnable_avg arithmetic

If we take runnable_avg in max(runnable_avg, util_avg) in cpu_util(), we
should then add or subtract task runnable_avg, but the arithmetic below
is still with task util_avg. This mixes runnable_avg with util_avg which
is incorrect.

Fix by always doing arithmetic with runnable_avg and only take
max(runnable_avg, util_avg) at the last step.

Fixes: 7d0583cf9ec7 ("sched/fair, cpufreq: Introduce 'runnable boosting'")
Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://patch.msgid.link/20260605094318.37931-1-hongyan.xia@transsion.com
---
 kernel/sched/fair.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f4ed841..1b23e73 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8968,25 +8968,32 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
 static unsigned long
 cpu_util(int cpu, struct task_struct *p, int dst_cpu, int boost)
 {
+	bool add_task = p && task_cpu(p) != cpu && dst_cpu == cpu;
+	bool sub_task = p && task_cpu(p) == cpu && dst_cpu != cpu;
 	struct cfs_rq *cfs_rq = &cpu_rq(cpu)->cfs;
 	unsigned long util = READ_ONCE(cfs_rq->avg.util_avg);
 	unsigned long runnable;
 
-	if (boost) {
-		runnable = READ_ONCE(cfs_rq->avg.runnable_avg);
-		util = max(util, runnable);
-	}
-
 	/*
 	 * If @dst_cpu is -1 or @p migrates from @cpu to @dst_cpu remove its
 	 * contribution. If @p migrates from another CPU to @cpu add its
 	 * contribution. In all the other cases @cpu is not impacted by the
 	 * migration so its util_avg is already correct.
 	 */
-	if (p && task_cpu(p) == cpu && dst_cpu != cpu)
-		lsub_positive(&util, task_util(p));
-	else if (p && task_cpu(p) != cpu && dst_cpu == cpu)
+	if (add_task)
 		util += task_util(p);
+	else if (sub_task)
+		lsub_positive(&util, task_util(p));
+
+	if (boost) {
+		runnable = READ_ONCE(cfs_rq->avg.runnable_avg);
+		if (add_task)
+			runnable += READ_ONCE(p->se.avg.runnable_avg);
+		else if (sub_task)
+			lsub_positive(&runnable,
+				      READ_ONCE(p->se.avg.runnable_avg));
+		util = max(util, runnable);
+	}
 
 	if (sched_feat(UTIL_EST)) {
 		unsigned long util_est;