Re: [PATCH 14/20 v3] sched/eevdf: Push short slice task that are not picked

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] sched/fair: fair_add_pushable_short() pushes running or blocked
  proxy tasks
- [Medium] sched/fair: set_next_task_fair() skips
  fair_queue_push_tasks() on SNT_REPICK
--

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

sched/eevdf: Push short slice task that are not picked

This commit adds waking short-slice tasks to the cfs_rq pushable list in
wakeup_preempt_fair() so that they can be pushed to another CPU if they are
not picked to run next.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3923587b66e0e..894bee51b99ad 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10273,6 +10273,12 @@ static void fair_add_pushable_prev(struct rq *rq, struct task_struct *prev,
>  		__fair_add_pushable_task(rq, prev);
>  }
>  
> +static void fair_add_pushable_short(struct rq *rq, struct task_struct *p)
> +{
> +	if (sched_push_task_enabled() && __check_pushable_fair_task(rq, p))
> +		__fair_add_pushable_task(rq, p);
> +}

[Severity: High]
With CONFIG_SCHED_PROXY_EXEC enabled, can fair_add_pushable_short() leave
an actively running proxy task or a mutex-blocked donor on
rq->cfs.pushable_tasks?

__check_pushable_fair_task() does not check !task_current(rq, p) or
!task_is_blocked(p).  If a short-slice mutex owner p wakes up and is added
to rq->cfs.pushable_tasks, and __schedule() picks a blocked donor D while
find_proxy_task() selects p as the execution context (rq->curr = p),
set_next_task_fair() only calls fair_remove_pushable_task(rq, D).  Likewise,
prio_changed_fair() and switched_to_fair() call wakeup_preempt_fair() when
p == rq->curr != rq->donor, adding rq->curr directly.

When fair_push_task() then calls pick_next_pushable_fair_task(), it hits
WARN_ON_ONCE(task_current(rq, p)) and still returns p, deactivating and
migrating rq->curr to another CPU while it is actively executing on
rq->cpu.  Similarly, when proxy_migrate_task() attaches a blocked donor via
wakeup_preempt(), fair_add_pushable_short() queues it to be pushed away
from its mutex owner's CPU and overwrites p->wake_cpu via set_task_cpu().

>  static int active_load_balance_cpu_stop(void *data);
>  
>  /*
> @@ -10723,8 +10729,10 @@ static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_f
>  preempt:
>  	cancel_protect_slice(se);
>  
> -	if (preempt_action == PREEMPT_WAKEUP_SHORT)
> +	if (preempt_action == PREEMPT_WAKEUP_SHORT) {
>  		set_short_buddy(cfs_rq, pse);
> +		fair_add_pushable_short(rq, p);
> +	}
>  
>  	resched_curr_lazy(rq);
>  }

[Severity: Medium]
Does set_next_task_fair() skip queuing the push callback when __schedule()
repicks the current donor (next == prev)?

Before this change, tasks were only added to rq->cfs.pushable_tasks in
put_prev_task_fair(), which only runs when switching to a different task
(SNT_PICK).  When fair_add_pushable_short() adds p at wakeup and the
subsequent __schedule() repicks the current task (for example, if lazy
preemption delays __schedule() until pse is no longer eligible, or an
existing shorter-slice buddy in cfs_rq->next blocks before __schedule() and
curr has an earlier deadline), put_prev_set_next_task() calls
set_next_task_fair() with SNT_REPICK:

kernel/sched/fair.c:set_next_task_fair() {
	...
	if (type == SNT_REPICK)
		goto repick;
	...
	fair_queue_push_tasks(rq);
repick:
	if (hrtick_enabled_fair(rq))
		hrtick_start_fair(rq, p);
}

Because SNT_REPICK jumps past fair_queue_push_tasks(rq), the unpicked
short-slice task remains on rq->cfs.pushable_tasks without queuing
fair_push_tasks().

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