Re: [PATCH v4 sched_ext/for-7.3 15/40] sched_ext: Add per-shard scx_sched storage scaffolding

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing scx_alloc_pshards() call for sub-schedulers in scx_sub_enable_workfn(), breaking shard scaffolding.
--

--- Patch [15]: [PATCH v4 sched_ext/for-7.3 15/40] sched_ext: Add per-shard scx_sched storage scaffolding ---
commit 3777e1c0aa64eae381accea1de5bddc71fe6cf2d
Author: Tejun Heo <[email protected]>

sched_ext: Add per-shard scx_sched storage scaffolding

This commit adds the scx_pshard structure and sch->pshard[] array indexed by shard_idx, allocating each entry on its shard's NUMA node. The allocation is intended for cid-type schedulers with an arena pool, serving as scaffolding for upcoming shard-local state.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 1a8d62efb5c52..6892928b0acbe 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
[ ... ]
> @@ -6724,6 +6726,12 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  		goto err_disable;
>  	}
>  
> +	ret = scx_alloc_pshards(sch);
> +	if (ret) {
> +		cpus_read_unlock();
> +		goto err_disable;
> +	}
> +

[Severity: Medium]
Is it intentional that scx_alloc_pshards() is called here for the root
scheduler, but omitted in scx_sub_enable_workfn() for sub-schedulers?

The commit message notes: "Only cid-type schedulers with an arena pool get
pshards." Sub-schedulers are cid-form and initialize an arena pool in
scx_sub_enable_workfn(), but appear to be missing the allocation call:

kernel/sched/ext/sub.c:scx_sub_enable_workfn() {
    ...
	ret = scx_arena_pool_init(sch);
	if (ret)
		goto err_disable;

	ret = scx_set_cmask_scratch_alloc(sch);
	if (ret)
		goto err_disable;

	if (scx_validate_ops(sch, ops))
		goto err_disable;
    ...
}

Could this cause sub-schedulers to fail to allocate the pshard array, leaving
sch->pshard as NULL and leading to crashes in follow-up patches that rely on
this scaffolding?

>  	if (sch->ops.init) {
>  		ret = SCX_CALL_OP_RET(sch, init, NULL);
>  		if (ret) {

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