Split the patches into two since there is subtle change with avg_irq.
There is no functionality change expected due to first patch, while
second patch may have. In case of any future regressions it would be
easy to bisect.
This is minor code simplification. Using existing helper function would
be simpler and easier to maintain.
v2->v1:
Vincent pointed out the READ_ONCE change for avg_irq. It would be
correct to use READ_ONCE for accessing avg_irq. Changed the current
helper function to do so.
v1: https://lore.kernel.org/lkml/[email protected]/
Shrikanth Hegde (2):
sched: use existing helper functions to access ->avg_rt and ->avg_dl
sched: add READ_ONCE and use existing helper function to access ->avg_irq
kernel/sched/fair.c | 12 +++++-------
kernel/sched/sched.h | 2 +-
2 files changed, 6 insertions(+), 8 deletions(-)
--
2.39.3
Use existing helper function cpu_util_irq instead of referencing it
directly.
It was noted that avg_irq could be updated by different CPU than the one
which is trying to access it. avg_irq is updated with WRITE_ONCE. Use
READ_ONCE to access it in order to avoid any compiler optimizations.
Signed-off-by: Shrikanth Hegde <[email protected]>
---
kernel/sched/fair.c | 4 +---
kernel/sched/sched.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1aeca3f943a8..02631060ca7e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9221,10 +9221,8 @@ static inline bool others_have_blocked(struct rq *rq)
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;
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index e58a54bda77d..edc20c5cc7ce 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3125,7 +3125,7 @@ static inline bool uclamp_rq_is_idle(struct rq *rq)
#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
static inline unsigned long cpu_util_irq(struct rq *rq)
{
- return rq->avg_irq.util_avg;
+ return READ_ONCE(rq->avg_irq.util_avg);
}
static inline
--
2.39.3
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. This patch doesn't intend any functional changes.
Signed-off-by: Shrikanth Hegde <[email protected]>
---
kernel/sched/fair.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bcea3d55d95d..1aeca3f943a8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9212,10 +9212,10 @@ 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))
@@ -9481,8 +9481,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 Mon, 1 Jan 2024 at 16:49, Shrikanth Hegde <[email protected]> wrote:
>
> 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. This patch doesn't intend any functional changes.
>
> Signed-off-by: Shrikanth Hegde <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
> ---
> kernel/sched/fair.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bcea3d55d95d..1aeca3f943a8 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9212,10 +9212,10 @@ 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))
> @@ -9481,8 +9481,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 Mon, 1 Jan 2024 at 16:47, Shrikanth Hegde <[email protected]> wrote:
>
> Use existing helper function cpu_util_irq instead of referencing it
> directly.
>
> It was noted that avg_irq could be updated by different CPU than the one
> which is trying to access it. avg_irq is updated with WRITE_ONCE. Use
> READ_ONCE to access it in order to avoid any compiler optimizations.
>
> Signed-off-by: Shrikanth Hegde <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
> ---
> kernel/sched/fair.c | 4 +---
> kernel/sched/sched.h | 2 +-
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 1aeca3f943a8..02631060ca7e 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9221,10 +9221,8 @@ static inline bool others_have_blocked(struct rq *rq)
> 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;
> }
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index e58a54bda77d..edc20c5cc7ce 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3125,7 +3125,7 @@ static inline bool uclamp_rq_is_idle(struct rq *rq)
> #ifdef CONFIG_HAVE_SCHED_AVG_IRQ
> static inline unsigned long cpu_util_irq(struct rq *rq)
> {
> - return rq->avg_irq.util_avg;
> + return READ_ONCE(rq->avg_irq.util_avg);
> }
>
> static inline
> --
> 2.39.3
>
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 8b936fc1d84f7d70009ea34d66bbf6c54c09fae7
Gitweb: https://git.kernel.org/tip/8b936fc1d84f7d70009ea34d66bbf6c54c09fae7
Author: Shrikanth Hegde <[email protected]>
AuthorDate: Mon, 01 Jan 2024 21:16:23 +05:30
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:11:14 +01:00
sched/fair: Use existing helper functions to access ->avg_rt and ->avg_dl
There are helper functions called cpu_util_dl() and cpu_util_rt() which give
the average utilization of DL and RT respectively. But there are a few
places in code where access to these variables is open-coded.
Instead use the helper function so that code becomes simpler and easier to
maintain later on.
No functional changes intended.
Signed-off-by: Shrikanth Hegde <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8e30e2b..127e727 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9237,10 +9237,10 @@ 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))
@@ -9506,8 +9506,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))
The following commit has been merged into the sched/core branch of tip:
Commit-ID: a6965b31888501f889261a6783f0de6afff84f8d
Gitweb: https://git.kernel.org/tip/a6965b31888501f889261a6783f0de6afff84f8d
Author: Shrikanth Hegde <[email protected]>
AuthorDate: Mon, 01 Jan 2024 21:16:24 +05:30
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:11:15 +01:00
sched/fair: Add READ_ONCE() and use existing helper function to access ->avg_irq
Use existing helper function cpu_util_irq() instead of open-coding
access to ->avg_irq.
During review it was noted that ->avg_irq could be updated by a
different CPU than the one which is trying to access it.
->avg_irq is updated with WRITE_ONCE(), use READ_ONCE to access it
in order to avoid any compiler optimizations.
Signed-off-by: Shrikanth Hegde <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 4 +---
kernel/sched/sched.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 127e727..ba36339 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9246,10 +9246,8 @@ static inline bool others_have_blocked(struct rq *rq)
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;
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 001fe04..d224267 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3136,7 +3136,7 @@ static inline bool uclamp_rq_is_idle(struct rq *rq)
#ifdef CONFIG_HAVE_SCHED_AVG_IRQ
static inline unsigned long cpu_util_irq(struct rq *rq)
{
- return rq->avg_irq.util_avg;
+ return READ_ONCE(rq->avg_irq.util_avg);
}
static inline