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

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

[luoliang]

From: Liang Luo <luoliang@kylinos.cn>

If the SCX_ENABLING -> SCX_ENABLED cmpxchg at the tail of
scx_root_enable_workfn() fails, the function jumps to err_disable
without setting ret. At that point ret still holds the return value
of the last successful __scx_init_task() call, which is 0, so the
err_disable fallback reports the meaningless message:

  scx_root_enable() failed (0)

Set ret = -EBUSY, consistent with the other enable-state guards at
the top of the same function, so the fallback always reports a real
errno.

Signed-off-by: Liang Luo <luoliang@kylinos.cn>
---
 kernel/sched/ext/ext.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 312938179e52..6a3e94707c8b 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -7520,6 +7520,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;
 	}
 
-- 
2.43.0

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

[sashiko-bot]

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

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

[luoliang]

On Tue, Jul 28, 2026 at 06:28:06AM +0000, sashiko-bot@kernel.org wrote:
> 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.

The catch-all scx_error() at the err_disable sink in
scx_sub_enable_workfn() already exists in mainline. It was added by
commit db4e9defd2e8 ("sched_ext: Record an error on errno-only
sub-enable failure"):

    err_disable:
        mutex_unlock(&scx_enable_mutex);
        /*
         * Some enable failures only return an errno (e.g. -ENOMEM from an
         * allocation) without calling scx_error(). Record it so
         * scx_flush_disable_work() runs the disable and ops.exit() fires.
         */
        scx_error(sch, "scx_sub_enable() failed (%d)", ret);
        scx_flush_disable_work(sch);
        cmd->ret = 0;

So the silent failure described in the review no longer exists.

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

[Tejun Heo]

Applied to sched_ext/for-7.3.

Thanks.

--
tejun