Re: [PATCH v4 sched_ext/for-7.3 27/40] sched_ext: Add the SCX_CAP_ENQ_IMMED cap

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scx_caps_for_enq() unconditionally maps all local DSQ enqueues to SCX_CAP_ENQ_IMMED, allowing sub-schedulers to bypass isolation guarantees and swamp the CPU with non-IMMED tasks.
- [High] Tasks in a bypassing descendant's runqueue can be permanently stranded if their nearest non-bypassing ancestor lacks SCX_CAP_BASE on the CPU.
--

--- Patch [27]: [PATCH v4 sched_ext/for-7.3 27/40] sched_ext: Add the SCX_CAP_ENQ_IMMED cap ---
commit 5bf5e66c851f3131250d2bde5c5d59c7f1533314
Author: Tejun Heo <[email protected]>

sched_ext: Add the SCX_CAP_ENQ_IMMED cap
    
This commit introduces the SCX_CAP_ENQ_IMMED capability to replace the dummy
placeholder, acting as a gate for inserting IMMED tasks into a local DSQ. It
establishes ENQ_IMMED as the baseline capability required to use a CPU,
enforcing that inserts without this cap are rejected or tasks reenqueued.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index 787cc36dc7b73..c741918c21a78 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -1279,6 +1279,14 @@ __bpf_kfunc bool scx_bpf_sub_dispatch(u64 cgroup_id, const struct bpf_prog_aux *
>  		return false;
>  	}
>  
> +	/*
> +	 * Skip a child that does not effectively hold the base cap on this cpu:
> +	 * its inserts would only be rejected. ecaps are synced at the top of
> +	 * balance_one() before dispatch, so this reflects the in-effect state.
> +	 */
> +	if (scx_missing_caps(child, cpu_of(this_rq), SCX_CAP_BASE))
> +		return false;
> +
> +	return scx_dispatch_sched(child, this_rq, this_rq->scx.sub_dispatch_prev,
> +				  true);
>  }

[Severity: High]
Could this early return cause a regression that permanently strands tasks in a
bypassing descendant's runqueue?

If a grandchild scheduler enters bypass mode, its enqueued tasks are routed 
to the per-CPU bypass DSQ of its nearest non-bypassing ancestor (the child 
scheduler). If that child scheduler then loses the SCX_CAP_BASE capability 
on this specific CPU, this check returns false and skips scx_dispatch_sched(). 

Since bypass DSQs are strictly per-CPU and only consumed via 
scx_dispatch_sched(), will the child's bypass DSQ remain unpumped? 

Capability revocation via scx_process_sync_ecaps() only schedules a 
re-enqueue scan for the local DSQ, so it seems tasks trapped in the bypass 
DSQ would never be rescued.

[ ... ]

> diff --git a/kernel/sched/ext/sub.h b/kernel/sched/ext/sub.h
> index 51400e174cd00..f3548976800b0 100644
> --- a/kernel/sched/ext/sub.h
> +++ b/kernel/sched/ext/sub.h
> @@ -106,13 +106,13 @@ static inline u64 scx_missing_caps(struct scx_sched *sch, s32 cpu, u64 needed)
>  /* map @enq_flags to the SCX_CAP_* bit required for the local-DSQ insert */
>  static inline u64 scx_caps_for_enq(u64 enq_flags)
>  {
> -	return 0;
> +	return SCX_CAP_ENQ_IMMED;
>  }
>  
>  /* map queued @p to the SCX_CAP_* bit required to stay on its local DSQ */
>  static inline u64 scx_caps_for_task(struct task_struct *p)
>  {
> -	return 0;
> +	return SCX_CAP_ENQ_IMMED;
>  }

[Severity: High]
Does unconditionally returning SCX_CAP_ENQ_IMMED introduce a regression 
allowing sub-schedulers to bypass isolation guarantees?

Because scx_caps_for_enq() does not inspect enq_flags for SCX_ENQ_IMMED, a
sub-scheduler holding only the baseline IMMED capability could call
scx_bpf_dispatch() to enqueue a non-IMMED task to a local DSQ. 

The task would then be placed in the local DSQ without the SCX_TASK_IMMED
flag. This would mean it sits in the queue indefinitely rather than forcing a 
CPU eviction, which seems to contradict the commit message's goal of 
ensuring IMMED work can never pile up.

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