* [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions
@ 2023-12-20 6:55 Shrikanth Hegde
2023-12-20 10:11 ` [tip: sched/core] sched/fair: Use existing helper functions to access ->avg_rt and ->avg_dl tip-bot2 for Shrikanth Hegde
2023-12-20 13:59 ` [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Vincent Guittot
0 siblings, 2 replies; 7+ messages in thread
From: Shrikanth Hegde @ 2023-12-20 6:55 UTC (permalink / raw)
To: mingo, peterz, vincent.guittot
Cc: sshegde, dietmar.eggemann, linux-kernel, srikar, yu.c.chen,
tim.c.chen
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 <sshegde@linux.vnet.ibm.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [tip: sched/core] sched/fair: Use existing helper functions to access ->avg_rt and ->avg_dl
2023-12-20 6:55 [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Shrikanth Hegde
@ 2023-12-20 10:11 ` tip-bot2 for Shrikanth Hegde
2023-12-20 13:59 ` [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Vincent Guittot
1 sibling, 0 replies; 7+ messages in thread
From: tip-bot2 for Shrikanth Hegde @ 2023-12-20 10:11 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Shrikanth Hegde, Ingo Molnar, x86, linux-kernel
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 <sshegde@linux.vnet.ibm.com>
AuthorDate: Wed, 20 Dec 2023 12:25:22 +05:30
Committer: Ingo Molnar <mingo@kernel.org>
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 <sshegde@linux.vnet.ibm.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20231220065522.351915-1-sshegde@linux.vnet.ibm.com
---
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))
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions
2023-12-20 6:55 [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Shrikanth Hegde
2023-12-20 10:11 ` [tip: sched/core] sched/fair: Use existing helper functions to access ->avg_rt and ->avg_dl tip-bot2 for Shrikanth Hegde
@ 2023-12-20 13:59 ` Vincent Guittot
2023-12-20 14:48 ` Shrikanth Hegde
2023-12-20 19:53 ` Ingo Molnar
1 sibling, 2 replies; 7+ messages in thread
From: Vincent Guittot @ 2023-12-20 13:59 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: mingo, peterz, dietmar.eggemann, linux-kernel, srikar, yu.c.chen,
tim.c.chen
On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
<sshegde@linux.vnet.ibm.com> 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 <sshegde@linux.vnet.ibm.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions
2023-12-20 13:59 ` [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Vincent Guittot
@ 2023-12-20 14:48 ` Shrikanth Hegde
2023-12-21 16:16 ` Vincent Guittot
2023-12-20 19:53 ` Ingo Molnar
1 sibling, 1 reply; 7+ messages in thread
From: Shrikanth Hegde @ 2023-12-20 14:48 UTC (permalink / raw)
To: Vincent Guittot
Cc: mingo, peterz, dietmar.eggemann, linux-kernel, srikar, yu.c.chen,
tim.c.chen
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
> <sshegde@linux.vnet.ibm.com> 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 <sshegde@linux.vnet.ibm.com>
>> ---
>> 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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions
2023-12-20 14:48 ` Shrikanth Hegde
@ 2023-12-21 16:16 ` Vincent Guittot
2023-12-22 8:02 ` Shrikanth Hegde
0 siblings, 1 reply; 7+ messages in thread
From: Vincent Guittot @ 2023-12-21 16:16 UTC (permalink / raw)
To: Shrikanth Hegde
Cc: mingo, peterz, dietmar.eggemann, linux-kernel, srikar, yu.c.chen,
tim.c.chen
Hi Shrikanth,
On Wed, 20 Dec 2023 at 15:49, Shrikanth Hegde
<sshegde@linux.vnet.ibm.com> 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
> > <sshegde@linux.vnet.ibm.com> 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
> >>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions
2023-12-21 16:16 ` Vincent Guittot
@ 2023-12-22 8:02 ` Shrikanth Hegde
0 siblings, 0 replies; 7+ messages in thread
From: Shrikanth Hegde @ 2023-12-22 8:02 UTC (permalink / raw)
To: Vincent Guittot, Ingo Molnar
Cc: peterz, dietmar.eggemann, linux-kernel, srikar, yu.c.chen,
tim.c.chen
On 12/21/23 9:46 PM, Vincent Guittot wrote:
> Hi Shrikanth,
>
> On Wed, 20 Dec 2023 at 15:49, Shrikanth Hegde
> <sshegde@linux.vnet.ibm.com> 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
>>> <sshegde@linux.vnet.ibm.com> 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
>>>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions
2023-12-20 13:59 ` [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Vincent Guittot
2023-12-20 14:48 ` Shrikanth Hegde
@ 2023-12-20 19:53 ` Ingo Molnar
1 sibling, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2023-12-20 19:53 UTC (permalink / raw)
To: Vincent Guittot
Cc: Shrikanth Hegde, peterz, dietmar.eggemann, linux-kernel, srikar,
yu.c.chen, tim.c.chen
* Vincent Guittot <vincent.guittot@linaro.org> wrote:
> On Wed, 20 Dec 2023 at 07:55, Shrikanth Hegde
> <sshegde@linux.vnet.ibm.com> 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 <sshegde@linux.vnet.ibm.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-22 8:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-20 6:55 [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Shrikanth Hegde
2023-12-20 10:11 ` [tip: sched/core] sched/fair: Use existing helper functions to access ->avg_rt and ->avg_dl tip-bot2 for Shrikanth Hegde
2023-12-20 13:59 ` [PATCH] sched: move access of avg_rt and avg_dl into existing helper functions Vincent Guittot
2023-12-20 14:48 ` Shrikanth Hegde
2023-12-21 16:16 ` Vincent Guittot
2023-12-22 8:02 ` Shrikanth Hegde
2023-12-20 19:53 ` Ingo Molnar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.