Re: [PATCH 7.0.y] sched_ext: Skip tasks with stale task_rq in bypass_lb_cpu()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unbounded lock hold time with IRQs disabled when skipping tasks in `bypass_lb_cpu()`.
- [Low] Obsolete comment contradicts the new task_rq lock constraints.
--

commit 68327c3a383140f9157a96a24a7b23be2c135ac5
Author: Tejun Heo <tj@kernel.org>

sched_ext: Skip tasks with stale task_rq in bypass_lb_cpu()

This commit updates bypass_lb_cpu() to skip tasks whose task_rq() does not
match the donor rq. These tasks were placed on the donor DSQ from a different
CPU but haven't been consumed yet, preventing their migration without the
proper rq lock.

> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c

[ ... ]

>  	while ((p = n)) {
>  		struct rq *donee_rq;

[ ... ]

> +		/*
> +		 * If an earlier pass placed @p on @donor_dsq from a different
> +		 * CPU and the donee hasn't consumed it yet, @p is still on the
> +		 * previous CPU and task_rq(@p) != @rq. @p can't be moved
> +		 * without its rq locked. Skip.
> +		 */
> +		if (task_rq(p) != rq)
> +			continue;

Because this uses continue, does it bypass the lock-yielding check at the
bottom of the loop (if (!(nr_balanced % SCX_BYPASS_LB_BATCH)))?

If there is a large batch of unconsumed tasks left over from previous passes,
could skipping them without incrementing nr_balanced result in an unbounded
loop?

Traversing a long list here while holding the rq lock and donor_dsq lock
with IRQs disabled could potentially cause latency spikes or trigger the
soft lockup watchdog.

>  		donee = cpumask_any_and_distribute(donee_mask, p->cpus_ptr);
>  		if (donee >= nr_cpu_ids)
>  			continue;
>  
>  		donee_rq = cpu_rq(donee);
>  		donee_dsq = &donee_rq->scx.bypass_dsq;
>  
>  		/*
>  		 * $p's rq is not locked but $p's DSQ lock protects its
>  		 * scheduling properties making this test safe.
>  		 */

This isn't a bug, but is this comment still accurate?

Since the function initially acquires raw_spin_rq_lock_irq(rq) and the
newly added check guarantees task_rq(p) == rq, it appears the task's rq is
actually locked when it reaches this point.

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