Re: [PATCH 12/12] sched_ext: scx_qmap - Add rescue support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the `!(enq_flags & SCX_ENQ_REENQ)` check for stranded tasks causes an infinite enqueue-reject-reenqueue loop when rescue is disabled.
--

--- Patch [12]: [PATCH 12/12] sched_ext: scx_qmap - Add rescue support ---
commit d8697209a4441d32aa069b5cede51b062fddcc4c
Author: Tejun Heo <tj@kernel.org>

sched_ext: scx_qmap - Add rescue support

This commit adds rescue support to scx_qmap for tasks that have no self cids
they can run on. It updates the qmap_enqueue and qmap_dispatch logic to use
the SCX_ENQ_RESCUE flag and scan the shared queue to redirect stranded tasks.

> diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
> index 9a0321e84e88f..b6e7b004611c0 100644
> --- a/tools/sched_ext/scx_qmap.bpf.c
> +++ b/tools/sched_ext/scx_qmap.bpf.c
> @@ -449,31 +449,33 @@ void BPF_STRUCT_OPS(qmap_enqueue, struct task_struct *p, u64 enq_flags)
>  	taskc->core_sched_seq = qa.core_sched_tail_seqs[idx]++;
>  
>  	/*
> -	 * A node with children delegates most cids. A task of ours that can run
> -	 * on none of our self cids (e.g. a per-NUMA kthread pinned to delegated
> -	 * cids) would starve in SHARED/FIFO since we never pull those on a
> -	 * delegated cid. Force it onto its first allowed cid's local DSQ with
> -	 * needs_immed(): if we hold access there it runs, else the kernel
> -	 * rejects and bounces it back via REENQ_CAP. Best-effort
> -	 * anti-starvation nudge.
> +	 * A task of ours that can run on none of our self cids - the parent
> +	 * didn't grant them or we delegated them to children - would starve in
> +	 * SHARED/FIFO since we only pull from those on self cids.
> +	 *
> +	 * Force it onto its first allowed cid's local DSQ. If we hold that cid
> +	 * it runs. Otherwise the insert carries SCX_ENQ_RESCUE and the kernel
> +	 * diverts the task to its rescue path.
>  	 */
> -	if (qa.nr_sub_scheds && !(enq_flags & SCX_ENQ_REENQ) &&
> -	    !cmask_intersects(&taskc->cpus_allowed, &qa.self_cids.mask)) {
> +	if (!cmask_intersects(&taskc->cpus_allowed, &qa.self_cids.mask)) {

[Severity: High]
Does removing the !(enq_flags & SCX_ENQ_REENQ) check here cause an
infinite bounce loop when rescue is disabled kernel-side?

If scx_qmap is executed with -B 0, the kernel capability check rejects the
insert and bounces the task back to ops.enqueue() with the SCX_ENQ_REENQ
flag.

Without the check for SCX_ENQ_REENQ, qmap_enqueue() will immediately hit
this condition again and force-insert the task back to the target CPU.

Could this lead to an endless, unthrottled loop that locks up the core?

>  		s32 c = cmask_next_set_wrap(&taskc->cpus_allowed, 0);
>  
>  		if (c >= 0 && c < scx_bpf_nr_cids()) {
>  			taskc->force_local = false;
> +			__sync_fetch_and_add(&qa.nr_rescue_dsp, 1);
>  			scx_bpf_dsq_insert(p, SCX_DSQ_LOCAL_ON | c, slice_ns,
> -					   enq_flags | needs_immed(c));
> +					   enq_flags | needs_immed(c) | SCX_ENQ_RESCUE);
>  			return;
>  		}
>  	}

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