Re: [PATCH 10/11] sched_ext: scx_qmap: Add proxy execution support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant !(enq_flags & SCX_ENQ_REENQ) checks left behind after moving the early return handler.
--

--- Patch [10]: [PATCH 10/11] sched_ext: scx_qmap: Add proxy execution support ---
commit fb0f1d9957f7124711aadbe323f38da6fa6e3857
Author: Andrea Righi <[email protected]>

sched_ext: scx_qmap: Add proxy execution support

This commit adds a -B option to scx_qmap to opt into queueing mutex-blocked
tasks for proxy execution. It dispatches blocked donors directly to the
local DSQ of their current cid with a fresh slice and SCX_ENQ_PREEMPT.

> diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
> index 09aee49120c28..ab14325a07b6d 100644
> --- a/tools/sched_ext/scx_qmap.bpf.c
> +++ b/tools/sched_ext/scx_qmap.bpf.c
> @@ -451,6 +451,40 @@ void BPF_STRUCT_OPS(qmap_enqueue, struct task_struct *p, u64 enq_flags)
>  	 */
>  	taskc->core_sched_seq = qa.core_sched_tail_seqs[idx]++;
>  
> +	/*
> +	 * If the task was re-enqueued due to the CPU being preempted by a
> +	 * higher priority scheduling class, just re-enqueue the task directly
> +	 * on the global DSQ. As we want another CPU to pick it up, find and
> +	 * kick an idle cid.
> +	 */
> +	if (enq_flags & SCX_ENQ_REENQ) {
> +		taskc->force_local = false;
> +		scx_bpf_dsq_insert(p, SHARED_DSQ, 0, enq_flags);
> +		cid = cmask_next_and2_set_wrap(&taskc->cpus_allowed,
> +					       &qa.idle_cids.mask,
> +					       &qa.self_cids.mask, 0);
> +		if (cid < scx_bpf_nr_cids())
> +			scx_bpf_kick_cid(cid, SCX_KICK_IDLE);
> +		return;
> +	}

[Severity: Low]
Since the early return block for SCX_ENQ_REENQ was moved higher up in
qmap_enqueue(), are the later checks for !(enq_flags & SCX_ENQ_REENQ)
now redundant?

For example, further down in the same function:

        if (qa.nr_sub_scheds && !(enq_flags & SCX_ENQ_REENQ) &&
            !cmask_intersects(&taskc->cpus_allowed, &qa.self_cids.mask)) {

It seems SCX_ENQ_REENQ will never be set when reaching this code now, making
the condition safely removable.

[ ... ]

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