Re: [PATCH 18/18 v2] sched/fair: Take into account slice in EAS

[Tim Chen <tim.c.chen@linux.intel.com>]

On Fri, 2026-10-02 at 17:44 +0200, Vincent Guittot wrote:
> When the cost is the same, take into account the slice of a task to try to
> select a CPU where is will run first.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>  kernel/sched/fair.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 94554f165f42..d60bb6db4ce9 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9598,8 +9598,17 @@ static int check_cpu_with_task(struct task_struct *p, int cpu)
>   */
>  static bool update_best_cpu(struct energy_cpu_stat *target,
>  			    struct energy_cpu_stat *min,
> -			    int prev, struct sched_domain *sd)
> +			    int prev, struct sched_domain *sd,
> +			    struct task_struct *p)
>  {
> +	unsigned long task_slice = p->se.slice;
> +
> +	/*  Select the one where you can run first */
> +	if (task_slice < get_rq_min_slice(cpu_rq(target->cpu)) &&
> +	    task_slice >= get_rq_min_slice(cpu_rq(min->cpu)))
> +		return true;
> +
> +	/* Favor previous CPU */
>  	if (target->cpu == prev)
>  		return true;

Hi Vincent,

The slice check above comes before "Favor previous CPU", so I read the intent
as: the CPU where the task can run first wins, even
against prev_cpu. But the slice check can fail for prev CPU
and we can skip to "Favor previous CPU" and pick prev CPU, even
though the task can run on min CPU first but not
necessarily on prev CPU first.

Take an idle CPU X and a busy prev_cpu. Any task can
run first on X, because an empty rq has min_slice == ULONG_MAX:

- prev_cpu scanned first: when X is the target, the slice check
  selects X.
- X scanned first: when prev_cpu is the target, the slice check fails,
  and "Favor previous CPU" then selects prev_cpu.

In the second case the task is stacked on the busy prev_cpu even though
it may not run first there.

Perhaps something like the following is better.

---
 kernel/sched/fair.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index a47c6521a411..3fe305d690eb 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9830,11 +9830,15 @@ static bool update_best_cpu(struct energy_cpu_stat *target,
                            struct task_struct *p)
 {
        unsigned long task_slice = p->se.slice;
+       bool target_first = task_slice < get_rq_min_slice(cpu_rq(target->cpu));
+       bool min_first = task_slice < get_rq_min_slice(cpu_rq(min->cpu));

-       /*  Select the one where you can run first */
-       if (task_slice < get_rq_min_slice(cpu_rq(target->cpu)) &&
-           task_slice >= get_rq_min_slice(cpu_rq(min->cpu)))
-               return true;
+       /*
+        * Select the one where you can run first. Check both ways, or the
+        * result depends on the order of the CPUs in the PD.
+        */
+       if (target_first != min_first)
+               return target_first;

        /* Favor previous CPU */
        if (target->cpu == prev)


Tim

>  	if (min->cpu == prev)
> @@ -9762,7 +9771,7 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>  			 */
>  			if (target_perf < min_stat.min_perf)
>  				find_pd_cost(pd->em_pd, target_perf, &target_stat);
> -			else if (!update_best_cpu(&target_stat, &min_stat, prev_cpu, sd))
> +			else if (!update_best_cpu(&target_stat, &min_stat, prev_cpu, sd, p))
>  				continue;
>  
>  			/* Save the new most efficient CPU of the PD */