Re: [PATCH] sched_ext: Set errno on ENABLING -> ENABLED transition failure
[email protected] Tue, 28 Jul 2026 06:28:06 +0000
| Newsgroups | dev.linux.lists.sched-ext |
|---|---|
| Message-ID | <[email protected]> |
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 pat= hs 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 transiti= on failure --- commit 23b2e75396f348a4f6a2b8cadd57f78dd5b36fcb Author: Liang Luo <[email protected]> sched_ext: Set errno on ENABLING -> ENABLED transition failure =20 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_w= ork *work) > =20 > if (!scx_tryset_enable_state(SCX_ENABLED, SCX_ENABLING)) { > WARN_ON_ONCE(atomic_read(&sch->exit_kind) =3D=3D SCX_EXIT_NONE); > + ret =3D -EBUSY; > goto err_disable; > } > =20 [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 =3D 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 =3D __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 =3D 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? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260728060955.4102= [email protected]?part=3D1