Re: [PATCH] sched_ext: Set errno on ENABLING -> ENABLED transition failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `scx_error()` calls in `scx_sub_enable_workfn()` error paths cause silent failures, leaving the sub-scheduler orphaned and returning a false success to userspace.
--

--- Patch [1]: [PATCH] sched_ext: Set errno on ENABLING -> ENABLED transition failure ---
commit 23b2e75396f348a4f6a2b8cadd57f78dd5b36fcb
Author: Liang Luo <luoliang@kylinos.cn>

sched_ext: Set errno on ENABLING -> ENABLED transition failure
    
This commit fixes an issue in scx_root_enable_workfn() where a failure
during the ENABLING to ENABLED transition would jump to err_disable without
setting a return value. It sets ret to -EBUSY so the fallback reports a real
errno instead of a meaningless message.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 691d53fe0f648..84322adb69c80 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -7323,6 +7323,7 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  
>  	if (!scx_tryset_enable_state(SCX_ENABLED, SCX_ENABLING)) {
>  		WARN_ON_ONCE(atomic_read(&sch->exit_kind) == SCX_EXIT_NONE);
> +		ret = -EBUSY;
>  		goto err_disable;
>  	}
>  

[Severity: High]
This is a pre-existing issue, but since this patch improves error reporting
in scx_root_enable_workfn(), does scx_sub_enable_workfn() suffer from a
similar silent failure problem?

Unlike scx_root_enable_workfn(), the err_disable label in
scx_sub_enable_workfn() lacks a catch-all scx_error() call. If an error
path jumps there without already invoking scx_error(), it appears to
result in a silent failure.

For example, if scx_arena_pool_init() fails:

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

Or if __scx_init_task() fails:

kernel/sched/ext/ext.c:scx_sub_enable_workfn() {
    ...
        ret = __scx_init_task(sch, p, false);
        if (ret)
            goto abort;
    ...
}

Both paths eventually reach err_disable, which just unlocks the mutex,
flushes the disable work, and returns success (cmd->ret = 0). Since
scx_error() was never called, sch->exit_kind remains SCX_EXIT_NONE,
scx_flush_disable_work() does nothing, and the subsystem is left in a
broken state while userspace is told the operation succeeded.

Would it be beneficial to also ensure scx_sub_enable_workfn() properly
reports these errors, perhaps by adding a catch-all scx_error() call there
too?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728060955.4102964-1-luoliang@kylinos.cn?part=1