This is a minor code simplification. There are helper functions called
cpu_util_dl and cpu_util_rt which gives the average utilization of DL
and RT respectively. But there are few places in code where these
variables are used directly.
Instead use the helper function so that code becomes simpler and easy to
maintain later on.
Signed-off-by: Shrikanth Hegde <[email protected]>
---
kernel/sched/fair.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bcea3d55d95d..02631060ca7e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9212,19 +9212,17 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq *cfs_rq)
static inline bool others_have_blocked(struct rq *rq)
{
- if (READ_ONCE(rq->avg_rt.util_avg))
+ if (cpu_util_rt(rq))
return true;
- if (READ_ONCE(rq->avg_dl.util_avg))
+ if (cpu_util_dl(rq))
return true;
if (thermal_load_avg(rq))
return true;
-#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
- if (READ_ONCE(rq->avg_irq.util_avg))
+ if (cpu_util_irq(rq))
return true;
-#endif
return false;
}
@@ -9481,8 +9479,8 @@ static unsigned long scale_rt_capacity(int cpu)
* avg_thermal.load_avg tracks thermal pressure and the weighted
* average uses the actual delta max capacity(load).
*/
- used = READ_ONCE(rq->avg_rt.util_avg);
- used += READ_ONCE(rq->avg_dl.util_avg);
+ used = cpu_util_rt(rq);
+ used += cpu_util_dl(rq);
used += thermal_load_avg(rq);
if (unlikely(used >= max))
--
2.39.3
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 6b5943af9198d21b4fdb1ce14ee11d11119cc709
Gitweb: https://git.kernel.org/tip/6b5943af9198d21b4fdb1ce14ee11d11119cc709
Author: Shrikanth Hegde <[email protected]>
AuthorDate: Wed, 20 Dec 2023 12:25:22 +05:30
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 20 Dec 2023 11:04:05 +01:00
sched/fair: Use existing helper functions to access ->avg_rt and ->avg_dl
This is a minor code simplification. There are helper functions called
cpu_util_dl() and cpu_util_rt() which gives the average utilization of DL
and RT respectively. But there are few places in code where these
variables are open-coded.
Instead use the helper function so that code becomes simpler and easier to
maintain later on.
No change in functionality intended.
Signed-off-by: Shrikanth Hegde <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bcea3d5..0263106 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9212,19 +9212,17 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq *cfs_rq)
static inline bool others_have_blocked(struct rq *rq)
{
- if (READ_ONCE(rq->avg_rt.util_avg))
+ if (cpu_util_rt(rq))
return true;
- if (READ_ONCE(rq->avg_dl.util_avg))
+ if (cpu_util_dl(rq))
return true;
if (thermal_load_avg(rq))
return true;
-#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
- if (READ_ONCE(rq->avg_irq.util_avg))
+ if (cpu_util_irq(rq))
return true;
-#endif
return false;
}
@@ -9481,8 +9479,8 @@ static unsigned long scale_rt_capacity(int cpu)
* avg_thermal.load_avg tracks thermal pressure and the weighted
* average uses the actual delta max capacity(load).
*/
- used = READ_ONCE(rq->avg_rt.util_avg);
- used += READ_ONCE(rq->avg_dl.util_avg);
+ used = cpu_util_rt(rq);
+ used += cpu_util_dl(rq);
used += thermal_load_avg(rq);
if (unlikely(used >= max))
On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
<[email protected]> wrote:
>
> This is a minor code simplification. There are helper functions called
> cpu_util_dl and cpu_util_rt which gives the average utilization of DL
> and RT respectively. But there are few places in code where these
> variables are used directly.
>
> Instead use the helper function so that code becomes simpler and easy to
> maintain later on.
>
> Signed-off-by: Shrikanth Hegde <[email protected]>
> ---
> kernel/sched/fair.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bcea3d55d95d..02631060ca7e 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9212,19 +9212,17 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq *cfs_rq)
>
> static inline bool others_have_blocked(struct rq *rq)
> {
> - if (READ_ONCE(rq->avg_rt.util_avg))
> + if (cpu_util_rt(rq))
> return true;
>
> - if (READ_ONCE(rq->avg_dl.util_avg))
> + if (cpu_util_dl(rq))
> return true;
>
> if (thermal_load_avg(rq))
> return true;
>
> -#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
> - if (READ_ONCE(rq->avg_irq.util_avg))
> + if (cpu_util_irq(rq))
cpu_util_irq doesn't call READ_ONCE()
> return true;
> -#endif
>
> return false;
> }
> @@ -9481,8 +9479,8 @@ static unsigned long scale_rt_capacity(int cpu)
> * avg_thermal.load_avg tracks thermal pressure and the weighted
> * average uses the actual delta max capacity(load).
> */
> - used = READ_ONCE(rq->avg_rt.util_avg);
> - used += READ_ONCE(rq->avg_dl.util_avg);
> + used = cpu_util_rt(rq);
> + used += cpu_util_dl(rq);
> used += thermal_load_avg(rq);
>
> if (unlikely(used >= max))
> --
> 2.39.3
>
On 12/20/23 7:29 PM, Vincent Guittot wrote:
Hi Vincent, thanks for taking a look.
> On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
> <[email protected]> wrote:
>>
>> This is a minor code simplification. There are helper functions called
>> cpu_util_dl and cpu_util_rt which gives the average utilization of DL
>> and RT respectively. But there are few places in code where these
>> variables are used directly.
>>
>> Instead use the helper function so that code becomes simpler and easy to
>> maintain later on.
>>
>> Signed-off-by: Shrikanth Hegde <[email protected]>
>> ---
>> kernel/sched/fair.c | 12 +++++-------
>> 1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index bcea3d55d95d..02631060ca7e 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -9212,19 +9212,17 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq *cfs_rq)
>>
>> static inline bool others_have_blocked(struct rq *rq)
>> {
>> - if (READ_ONCE(rq->avg_rt.util_avg))
>> + if (cpu_util_rt(rq))
>> return true;
>>
>> - if (READ_ONCE(rq->avg_dl.util_avg))
>> + if (cpu_util_dl(rq))
>> return true;
>>
>> if (thermal_load_avg(rq))
>> return true;
>>
>> -#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
>> - if (READ_ONCE(rq->avg_irq.util_avg))
>> + if (cpu_util_irq(rq))
>
> cpu_util_irq doesn't call READ_ONCE()
>
I see. Actually it would be right if cpu_util_irq does call READ_ONCE no?
Sorry i havent yet understood the memory barriers in details. Please correct me
if i am wrong here,
since ___update_load_avg(&rq->avg_irq, 1) does use WRITE_ONCE and reading out this
value using cpu_util_irq on a different CPU should use READ_ONCE no?
>
>> return true;
>> -#endif
>>
>> return false;
>> }
>> @@ -9481,8 +9479,8 @@ static unsigned long scale_rt_capacity(int cpu)
>> * avg_thermal.load_avg tracks thermal pressure and the weighted
>> * average uses the actual delta max capacity(load).
>> */
>> - used = READ_ONCE(rq->avg_rt.util_avg);
>> - used += READ_ONCE(rq->avg_dl.util_avg);
>> + used = cpu_util_rt(rq);
>> + used += cpu_util_dl(rq);
>> used += thermal_load_avg(rq);
>>
>> if (unlikely(used >= max))
>> --
>> 2.39.3
>>
* Vincent Guittot <[email protected]> wrote:
> On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
> <[email protected]> wrote:
> >
> > This is a minor code simplification. There are helper functions called
> > cpu_util_dl and cpu_util_rt which gives the average utilization of DL
> > and RT respectively. But there are few places in code where these
> > variables are used directly.
> >
> > Instead use the helper function so that code becomes simpler and easy to
> > maintain later on.
> >
> > Signed-off-by: Shrikanth Hegde <[email protected]>
> > ---
> > kernel/sched/fair.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index bcea3d55d95d..02631060ca7e 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -9212,19 +9212,17 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq *cfs_rq)
> >
> > static inline bool others_have_blocked(struct rq *rq)
> > {
> > - if (READ_ONCE(rq->avg_rt.util_avg))
> > + if (cpu_util_rt(rq))
> > return true;
> >
> > - if (READ_ONCE(rq->avg_dl.util_avg))
> > + if (cpu_util_dl(rq))
> > return true;
> >
> > if (thermal_load_avg(rq))
> > return true;
> >
> > -#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
> > - if (READ_ONCE(rq->avg_irq.util_avg))
> > + if (cpu_util_irq(rq))
>
> cpu_util_irq doesn't call READ_ONCE()
Oh, that's nasty - according to the title only avg_rt and avg_dl were
changed, which I double checked, but the patch indeed does more ...
I've removed this patch from tip:sched/core.
Thanks,
Ingo
Hi Shrikanth,
On Wed, 20 Dec 2023 at 15:49, Shrikanth Hegde
<[email protected]> wrote:
>
>
>
> On 12/20/23 7:29 PM, Vincent Guittot wrote:
>
> Hi Vincent, thanks for taking a look.
>
> > On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
> > <[email protected]> wrote:
> >>
[...]
> >> -#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
> >> - if (READ_ONCE(rq->avg_irq.util_avg))
> >> + if (cpu_util_irq(rq))
> >
> > cpu_util_irq doesn't call READ_ONCE()
> >
>
>
> I see. Actually it would be right if cpu_util_irq does call READ_ONCE no?
Yes, cpu_util_irq should call READ_ONCE()
>
> Sorry i havent yet understood the memory barriers in details. Please correct me
> if i am wrong here,
> since ___update_load_avg(&rq->avg_irq, 1) does use WRITE_ONCE and reading out this
> value using cpu_util_irq on a different CPU should use READ_ONCE no?
Yes
>
> >
> >> return true;
> >> -#endif
> >>
> >> return false;
> >> }
> >> @@ -9481,8 +9479,8 @@ static unsigned long scale_rt_capacity(int cpu)
> >> * avg_thermal.load_avg tracks thermal pressure and the weighted
> >> * average uses the actual delta max capacity(load).
> >> */
> >> - used = READ_ONCE(rq->avg_rt.util_avg);
> >> - used += READ_ONCE(rq->avg_dl.util_avg);
> >> + used = cpu_util_rt(rq);
> >> + used += cpu_util_dl(rq);
> >> used += thermal_load_avg(rq);
> >>
> >> if (unlikely(used >= max))
> >> --
> >> 2.39.3
> >>
On 12/21/23 9:46 PM, Vincent Guittot wrote:
> Hi Shrikanth,
>
> On Wed, 20 Dec 2023 at 15:49, Shrikanth Hegde
> <[email protected]> wrote:
>>
>>
>>
>> On 12/20/23 7:29 PM, Vincent Guittot wrote:
>>
>> Hi Vincent, thanks for taking a look.
>>
>>> On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
>>> <[email protected]> wrote:
>>>>
>
> [...]
>
>>>> -#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
>>>> - if (READ_ONCE(rq->avg_irq.util_avg))
>>>> + if (cpu_util_irq(rq))
>>>
>>> cpu_util_irq doesn't call READ_ONCE()
>>>
>>
>>
>> I see. Actually it would be right if cpu_util_irq does call READ_ONCE no?
>
> Yes, cpu_util_irq should call READ_ONCE()
>
Ok.
Sorry I forgot to mention about avg_irq.
I will send out v2 with the above change of READ_ONCE added soon.
>>
>> Sorry i havent yet understood the memory barriers in details. Please correct me
>> if i am wrong here,
>> since ___update_load_avg(&rq->avg_irq, 1) does use WRITE_ONCE and reading out this
>> value using cpu_util_irq on a different CPU should use READ_ONCE no?
>
> Yes
>
>>
>>>
>>>> return true;
>>>> -#endif
>>>>
>>>> return false;
>>>> }
>>>> @@ -9481,8 +9479,8 @@ static unsigned long scale_rt_capacity(int cpu)
>>>> * avg_thermal.load_avg tracks thermal pressure and the weighted
>>>> * average uses the actual delta max capacity(load).
>>>> */
>>>> - used = READ_ONCE(rq->avg_rt.util_avg);
>>>> - used += READ_ONCE(rq->avg_dl.util_avg);
>>>> + used = cpu_util_rt(rq);
>>>> + used += cpu_util_dl(rq);
>>>> used += thermal_load_avg(rq);
>>>>
>>>> if (unlikely(used >= max))
>>>> --
>>>> 2.39.3
>>>>