Re: [PATCH 19/20 v3] sched/fair: Take into account slice in EAS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] sched/fair: slice check on idle CPUs in update_best_cpu()
  prefers SCHED_IDLE
--

commit 94c14b8b01a87751cdea1dbf5bbcde621aa150bb
Author: Vincent Guittot <vincent.guittot@linaro.org>

sched/fair: Take into account slice in EAS

This commit updates update_best_cpu() in Energy Aware Scheduling to compare
the waking task's slice against each candidate runqueue's min_slice when
energy costs are equal.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index ea117c3d0dd71..da9c4416f1b5c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9885,13 +9885,22 @@ static bool update_best_cpu(struct energy_cpu_stat *target,
>  			    int prev, struct sched_domain *sd,
>  			    struct task_struct *p)
>  {
> +	unsigned long task_slice = p->se.slice;
>  	bool target_idle = choose_idle_cpu(target->cpu, p);
>  	bool min_idle = choose_idle_cpu(min->cpu, p);
> +	bool target_first, min_first;
>  
>  	/*  Select the one which is idle */
>  	if (target_idle != min_idle)
>  		return target_idle;
>  
> +	target_first = task_slice < get_rq_min_slice(cpu_rq(target->cpu));
> +	min_first = task_slice < get_rq_min_slice(cpu_rq(min->cpu));
> +
> +	/*  Select the one where you can run first */
> +	if (target_first != min_first)
> +		return target_first;

[Severity: Medium]
Can this slice comparison prefer a CPU running SCHED_IDLE tasks over a
completely empty idle CPU when both target_idle and min_idle are true?

choose_idle_cpu() returns true both for an empty CPU (available_idle_cpu())
and for a CPU running only SCHED_IDLE tasks (choose_sched_idle_rq()).  On
an empty runqueue, __update_rq_min_slice() resets rq->min_slice to 0, so
task_slice < get_rq_min_slice() is false.  On a CPU with a queued
SCHED_IDLE task, rq->min_slice holds that task's slice, so task_slice <
get_rq_min_slice() is true whenever p->se.slice is shorter.

When both candidate CPUs pass choose_idle_cpu(), execution falls through
if (target_idle != min_idle) and returns at if (target_first != min_first),
selecting the SCHED_IDLE CPU over a completely idle CPU and bypassing the
prev_cpu and nr_running checks below.  Should the slice check be skipped
when target_idle is true?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106151947.3991524-2-vincent.guittot@linaro.org?part=19