Re: [PATCH 14/14] sched_ext: Fix queued bypassed tasks stalling when the host loses SCX_CAP_BASE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sched_ext: bypass_lb_reenq_home() strands tasks when reenq_cnt hits limit
--

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

sched_ext: Fix queued bypassed tasks stalling when the host loses SCX_CAP_BASE

This commit drains bypass DSQs on CPUs where the host scheduler has lost
SCX_CAP_BASE, reenqueuing tasks at their home runqueue when no donee CPU can
take them or when a remote move is refused, and kicking the bypass load
balancer timer immediately upon SCX_CAP_BASE loss.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 1983ddaa0d605..aaa57143ff751 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -6575,36 +6575,57 @@ bool scx_hardlockup(int cpu)
>  	return true;
>  }
>  
> +/**
> + * bypass_lb_reenq_home - Reenqueue a task at its home from the balancer
> + * @rq: @p's home rq, locked
> + * @p: task off any DSQ, taken from the bypass DSQ of @dsq_cpu
> + * @dsq_cpu: cpu whose bypass DSQ gave up @p
> + *
> + * @p came off the bypass DSQ of @dsq_cpu with no donee able to take it. The
> + * enqueue sends @p to rescue when the host lacks %SCX_CAP_BASE at home and to
> + * the home's bypass DSQ otherwise, see bypass_enq_target_dsq(). No DSQ lock may
> + * be held, as the enqueue takes one.
> + */
> +static void bypass_lb_reenq_home(struct rq *rq, struct task_struct *p, s32 dsq_cpu)
> +{
> +	lockdep_assert_rq_held(rq);
> +
> +	WARN_ON_ONCE(p->scx.flags & SCX_TASK_REENQ_REASON_MASK);
> +	p->scx.flags |= SCX_TASK_REENQ_CAP;
> +	p->scx.reenq_reason_caps = SCX_CAP_BASE;
> +	p->scx.reenq_reason_cid = __scx_cpu_to_cid(dsq_cpu);
> +	scx_do_enqueue_task(rq, p, SCX_ENQ_REENQ, -1);
> +}

[Severity: High]
Can passing SCX_ENQ_REENQ from bypass_lb_reenq_home() to
scx_do_enqueue_task() strand p off all DSQs when p->scx.reenq_cnt exceeds
SCX_REENQ_MAX_REPEAT?

In kernel/sched/ext/ext.c, bypass_lb_move() unlinks p from donor_dsq (leaving
p->scx.dsq == NULL) and calls bypass_lb_reenq_home() when donee < 0 or
task_can_run_on_remote_rq() returns false.

In kernel/sched/ext/ext.c:scx_do_enqueue_task(), the SCX_ENQ_REENQ check runs
before the scx_bypassing() check:

	if (enq_flags & SCX_ENQ_REENQ) {
		if (++p->scx.reenq_cnt > 1)
			__scx_add_event(sch, SCX_EV_REENQ_REPEAT, 1);

		if (unlikely(p->scx.reenq_cnt > SCX_REENQ_MAX_REPEAT)) {
			__scx_exit(sch, SCX_EXIT_ERROR_REENQ, 0, cpu_of(rq),
				   "%s[%d] reenqueued %u times without running",
				   p->comm, p->pid, p->scx.reenq_cnt);
			goto out;
		}
	}

	if (!scx_rq_online(rq))
		goto local;

	if (scx_bypassing(sch, cpu_of(rq))) {
		__scx_add_event(sch, SCX_EV_BYPASS_DISPATCH, 1);
		goto bypass;
	}

When ++p->scx.reenq_cnt > SCX_REENQ_MAX_REPEAT, scx_do_enqueue_task() calls
__scx_exit() on sch = scx_task_sched(p) and jumps to goto out with
action == ENQ_ACTION_NONE, returning without inserting p into any DSQ.

In normal operation, stranding p on rq->scx.runnable_list is recovered when
scx_sub_disable() or scx_root_disable() calls scx_bypass(sch, true), which
transitions sch->bypass_depth from 0 to 1 and re-enqueues all runnable tasks
onto the bypass DSQ.

However, a task on a bypass DSQ already has sch->bypass_depth >= 1:

kernel/sched/ext/ext.c:scx_bypass() {
	if (bypass) {
		changed = inc_bypass_depth(sch);
		if (changed)
			enable_bypass_dsp(sch);
	} else {
		changed = dec_bypass_depth(sch);
	}
	...
	if (!changed)
		goto unlock;
}

Because sch is already in bypass mode, scx_bypass(sch, true) either already
ran or sees changed == false and returns early without walking
rq->scx.runnable_list.

This appears to strand p in two ways:

1. If sch was previously ejected by SCX_EXIT_ERROR_REENQ, p enters
scx_bypass(sch, true) with p->scx.reenq_cnt == SCX_REENQ_MAX_REPEAT + 1.
During scx_bypass(sch, true), dequeue_task_scx() -> ops_dequeue() calls
clr_task_runnable(p, false):

kernel/sched/ext/ext.c:clr_task_runnable() {
	list_del_init(&p->scx.runnable_node);
	WRITE_ONCE(p->scx.runnable_cpu, -1);
	if (reset_runnable_at) {
		p->scx.flags |= SCX_TASK_RESET_RUNNABLE_AT;
		p->scx.reenq_cnt = 0;
	}
}

Because reset_runnable_at is false until p actually runs in
set_next_task_scx(), p sits on the bypass DSQ with p->scx.reenq_cnt still
above SCX_REENQ_MAX_REPEAT. On the first call to bypass_lb_reenq_home()
before p runs, scx_do_enqueue_task() hits the limit and jumps to goto out
before reaching goto bypass, leaving p off all DSQs instead of sending it to
rescue or its home bypass DSQ.

2. Even if p->scx.reenq_cnt starts at 0, after SCX_REENQ_MAX_REPEAT + 1
passes in bypass mode (for instance, when rescue is disabled on a capless CPU
or when task_can_run_on_remote_rq() repeatedly refuses a move for a
proxy-migrated blocked donor), scx_do_enqueue_task() calls __scx_exit() on
the already-bypassing child sch = scx_task_sched(p) rather than the host
scheduler that lost SCX_CAP_BASE, and jumps to goto out. Since
sch->bypass_depth is already >= 1, nothing rescues p from
rq->scx.runnable_list, leaving p permanently off all DSQs and able to stall
scx_sub_disable() or scx_root_disable().

Should scx_do_enqueue_task() avoid stranding p (or check scx_bypassing() /
target the host scheduler) when reenqueuing a bypassed task?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022213836.2259069-1-tj@kernel.org?part=14