Re: [PATCH 13/20] sched/eevdf: Try to push short slice task on a better CPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] sched/fair: fair_check_pushable_task() pushes !se.on_rq and
  idle-next tasks
- [High] sched/fair: fair_check_pushable_task() pushes proxy-exec owners
  and donors
--

commit 016c5e088e8b372064ad1e24d8191243bb3c5d70
Author: Vincent Guittot <vincent.guittot@linaro.org>

sched/eevdf: Try to push short slice task on a better CPU

This commit extends fair_check_pushable_task() to mark a fair task as
pushable when preempted by another scheduling class or when it is an
eligible short-slice task.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 980eda9c935a8..3923587b66e0e 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10112,6 +10112,23 @@ static bool __check_pushable_fair_task(struct rq *rq, struct task_struct *p)
>  	return true;
>  }
>  
> +static bool check_pushable_short_task(struct rq *rq, struct task_struct *p)
> +{
> +	struct sched_entity *pse = &p->se;
> +	struct cfs_rq *cfs_rq = &rq->cfs;
> +
> +	if (cfs_rq->h_nr_runnable <= 1)
> +		return false;
> +
> +	if (!entity_eligible(cfs_rq, pse))
> +		return false;
> +
> +	if (pse->slice < cfs_rq_max_slice(cfs_rq))
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool fair_check_pushable_task(struct rq *rq, struct task_struct *p, struct task_struct *next)
>  {
>  	if (!__check_pushable_fair_task(rq, p))
> @@ -10120,6 +10137,12 @@ static bool fair_check_pushable_task(struct rq *rq, struct task_struct *p, struc
>  	if (next == p)
>  		return false;
>  
> +	if (next && next->sched_class != p->sched_class)
> +		return true;
> +
> +	if (check_pushable_short_task(rq, p))
> +		return true;
> +
>  	return false;
>  }

[Severity: High]
Can fair_check_pushable_task() queue a task onto rq->cfs.pushable_tasks
when p->se.on_rq is 0 or when next is rq->idle?

In put_prev_task_fair(), fair_add_pushable_prev() runs even when se->on_rq
is 0, and __check_pushable_fair_task() only checks task_on_rq_queued(p) and
p->se.sched_delayed.  When throttle_cfs_rq_work() throttles a running task,
it calls dequeue_task_fair() (clearing p->se.on_rq) while leaving p->on_rq
as TASK_ON_RQ_QUEUED.  Because next->sched_class != p->sched_class also
matches idle_sched_class (instead of checking sched_class_above()),
switching to idle or another task queues the throttled task onto
rq->cfs.pushable_tasks.  fair_push_task() can then migrate it to an
unthrottled remote CPU where enqueue_throttled_task() clears p->throttled
and lets it escape CFS bandwidth enforcement, or dequeue_task_fair() can
take the early task_is_throttled() return and leave a dangling node in
pushable_tasks.

Similarly, under CONFIG_SCHED_PROXY_EXEC, proxy_needs_return() calls
dequeue_block_task() before proxy_reset_donor() -> put_prev_task_fair(),
re-adding p to pushable_tasks after dequeue_task_fair() has already run and
before __block_task() clears p->on_rq.

[Severity: High]
Under CONFIG_SCHED_PROXY_EXEC, can fair_check_pushable_task() cause
fair_push_task() to migrate a currently running mutex owner or a blocked
donor task?

If a running fair task p owns a mutex held across put_prev_set_next_task()
when a blocked waiter is picked as the next donor, put_prev_task_fair()
adds p to rq->cfs.pushable_tasks, and find_proxy_task() then selects p to
keep running as rq->curr without calling set_next_task_fair().  Because
pick_next_pushable_fair_task() only warns on task_current(rq, p) rather
than skipping task_on_cpu(rq, p), fair_push_task() deactivates and migrates
p to another CPU while it is actively executing on the local CPU.

In addition, __check_pushable_fair_task() does not check
task_is_blocked(p), so a mutex-blocked fair donor deselected in
put_prev_task_fair() can be pushed away from its mutex owner's CPU via
set_task_cpu(), clobbering p->wake_cpu.

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