Re: [PATCH 5/5] sched_ext: Gate scx_bpf_cidperf_set() behind a new SCX_CAP_PERF

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The scx_qmap scheduler breaks backward compatibility on older kernels by unconditionally passing SCX_CAP_PERF to scx_bpf_sub_grant() and scx_bpf_sub_revoke().
--

--- Patch [5]: [PATCH 5/5] sched_ext: Gate scx_bpf_cidperf_set() behind a new SCX_CAP_PERF ---
commit 3bc27e508735d52ab01e9e24c0c210b6cb92366e
Author: Tejun Heo <[email protected]>

sched_ext: Gate scx_bpf_cidperf_set() behind a new SCX_CAP_PERF

scx_bpf_cidperf_set() reaches cpufreq with no cap check, so any cid-form
sub-sched can steer the frequency of any cid in its view. This gates the
operation behind a new SCX_CAP_PERF capability check.

> diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
> index aead17658573b..8822ed11c0d8b 100644
> --- a/tools/sched_ext/scx_qmap.bpf.c
> +++ b/tools/sched_ext/scx_qmap.bpf.c
> @@ -1585,11 +1585,13 @@ __noinline void apply_partition(void)
>  		cmask_copy(&qa.to_grant_cids.mask, &ssc->granted_cids.mask);
>  		cmask_andnot(&qa.to_grant_cids.mask, &ssc->prev_granted.mask);
>  
> -		scx_bpf_sub_revoke(cgid, SCX_CAP_ENQ_IMMED,
> +		scx_bpf_sub_revoke(cgid, SCX_CAP_ENQ_IMMED | SCX_CAP_PERF,
>  				   (void *)(long)&qa.prev_rr_cids.mask);
> -		scx_bpf_sub_revoke(cgid, SCX_CAP_ENQ | SCX_CAP_PREEMPT | SCX_CAP_ENQ_IMMED,
> +		scx_bpf_sub_revoke(cgid, SCX_CAP_ENQ | SCX_CAP_PREEMPT |
> +				   SCX_CAP_ENQ_IMMED | SCX_CAP_PERF,
>  				   (void *)(long)&qa.to_revoke_cids.mask);
> -		scx_bpf_sub_grant(cgid, SCX_CAP_ENQ | SCX_CAP_PREEMPT | SCX_CAP_ENQ_IMMED,
> +		scx_bpf_sub_grant(cgid, SCX_CAP_ENQ | SCX_CAP_PREEMPT |
> +				  SCX_CAP_ENQ_IMMED | SCX_CAP_PERF,
>  				  (void *)(long)&qa.to_grant_cids.mask, NULL);
>  	}

[Severity: High]
Does unconditionally passing SCX_CAP_PERF break backward compatibility on
older kernels?

If scx_qmap is loaded on a pre-gate kernel where SCX_CAP_PERF is not part of
__SCX_CAP_ALL, won't the kernel's scx_bpf_sub_grant() and scx_bpf_sub_revoke()
reject the unknown cap bit?

Since they call scx_error() on invalid caps, would this immediately abort
the BPF scheduler and revert the system to CFS on older kernels?

[ ... ]

> @@ -1696,10 +1699,12 @@ static void rr_advance(void)
>  		 * time-share.
>  		 */
>  		if (old_cgid)
> -			scx_bpf_sub_revoke(old_cgid, SCX_CAP_ENQ_IMMED,
> +			scx_bpf_sub_revoke(old_cgid,
> +					   SCX_CAP_ENQ_IMMED | SCX_CAP_PERF,
>  					   (void *)(long)&qa.rr_cids.mask);
>  		if (new_cgid)
> -			scx_bpf_sub_grant(new_cgid, SCX_CAP_ENQ_IMMED,
> +			scx_bpf_sub_grant(new_cgid,
> +					  SCX_CAP_ENQ_IMMED | SCX_CAP_PERF,
>  					  (void *)(long)&qa.rr_cids.mask, NULL);
>  	}

[Severity: High]
Does the same compatibility issue apply here during the round-robin advance
in rr_advance()?

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