Re: [PATCH 14/18] sched_ext: Split curr|donor references properly

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The physical proxy execution owner is spuriously exposed to the BPF scheduler and loses its personal slice during SAVE/RESTORE cycles.
--

commit 9c0210f5a5bf78476570e1133b4136f7fc67852c
Author: Andrea Righi <arighi@nvidia.com>

sched_ext: Split curr|donor references properly

This commit separates the physical execution context (rq->curr) from the
scheduling context (rq->donor) to support proxy execution in sched_ext.
It updates various scheduler paths to use the donor where appropriate.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index b2e6be4a887de..f23e9f1ee2f3c 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
[ ... ]
> @@ -2170,13 +2177,14 @@ static void enqueue_task_scx(struct rq *rq, struct task_struct *p, int core_enq_
>  		rq->scx.flags |= SCX_RQ_IN_WAKEUP;
>  
>  	/*
> -	 * Restoring a running task will be immediately followed by
> -	 * set_next_task_scx() which expects the task to not be on the BPF
> +	 * Restoring the current scheduling context will be immediately followed
> +	 * by set_next_task_scx() which expects the task to not be on the BPF
>  	 * scheduler as tasks can only start running through local DSQs. Force
>  	 * direct-dispatch into the local DSQ by setting the sticky_cpu. Mark
>  	 * IGNORE_CAPS to force entry into the local DSQ.
>  	 */
> -	if (unlikely(enq_flags & ENQUEUE_RESTORE) && task_current(rq, p)) {
> +	if (unlikely(enq_flags & ENQUEUE_RESTORE) &&
> +	    task_current_donor(rq, p)) {

[Severity: High]
Does this check fail to recognize the physical execution context during
ENQUEUE_RESTORE?

During proxy execution, the currently executing task (rq->curr) and the donor
(rq->donor) are different. If a property change (such as sched_setscheduler or
setpriority) is requested on the physically executing task while it is actively
running, it will undergo a DEQUEUE_SAVE / ENQUEUE_RESTORE cycle.

Because task_current_donor(rq, p) evaluates to false for the physically
executing task, wouldn't this bypass the SCX_ENQ_IGNORE_CAPS fast-path and
erroneously expose the actively executing task to ops.enqueue(), which violates
the constraint that physical execution contexts are not exposed to the BPF
scheduler?

[ ... ]
> @@ -2342,7 +2350,7 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int core_deq_
>  	p->scx.flags &= ~SCX_TASK_REENQ_REASON_MASK;
>  
>  	/* see scx_task_slice_ended() for the save/restore exception */
> -	if (!((deq_flags & DEQUEUE_SAVE) && task_current(rq, p)))
> +	if (!((deq_flags & DEQUEUE_SAVE) && task_current_donor(rq, p)))
>  		scx_task_slice_ended(rq, p);

[Severity: High]
Similarly, does this fail to recognize the physical execution context during
DEQUEUE_SAVE?

If the physically executing task undergoes a DEQUEUE_SAVE cycle,
task_current_donor(rq, p) evaluates to false. Could this spuriously trigger
scx_task_slice_ended(), incorrectly zeroing out its personal slice while it is
running on behalf of the donor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831134338.1531664-1-arighi@nvidia.com?part=14