[v4,4/6] sched/fair: update cpu_capcity to reflect thermal pressure

Message ID 1571776465-29763-5-git-send-email-thara.gopinath@linaro.org
State Superseded
Headers show
Series
  • Introduce Thermal Pressure
Related show

Commit Message

Thara Gopinath Oct. 22, 2019, 8:34 p.m.
cpu_capacity relflects the maximum available capacity of a cpu. Thermal
pressure on a cpu means this maximum available capacity is reduced. This
patch reduces the average thermal pressure for a cpu from its maximum
available capacity so that cpu_capacity reflects the actual
available capacity.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

---
 kernel/sched/fair.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.1.4

Comments

Qais Yousef Oct. 23, 2019, 12:28 p.m. | #1
On 10/22/19 16:34, Thara Gopinath wrote:
> cpu_capacity relflects the maximum available capacity of a cpu. Thermal

> pressure on a cpu means this maximum available capacity is reduced. This

> patch reduces the average thermal pressure for a cpu from its maximum

> available capacity so that cpu_capacity reflects the actual

> available capacity.

> 

> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> ---

>  kernel/sched/fair.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> index 4f9c2cb..be3e802 100644

> --- a/kernel/sched/fair.c

> +++ b/kernel/sched/fair.c

> @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

>  

>  	used = READ_ONCE(rq->avg_rt.util_avg);

>  	used += READ_ONCE(rq->avg_dl.util_avg);

> +	used += READ_ONCE(rq->avg_thermal.load_avg);


Maybe a naive question - but can we add util_avg with load_avg without
a conversion? I thought the 2 signals have different properties.

Cheers

--
Qais Yousef

>  

>  	if (unlikely(used >= max))

>  		return 1;

> -- 

> 2.1.4

>
Peter Zijlstra Oct. 28, 2019, 3:30 p.m. | #2
On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:
> On 10/22/19 16:34, Thara Gopinath wrote:

> > cpu_capacity relflects the maximum available capacity of a cpu. Thermal

> > pressure on a cpu means this maximum available capacity is reduced. This

> > patch reduces the average thermal pressure for a cpu from its maximum

> > available capacity so that cpu_capacity reflects the actual

> > available capacity.

> > 

> > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> > ---

> >  kernel/sched/fair.c | 1 +

> >  1 file changed, 1 insertion(+)

> > 

> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> > index 4f9c2cb..be3e802 100644

> > --- a/kernel/sched/fair.c

> > +++ b/kernel/sched/fair.c

> > @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

> >  

> >  	used = READ_ONCE(rq->avg_rt.util_avg);

> >  	used += READ_ONCE(rq->avg_dl.util_avg);

> > +	used += READ_ONCE(rq->avg_thermal.load_avg);

> 

> Maybe a naive question - but can we add util_avg with load_avg without

> a conversion? I thought the 2 signals have different properties.


Changelog of patch #1 explains, it's in that dense blob of text.

But yes, you're quite right that that wants a comment here.
Qais Yousef Oct. 31, 2019, 10:53 a.m. | #3
On 10/28/19 16:30, Peter Zijlstra wrote:
> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

> > On 10/22/19 16:34, Thara Gopinath wrote:

> > > cpu_capacity relflects the maximum available capacity of a cpu. Thermal

> > > pressure on a cpu means this maximum available capacity is reduced. This

> > > patch reduces the average thermal pressure for a cpu from its maximum

> > > available capacity so that cpu_capacity reflects the actual

> > > available capacity.

> > > 

> > > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> > > ---

> > >  kernel/sched/fair.c | 1 +

> > >  1 file changed, 1 insertion(+)

> > > 

> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> > > index 4f9c2cb..be3e802 100644

> > > --- a/kernel/sched/fair.c

> > > +++ b/kernel/sched/fair.c

> > > @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

> > >  

> > >  	used = READ_ONCE(rq->avg_rt.util_avg);

> > >  	used += READ_ONCE(rq->avg_dl.util_avg);

> > > +	used += READ_ONCE(rq->avg_thermal.load_avg);

> > 

> > Maybe a naive question - but can we add util_avg with load_avg without

> > a conversion? I thought the 2 signals have different properties.

> 

> Changelog of patch #1 explains, it's in that dense blob of text.

> 

> But yes, you're quite right that that wants a comment here.


Thanks for the pointer! A comment would be nice indeed.

To make sure I got this correctly - it's because avg_thermal.load_avg
represents delta_capacity which is already a 'converted' form of load. So this
makes avg_thermal.load_avg a util_avg really. Correct?

If I managed to get it right somehow. It'd be nice if we can do inverse
conversion on delta_capacity so that avg_thermal.{load_avg, util_avg} meaning
is consistent across the board. But I don't feel strongly about it if this gets
documented properly.

Thanks

--
Qais Yousef
Dietmar Eggemann Oct. 31, 2019, 3:38 p.m. | #4
On 31.10.19 11:53, Qais Yousef wrote:
> On 10/28/19 16:30, Peter Zijlstra wrote:

>> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

>>> On 10/22/19 16:34, Thara Gopinath wrote:

>>>> cpu_capacity relflects the maximum available capacity of a cpu. Thermal

>>>> pressure on a cpu means this maximum available capacity is reduced. This

>>>> patch reduces the average thermal pressure for a cpu from its maximum

>>>> available capacity so that cpu_capacity reflects the actual

>>>> available capacity.

>>>>

>>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

>>>> ---

>>>>  kernel/sched/fair.c | 1 +

>>>>  1 file changed, 1 insertion(+)

>>>>

>>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

>>>> index 4f9c2cb..be3e802 100644

>>>> --- a/kernel/sched/fair.c

>>>> +++ b/kernel/sched/fair.c

>>>> @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

>>>>  

>>>>  	used = READ_ONCE(rq->avg_rt.util_avg);

>>>>  	used += READ_ONCE(rq->avg_dl.util_avg);

>>>> +	used += READ_ONCE(rq->avg_thermal.load_avg);

>>>

>>> Maybe a naive question - but can we add util_avg with load_avg without

>>> a conversion? I thought the 2 signals have different properties.

>>

>> Changelog of patch #1 explains, it's in that dense blob of text.

>>

>> But yes, you're quite right that that wants a comment here.

> 

> Thanks for the pointer! A comment would be nice indeed.

> 

> To make sure I got this correctly - it's because avg_thermal.load_avg

> represents delta_capacity which is already a 'converted' form of load. So this

> makes avg_thermal.load_avg a util_avg really. Correct?

> 

> If I managed to get it right somehow. It'd be nice if we can do inverse

> conversion on delta_capacity so that avg_thermal.{load_avg, util_avg} meaning

> is consistent across the board. But I don't feel strongly about it if this gets

> documented properly.


So why can't we use rq->avg_thermal.util_avg here? Since capacity is
closer to util than to load?

Is it because you want to use the influence of ___update_load_sum(...,
unsigned long load eq. per-cpu delta_capacity in your signal?

Why not call it this way then?

diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
index 38210691c615..d3035457483f 100644
--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -357,9 +357,9 @@ int update_thermal_load_avg(u64 now, struct rq *rq,
u64 capacity)
 {
        if (___update_load_sum(now, &rq->avg_thermal,
                               capacity,
-                              capacity,
-                              capacity)) {
-               ___update_load_avg(&rq->avg_thermal, 1, 1);
+                              0,
+                              0)) {
+               ___update_load_avg(&rq->avg_thermal, 1, 0);
                return 1;
        }
Vincent Guittot Oct. 31, 2019, 3:48 p.m. | #5
On Thu, 31 Oct 2019 at 16:38, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>

> On 31.10.19 11:53, Qais Yousef wrote:

> > On 10/28/19 16:30, Peter Zijlstra wrote:

> >> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

> >>> On 10/22/19 16:34, Thara Gopinath wrote:

> >>>> cpu_capacity relflects the maximum available capacity of a cpu. Thermal

> >>>> pressure on a cpu means this maximum available capacity is reduced. This

> >>>> patch reduces the average thermal pressure for a cpu from its maximum

> >>>> available capacity so that cpu_capacity reflects the actual

> >>>> available capacity.

> >>>>

> >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> >>>> ---

> >>>>  kernel/sched/fair.c | 1 +

> >>>>  1 file changed, 1 insertion(+)

> >>>>

> >>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> >>>> index 4f9c2cb..be3e802 100644

> >>>> --- a/kernel/sched/fair.c

> >>>> +++ b/kernel/sched/fair.c

> >>>> @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

> >>>>

> >>>>    used = READ_ONCE(rq->avg_rt.util_avg);

> >>>>    used += READ_ONCE(rq->avg_dl.util_avg);

> >>>> +  used += READ_ONCE(rq->avg_thermal.load_avg);

> >>>

> >>> Maybe a naive question - but can we add util_avg with load_avg without

> >>> a conversion? I thought the 2 signals have different properties.

> >>

> >> Changelog of patch #1 explains, it's in that dense blob of text.

> >>

> >> But yes, you're quite right that that wants a comment here.

> >

> > Thanks for the pointer! A comment would be nice indeed.

> >

> > To make sure I got this correctly - it's because avg_thermal.load_avg

> > represents delta_capacity which is already a 'converted' form of load. So this

> > makes avg_thermal.load_avg a util_avg really. Correct?

> >

> > If I managed to get it right somehow. It'd be nice if we can do inverse

> > conversion on delta_capacity so that avg_thermal.{load_avg, util_avg} meaning

> > is consistent across the board. But I don't feel strongly about it if this gets

> > documented properly.

>

> So why can't we use rq->avg_thermal.util_avg here? Since capacity is

> closer to util than to load?

>

> Is it because you want to use the influence of ___update_load_sum(...,

> unsigned long load eq. per-cpu delta_capacity in your signal?

>

> Why not call it this way then?


util_avg tracks a binary state with 2 fixed weights: running(1024)  vs
not running (0)
In the case of thermal pressure, we want to track how much pressure is
put on the CPU: capping to half the max frequency is not the same as
capping only 10%
load_avg is not boolean but you set the weight  you want to apply and
this weight reflects the amount of pressure.

>

> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c

> index 38210691c615..d3035457483f 100644

> --- a/kernel/sched/pelt.c

> +++ b/kernel/sched/pelt.c

> @@ -357,9 +357,9 @@ int update_thermal_load_avg(u64 now, struct rq *rq,

> u64 capacity)

>  {

>         if (___update_load_sum(now, &rq->avg_thermal,

>                                capacity,

> -                              capacity,

> -                              capacity)) {

> -               ___update_load_avg(&rq->avg_thermal, 1, 1);

> +                              0,

> +                              0)) {

> +               ___update_load_avg(&rq->avg_thermal, 1, 0);

>                 return 1;

>         }
Thara Gopinath Oct. 31, 2019, 4:03 p.m. | #6
On 10/31/2019 06:53 AM, Qais Yousef wrote:
> On 10/28/19 16:30, Peter Zijlstra wrote:

>> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

>>> On 10/22/19 16:34, Thara Gopinath wrote:

>>>> cpu_capacity relflects the maximum available capacity of a cpu. Thermal

>>>> pressure on a cpu means this maximum available capacity is reduced. This

>>>> patch reduces the average thermal pressure for a cpu from its maximum

>>>> available capacity so that cpu_capacity reflects the actual

>>>> available capacity.

>>>>

>>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

>>>> ---

>>>>  kernel/sched/fair.c | 1 +

>>>>  1 file changed, 1 insertion(+)

>>>>

>>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

>>>> index 4f9c2cb..be3e802 100644

>>>> --- a/kernel/sched/fair.c

>>>> +++ b/kernel/sched/fair.c

>>>> @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

>>>>  

>>>>  	used = READ_ONCE(rq->avg_rt.util_avg);

>>>>  	used += READ_ONCE(rq->avg_dl.util_avg);

>>>> +	used += READ_ONCE(rq->avg_thermal.load_avg);

>>>

>>> Maybe a naive question - but can we add util_avg with load_avg without

>>> a conversion? I thought the 2 signals have different properties.

>>

>> Changelog of patch #1 explains, it's in that dense blob of text.

>>

>> But yes, you're quite right that that wants a comment here.

> 

> Thanks for the pointer! A comment would be nice indeed.

> 

> To make sure I got this correctly - it's because avg_thermal.load_avg

> represents delta_capacity which is already a 'converted' form of load. So this

> makes avg_thermal.load_avg a util_avg really. Correct?

Hello Quais,

Sorry for not replying to your earlier email. Thanks for the review.
So if you look at the code, util_sum in calculated as a binary signal
converted into capacity. Check out the the below snippet from accumulate_sum

   if (load)
                sa->load_sum += load * contrib;
        if (runnable)
                sa->runnable_load_sum += runnable * contrib;
        if (running)
                sa->util_sum += contrib << SCHED_CAPACITY_SHIFT;

So the actual delta for the thermal pressure will never be considered
if util_avg is used.

I will update this patch with relevant comment.




-- 
Warm Regards
Thara
Dietmar Eggemann Oct. 31, 2019, 4:17 p.m. | #7
On 31.10.19 16:48, Vincent Guittot wrote:
> On Thu, 31 Oct 2019 at 16:38, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:

>>

>> On 31.10.19 11:53, Qais Yousef wrote:

>>> On 10/28/19 16:30, Peter Zijlstra wrote:

>>>> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

>>>>> On 10/22/19 16:34, Thara Gopinath wrote:


[...]

>>> To make sure I got this correctly - it's because avg_thermal.load_avg

>>> represents delta_capacity which is already a 'converted' form of load. So this

>>> makes avg_thermal.load_avg a util_avg really. Correct?

>>>

>>> If I managed to get it right somehow. It'd be nice if we can do inverse

>>> conversion on delta_capacity so that avg_thermal.{load_avg, util_avg} meaning

>>> is consistent across the board. But I don't feel strongly about it if this gets

>>> documented properly.

>>

>> So why can't we use rq->avg_thermal.util_avg here? Since capacity is

>> closer to util than to load?

>>

>> Is it because you want to use the influence of ___update_load_sum(...,

>> unsigned long load eq. per-cpu delta_capacity in your signal?

>>

>> Why not call it this way then?

> 

> util_avg tracks a binary state with 2 fixed weights: running(1024)  vs

> not running (0)

> In the case of thermal pressure, we want to track how much pressure is

> put on the CPU: capping to half the max frequency is not the same as

> capping only 10%

> load_avg is not boolean but you set the weight  you want to apply and

> this weight reflects the amount of pressure.


I see. This is what I meant by 'load (weight) eq. per-cpu delta_capacity
(pressure)'.


>> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c

>> index 38210691c615..d3035457483f 100644

>> --- a/kernel/sched/pelt.c

>> +++ b/kernel/sched/pelt.c

>> @@ -357,9 +357,9 @@ int update_thermal_load_avg(u64 now, struct rq *rq,

>> u64 capacity)

>>  {

>>         if (___update_load_sum(now, &rq->avg_thermal,

>>                                capacity,

>> -                              capacity,

>> -                              capacity)) {

>> -               ___update_load_avg(&rq->avg_thermal, 1, 1);

>> +                              0,

>> +                              0)) {

>> +               ___update_load_avg(&rq->avg_thermal, 1, 0);

>>                 return 1;

>>         }


So we could call it this way since we don't care about runnable_load or
util?
Vincent Guittot Oct. 31, 2019, 4:31 p.m. | #8
On Thu, 31 Oct 2019 at 17:17, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>

> On 31.10.19 16:48, Vincent Guittot wrote:

> > On Thu, 31 Oct 2019 at 16:38, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:

> >>

> >> On 31.10.19 11:53, Qais Yousef wrote:

> >>> On 10/28/19 16:30, Peter Zijlstra wrote:

> >>>> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

> >>>>> On 10/22/19 16:34, Thara Gopinath wrote:

>

> [...]

>

> >>> To make sure I got this correctly - it's because avg_thermal.load_avg

> >>> represents delta_capacity which is already a 'converted' form of load. So this

> >>> makes avg_thermal.load_avg a util_avg really. Correct?

> >>>

> >>> If I managed to get it right somehow. It'd be nice if we can do inverse

> >>> conversion on delta_capacity so that avg_thermal.{load_avg, util_avg} meaning

> >>> is consistent across the board. But I don't feel strongly about it if this gets

> >>> documented properly.

> >>

> >> So why can't we use rq->avg_thermal.util_avg here? Since capacity is

> >> closer to util than to load?

> >>

> >> Is it because you want to use the influence of ___update_load_sum(...,

> >> unsigned long load eq. per-cpu delta_capacity in your signal?

> >>

> >> Why not call it this way then?

> >

> > util_avg tracks a binary state with 2 fixed weights: running(1024)  vs

> > not running (0)

> > In the case of thermal pressure, we want to track how much pressure is

> > put on the CPU: capping to half the max frequency is not the same as

> > capping only 10%

> > load_avg is not boolean but you set the weight  you want to apply and

> > this weight reflects the amount of pressure.

>

> I see. This is what I meant by 'load (weight) eq. per-cpu delta_capacity

> (pressure)'.

>

>

> >> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c

> >> index 38210691c615..d3035457483f 100644

> >> --- a/kernel/sched/pelt.c

> >> +++ b/kernel/sched/pelt.c

> >> @@ -357,9 +357,9 @@ int update_thermal_load_avg(u64 now, struct rq *rq,

> >> u64 capacity)

> >>  {

> >>         if (___update_load_sum(now, &rq->avg_thermal,

> >>                                capacity,

> >> -                              capacity,

> >> -                              capacity)) {

> >> -               ___update_load_avg(&rq->avg_thermal, 1, 1);

> >> +                              0,

> >> +                              0)) {

> >> +               ___update_load_avg(&rq->avg_thermal, 1, 0);

> >>                 return 1;

> >>         }

>

> So we could call it this way since we don't care about runnable_load or

> util?


one way or the other is quite similar but the current solution is
aligned with other irq, rt, dl signals which duplicates the same state
in each fields
Dietmar Eggemann Oct. 31, 2019, 4:44 p.m. | #9
On 31.10.19 17:31, Vincent Guittot wrote:
> On Thu, 31 Oct 2019 at 17:17, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:

>>

>> On 31.10.19 16:48, Vincent Guittot wrote:

>>> On Thu, 31 Oct 2019 at 16:38, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:

>>>>

>>>> On 31.10.19 11:53, Qais Yousef wrote:

>>>>> On 10/28/19 16:30, Peter Zijlstra wrote:

>>>>>> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

>>>>>>> On 10/22/19 16:34, Thara Gopinath wrote:


[...]

>>>> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c

>>>> index 38210691c615..d3035457483f 100644

>>>> --- a/kernel/sched/pelt.c

>>>> +++ b/kernel/sched/pelt.c

>>>> @@ -357,9 +357,9 @@ int update_thermal_load_avg(u64 now, struct rq *rq,

>>>> u64 capacity)

>>>>  {

>>>>         if (___update_load_sum(now, &rq->avg_thermal,

>>>>                                capacity,

>>>> -                              capacity,

>>>> -                              capacity)) {

>>>> -               ___update_load_avg(&rq->avg_thermal, 1, 1);

>>>> +                              0,

>>>> +                              0)) {

>>>> +               ___update_load_avg(&rq->avg_thermal, 1, 0);

>>>>                 return 1;

>>>>         }

>>

>> So we could call it this way since we don't care about runnable_load or

>> util?

> 

> one way or the other is quite similar but the current solution is

> aligned with other irq, rt, dl signals which duplicates the same state

> in each fields


I see. But there is a subtle difference. For irq, rt, dl, we have to
also set load (even we only use util) because of:

___update_load_sum() {

        ...
        if (!load)
                runnable = running = 0;
        ...
}

which is there for se's only.

I like self-explanatory code but I agree in this case it's not obvious.
Qais Yousef Oct. 31, 2019, 4:56 p.m. | #10
On 10/31/19 12:03, Thara Gopinath wrote:
> On 10/31/2019 06:53 AM, Qais Yousef wrote:

> > On 10/28/19 16:30, Peter Zijlstra wrote:

> >> On Wed, Oct 23, 2019 at 01:28:40PM +0100, Qais Yousef wrote:

> >>> On 10/22/19 16:34, Thara Gopinath wrote:

> >>>> cpu_capacity relflects the maximum available capacity of a cpu. Thermal

> >>>> pressure on a cpu means this maximum available capacity is reduced. This

> >>>> patch reduces the average thermal pressure for a cpu from its maximum

> >>>> available capacity so that cpu_capacity reflects the actual

> >>>> available capacity.

> >>>>

> >>>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> >>>> ---

> >>>>  kernel/sched/fair.c | 1 +

> >>>>  1 file changed, 1 insertion(+)

> >>>>

> >>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c

> >>>> index 4f9c2cb..be3e802 100644

> >>>> --- a/kernel/sched/fair.c

> >>>> +++ b/kernel/sched/fair.c

> >>>> @@ -7727,6 +7727,7 @@ static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)

> >>>>  

> >>>>  	used = READ_ONCE(rq->avg_rt.util_avg);

> >>>>  	used += READ_ONCE(rq->avg_dl.util_avg);

> >>>> +	used += READ_ONCE(rq->avg_thermal.load_avg);

> >>>

> >>> Maybe a naive question - but can we add util_avg with load_avg without

> >>> a conversion? I thought the 2 signals have different properties.

> >>

> >> Changelog of patch #1 explains, it's in that dense blob of text.

> >>

> >> But yes, you're quite right that that wants a comment here.

> > 

> > Thanks for the pointer! A comment would be nice indeed.

> > 

> > To make sure I got this correctly - it's because avg_thermal.load_avg

> > represents delta_capacity which is already a 'converted' form of load. So this

> > makes avg_thermal.load_avg a util_avg really. Correct?

> Hello Quais,

> 

> Sorry for not replying to your earlier email. Thanks for the review.

> So if you look at the code, util_sum in calculated as a binary signal

> converted into capacity. Check out the the below snippet from accumulate_sum

> 

>    if (load)

>                 sa->load_sum += load * contrib;

>         if (runnable)

>                 sa->runnable_load_sum += runnable * contrib;

>         if (running)

>                 sa->util_sum += contrib << SCHED_CAPACITY_SHIFT;

> 

> So the actual delta for the thermal pressure will never be considered

> if util_avg is used.

> 

> I will update this patch with relevant comment.


Okay thanks for the explanation.

--
Qais Yousef

Patch

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4f9c2cb..be3e802 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7727,6 +7727,7 @@  static unsigned long scale_rt_capacity(struct sched_domain *sd, int cpu)
 
 	used = READ_ONCE(rq->avg_rt.util_avg);
 	used += READ_ONCE(rq->avg_dl.util_avg);
+	used += READ_ONCE(rq->avg_thermal.load_avg);
 
 	if (unlikely(used >= max))
 		return 1;