Re: [PATCH v3 sched_ext/for-7.3 10/36] 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:
- [Medium] Inconsistent ops.init() execution order and silent ignoring of ops.init_cids() for sub-schedulers.
--

--- Patch [10]: [PATCH v3 sched_ext/for-7.3 10/36] sched_ext: Add ops.init_cids() to finalize the cid layout before init ---
commit 203e12db1eb59d2f31e8e189609af23a163797d2
Author: Tejun Heo <tj@kernel.org>

sched_ext: Add ops.init_cids() to finalize the cid layout before init
    
This commit introduces an ops.init_cids() callback for cid-form schedulers
to finalize their layout prior to initialization. It allows enabling setup
that relies on the final cid layout to run after the layout is established,
and moves ops.init() execution to run after arena pool and cmask allocations.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 150e74129266b..ee390d95cd2e3 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -6684,15 +6684,19 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  
>  	scx_idle_enable(ops);
>  
> -	if (sch->ops.init) {
> -		ret = SCX_CALL_OP_RET(sch, init, NULL);
> +	/*
> +	 * A cid-form scheduler finalizes its cid layout in ops.init_cids(),
> +	 * which may call scx_bpf_cid_override(). Run it before ops.init() so
> +	 * the final layout is in effect.
> +	 */
> +	if (sch->is_cid_type && sch->ops_cid.init_cids) {
> +		ret = SCX_CALL_OP_RET(sch, init_cids, NULL);
>  		if (ret) {
> -			ret = scx_ops_sanitize_err(sch, "init", ret);
> +			ret = scx_ops_sanitize_err(sch, "init_cids", ret);
>  			cpus_read_unlock();
> -			scx_error(sch, "ops.init() failed (%d)", ret);
> +			scx_error(sch, "ops.init_cids() failed (%d)", ret);
>  			goto err_disable;
>  		}
> -		sch->exit_info->flags |= SCX_EFLAG_INITIALIZED;
>  	}
>  
>  	ret = scx_arena_pool_init(sch);
> @@ -6707,6 +6711,17 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  		goto err_disable;
>  	}
>  
> +	if (sch->ops.init) {
> +		ret = SCX_CALL_OP_RET(sch, init, NULL);

[Severity: Medium]
Will this change cause an inconsistent initialization state for sub-schedulers?

While scx_root_enable_workfn() has been updated to run ops.init() after 
arena pool and cmask allocations and introduces ops.init_cids(), the
equivalent sub-scheduler setup path scx_sub_enable_workfn() in
kernel/sched/ext/sub.c appears to have been left unmodified.

As a result, in scx_sub_enable_workfn(), ops.init_cids() is silently
ignored, and ops.init() still executes before the arena and cmask
allocations. Should scx_sub_enable_workfn() be updated to maintain a
consistent initialization order and either support or explicitly reject
ops.init_cids()?

> +		if (ret) {
> +			ret = scx_ops_sanitize_err(sch, "init", ret);
> +			cpus_read_unlock();
> +			scx_error(sch, "ops.init() failed (%d)", ret);
> +			goto err_disable;
> +		}
> +		sch->exit_info->flags |= SCX_EFLAG_INITIALIZED;
> +	}
> +

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