Re: [PATCH sched_ext/for-7.3] sched_ext: skip ops.set_weight() for disabled tasks

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] reweight_task_scx() fails to update p->se.load, causing tasks to have permanently stale load weights when their priority changes or when transitioning from sched_ext back to sched_fair (CFS).
--

--- Patch [1]: [PATCH sched_ext/for-7.3] sched_ext: skip ops.set_weight() for disabled tasks ---
commit 009696d9cb0a5605e8759533b00b9204ec011c72
Author: Kuba Piecuch <[email protected]>

sched_ext: skip ops.set_weight() for disabled tasks

When switching a task's sched_class away from sched_ext, ops.set_weight()
is currently called after ops.disable(). This commit skips the weight
adjustment for disabled tasks to respect the expected callback semantics,
allowing the weight to be recalculated later if the task rejoins SCX.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 691d53fe0f648..ff22f31594c5c 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -3925,6 +3925,17 @@ static void reweight_task_scx(struct rq *rq, struct task_struct *p,
>  	if (task_dead_and_done(p))
>  		return;
>  
> +	/*
> +	 * When switching sched_class away from SCX, reweight_task_scx()
> +	 * is called _after_ scx_disable_task(). Skip calling ops.set_weight()
> +	 * since the BPF scheduler may have already forgotten the task in
> +	 * ops.disable().
> +	 * p->scx.weight will be recalculated in scx_enable_task() if the task
> +	 * ever returns to SCX class.
> +	 */
> +	if (scx_get_task_state(p) != SCX_TASK_ENABLED)
> +		return;
> +
>  	p->scx.weight = sched_weight_to_cgroup(scale_load_down(lw->weight));
>  	if (SCX_HAS_OP(sch, set_weight))
>  		SCX_CALL_OP_TASK(sch, set_weight, rq, p, p->scx.weight);

[Severity: High]
This is a pre-existing issue, but does reweight_task_scx() need to update
p->se.load as well?

Looking at set_load_weight() in kernel/sched/core.c, it delegates the load
weight update to the current scheduling class's callback if available:

kernel/sched/core.c:set_load_weight() {
	...
	if (update_load && p->sched_class->reweight_task)
		p->sched_class->reweight_task(task_rq(p), p, &lw);
	else
		p->se.load = lw;
	...
}

When a task's priority is modified while it is in the sched_ext class,
set_load_weight() calls reweight_task_scx(). Because reweight_task_scx()
only updates p->scx.weight and entirely skips updating p->se.load, does the
CFS entity end up with a permanently stale load weight?

If the task's scheduling policy is later switched back to sched_fair, it
seems it would enter CFS with this incorrect load weight. Could this bypass
nice limits and break CFS fairness since the load weight dictates the task's
share of CPU time?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1