Re: [PATCH v4 sched_ext/for-7.3 11/40] sched_ext: Add ops.init_cids() to finalize the cid layout before init

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] scx_bpf_cid_override() partially mutates global CID topology mapping tables on failure, leaving them in a corrupted state.
--

--- Patch [11]: [PATCH v4 sched_ext/for-7.3 11/40] sched_ext: Add ops.init_cids() to finalize the cid layout before init ---
commit 0ba160b3b7201c0388f756d1be42ba4c9fc9a45c
Author: Tejun Heo <tj@kernel.org>

sched_ext: Add ops.init_cids() to finalize the cid layout before init

This commit adds a dedicated ops.init_cids() callback for cid-form schedulers
to finalize their cid layout using scx_bpf_cid_override(). This callback runs
before the main ops.init() setup to ensure a stable layout for subsequent
initialization steps.

> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c
> index f31113b080e90..b0ec4d76f8552 100644
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
[ ... ]
> @@ -309,11 +309,6 @@ __bpf_kfunc void scx_bpf_cid_override(const s32 *cpu_to_cid, u32 cpu_to_cid__sz,
>  		return;
>  	}
>  
> -	if (scx_parent(sch)) {
> -		scx_error(sch, "scx_bpf_cid_override() only allowed from root sched");
> -		return;
> -	}
> -
>  	if (cpu_to_cid__sz != nr_cpu_ids * sizeof(s32)) {

[Severity: High]
This is a pre-existing issue, but can this leave the global CID topology
mapping tables in a corrupted state on failure?

Looking at scx_bpf_cid_override() just below this diff, the loop updates
the global tables before fully validating the input:

kernel/sched/ext/cid.c:scx_bpf_cid_override() {
    ...
    for_each_possible_cpu(cpu) {
        s32 c = cpu_to_cid[cpu];

        if (!cid_valid(sch, c))
            return;
        if (cpumask_test_and_set_cpu(c, seen)) {
            scx_error(sch, "cid %d assigned to multiple cpus", c);
            return;
        }
        scx_cpu_to_cid_tbl[cpu] = c;
        scx_cid_to_cpu_tbl[c] = cpu;
    }
    ...
}

If it hits an invalid or duplicate CID mid-way, it calls scx_error() and
returns void. The calling BPF program cannot detect this failure and returns
0 (success), allowing the initialization sequence to proceed using the
partially corrupted CID mapping.

Since the asynchronous teardown work scheduled by scx_error() is blocked
behind scx_enable_mutex during initialization, does this allow the system
to fully transition tasks to the BPF scheduler using the corrupted state
before it can be disabled?

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