diff mbox series

[v3,2/3] sched/uclamp: Ignore (util == 0) optimization in feec() when p_util_max = 0

Message ID 20230717215717.309174-3-qyousef@layalina.io
State New
Headers
Series Fix a couple of corner cases in feec() when using uclamp_max |

Commit Message

Qais Yousef July 17, 2023, 9:57 p.m. UTC 
  find_energy_efficient_cpu() bails out early if effective util of the
task is 0 as the delta at this point will be zero and there's nothing
for EAS to do. When uclamp is being used, this could lead to wrong
decisions when uclamp_max is set to 0. In this case the task is capped
to performance point 0, but it is actually running and consuming energy
and we can benefit from EAS energy calculations.

Rework the condition so that it bails out for when util is actually 0 or
uclamp_min is requesting a higher performance point.

We can do that without needing to use uclamp_task_util(); remove it.

Fixes: d81304bc6193 ("sched/uclamp: Cater for uclamp in find_energy_efficient_cpu()'s early exit condition")
Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>
---
 kernel/sched/fair.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

Comments

Vincent Guittot July 21, 2023, 9:57 a.m. UTC | #1 
On Mon, 17 Jul 2023 at 23:57, Qais Yousef <qyousef@layalina.io> wrote:
>
> find_energy_efficient_cpu() bails out early if effective util of the
> task is 0 as the delta at this point will be zero and there's nothing
> for EAS to do. When uclamp is being used, this could lead to wrong
> decisions when uclamp_max is set to 0. In this case the task is capped
> to performance point 0, but it is actually running and consuming energy
> and we can benefit from EAS energy calculations.
>
> Rework the condition so that it bails out for when util is actually 0 or
> uclamp_min is requesting a higher performance point.
>
> We can do that without needing to use uclamp_task_util(); remove it.
>
> Fixes: d81304bc6193 ("sched/uclamp: Cater for uclamp in find_energy_efficient_cpu()'s early exit condition")
> Signed-off-by: Qais Yousef (Google) <qyousef@layalina.io>

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

> ---
>  kernel/sched/fair.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d489eece5a0d..c701f490ca4c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4348,22 +4348,6 @@ static inline unsigned long task_util_est(struct task_struct *p)
>         return max(task_util(p), _task_util_est(p));
>  }
>
> -#ifdef CONFIG_UCLAMP_TASK
> -static inline unsigned long uclamp_task_util(struct task_struct *p,
> -                                            unsigned long uclamp_min,
> -                                            unsigned long uclamp_max)
> -{
> -       return clamp(task_util_est(p), uclamp_min, uclamp_max);
> -}
> -#else
> -static inline unsigned long uclamp_task_util(struct task_struct *p,
> -                                            unsigned long uclamp_min,
> -                                            unsigned long uclamp_max)
> -{
> -       return task_util_est(p);
> -}
> -#endif
> -
>  static inline void util_est_enqueue(struct cfs_rq *cfs_rq,
>                                     struct task_struct *p)
>  {
> @@ -7588,7 +7572,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>         target = prev_cpu;
>
>         sync_entity_load_avg(&p->se);
> -       if (!uclamp_task_util(p, p_util_min, p_util_max))
> +       if (!task_util_est(p) && p_util_min == 0)
>                 goto unlock;
>
>         eenv_task_busy_time(&eenv, p, prev_cpu);
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d489eece5a0d..c701f490ca4c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4348,22 +4348,6 @@  static inline unsigned long task_util_est(struct task_struct *p)
 	return max(task_util(p), _task_util_est(p));
 }
 
-#ifdef CONFIG_UCLAMP_TASK
-static inline unsigned long uclamp_task_util(struct task_struct *p,
-					     unsigned long uclamp_min,
-					     unsigned long uclamp_max)
-{
-	return clamp(task_util_est(p), uclamp_min, uclamp_max);
-}
-#else
-static inline unsigned long uclamp_task_util(struct task_struct *p,
-					     unsigned long uclamp_min,
-					     unsigned long uclamp_max)
-{
-	return task_util_est(p);
-}
-#endif
-
 static inline void util_est_enqueue(struct cfs_rq *cfs_rq,
 				    struct task_struct *p)
 {
@@ -7588,7 +7572,7 @@  static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
 	target = prev_cpu;
 
 	sync_entity_load_avg(&p->se);
-	if (!uclamp_task_util(p, p_util_min, p_util_max))
+	if (!task_util_est(p) && p_util_min == 0)
 		goto unlock;
 
 	eenv_task_busy_time(&eenv, p, prev_cpu);