* [PATCH] sched_ext: Fix stale errno in scx_sub_enable_workfn()
@ 2026-07-18 20:17 Cui Jian
2026-07-21 17:32 ` Andrea Righi
0 siblings, 1 reply; 4+ messages in thread
From: Cui Jian @ 2026-07-18 20:17 UTC (permalink / raw)
To: Tejun Heo
Cc: David Vernet, Andrea Righi, Changwoo Min, sched-ext, linux-kernel,
Cui Jian
scx_sub_enable_workfn() drops the return value of validate_ops().
When validate_ops() fails, the function jumps to err_disable with
ret still holding 0 from the previous call, so the fallback error
added by commit db4e9defd2e8 ("sched_ext: Record an error on
errno-only sub-enable failure") reports "scx_sub_enable() failed
(0)".
This is currently harmless because validate_ops() records its own
scx_error() first and the first error wins, but it leaves the
fallback broken for this path. Save the return value into ret,
like scx_root_enable_workfn() already does.
The nesting depth check and the cgroup online check also reach
err_disable without setting ret. Set -EINVAL and -ENODEV there so
the fallback always reports a real errno.
Signed-off-by: Cui Jian <cjian720@163.com>
---
kernel/sched/ext/ext.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index e3fa7b2fac9d..e623d6375f66 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -7559,6 +7559,7 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
if (sch->level >= SCX_SUB_MAX_DEPTH) {
scx_error(sch, "max nesting depth %d violated",
SCX_SUB_MAX_DEPTH);
+ ret = -EINVAL;
goto err_disable;
}
@@ -7580,7 +7581,8 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
if (ret)
goto err_disable;
- if (validate_ops(sch, ops))
+ ret = validate_ops(sch, ops);
+ if (ret)
goto err_disable;
struct scx_sub_attach_args sub_attach_args = {
@@ -7613,6 +7615,7 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
set_cgroup_sched(sch_cgroup(sch), sch);
if (!(cgrp->self.flags & CSS_ONLINE)) {
scx_error(sch, "cgroup is not online");
+ ret = -ENODEV;
goto err_unlock_and_disable;
}
base-commit: 1229e2e57a5c2980ccd457b9b53ea0eed5a22ab3
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] sched_ext: Fix stale errno in scx_sub_enable_workfn()
2026-07-18 20:17 [PATCH] sched_ext: Fix stale errno in scx_sub_enable_workfn() Cui Jian
@ 2026-07-21 17:32 ` Andrea Righi
2026-07-22 10:03 ` [PATCH v2] " Cui Jian
0 siblings, 1 reply; 4+ messages in thread
From: Andrea Righi @ 2026-07-21 17:32 UTC (permalink / raw)
To: Cui Jian; +Cc: Tejun Heo, David Vernet, Changwoo Min, sched-ext, linux-kernel
Hi Cui,
On Sun, Jul 19, 2026 at 04:17:13AM +0800, Cui Jian wrote:
> scx_sub_enable_workfn() drops the return value of validate_ops().
> When validate_ops() fails, the function jumps to err_disable with
> ret still holding 0 from the previous call, so the fallback error
> added by commit db4e9defd2e8 ("sched_ext: Record an error on
> errno-only sub-enable failure") reports "scx_sub_enable() failed
> (0)".
>
> This is currently harmless because validate_ops() records its own
> scx_error() first and the first error wins, but it leaves the
> fallback broken for this path. Save the return value into ret,
> like scx_root_enable_workfn() already does.
>
> The nesting depth check and the cgroup online check also reach
> err_disable without setting ret. Set -EINVAL and -ENODEV there so
> the fallback always reports a real errno.
>
> Signed-off-by: Cui Jian <cjian720@163.com>
This doesn't apply anymore, but the logic should still be valid. Can you rebase
it to the latest branch?
git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext.git for-7.3
Thanks,
-Andrea
> ---
> kernel/sched/ext/ext.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index e3fa7b2fac9d..e623d6375f66 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -7559,6 +7559,7 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
> if (sch->level >= SCX_SUB_MAX_DEPTH) {
> scx_error(sch, "max nesting depth %d violated",
> SCX_SUB_MAX_DEPTH);
> + ret = -EINVAL;
> goto err_disable;
> }
>
> @@ -7580,7 +7581,8 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
> if (ret)
> goto err_disable;
>
> - if (validate_ops(sch, ops))
> + ret = validate_ops(sch, ops);
> + if (ret)
> goto err_disable;
>
> struct scx_sub_attach_args sub_attach_args = {
> @@ -7613,6 +7615,7 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
> set_cgroup_sched(sch_cgroup(sch), sch);
> if (!(cgrp->self.flags & CSS_ONLINE)) {
> scx_error(sch, "cgroup is not online");
> + ret = -ENODEV;
> goto err_unlock_and_disable;
> }
>
>
> base-commit: 1229e2e57a5c2980ccd457b9b53ea0eed5a22ab3
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] sched_ext: Fix stale errno in scx_sub_enable_workfn()
2026-07-21 17:32 ` Andrea Righi
@ 2026-07-22 10:03 ` Cui Jian
2026-07-22 13:38 ` Andrea Righi
0 siblings, 1 reply; 4+ messages in thread
From: Cui Jian @ 2026-07-22 10:03 UTC (permalink / raw)
To: Andrea Righi
Cc: Tejun Heo, David Vernet, Changwoo Min, sched-ext, linux-kernel,
Cui Jian
The nesting depth check and the cgroup online check in
scx_sub_enable_workfn() reach err_disable without setting ret, so
the fallback error added by commit db4e9defd2e8 ("sched_ext: Record
an error on errno-only sub-enable failure") reports
"scx_sub_enable() failed (0)".
This is currently harmless because both paths record their own
scx_error() first and the first error wins, but it leaves the
fallback broken for these paths. Set -EINVAL and -ENODEV there
so the fallback always reports a real errno.
v2: The validate_ops() path from v1 is already fixed in for-7.3
(sub.c already has ret = scx_validate_ops()), so only the two
remaining paths are addressed.
Signed-off-by: Cui Jian <cjian720@163.com>
---
kernel/sched/ext/sub.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
index d7842a609d96..6da6c91e4287 100644
--- a/kernel/sched/ext/sub.c
+++ b/kernel/sched/ext/sub.c
@@ -1331,6 +1331,7 @@ void scx_sub_enable_workfn(struct kthread_work *work)
if (sch->level >= SCX_SUB_MAX_DEPTH) {
scx_error(sch, "max nesting depth %d violated",
SCX_SUB_MAX_DEPTH);
+ ret = -EINVAL;
goto err_disable;
}
@@ -1378,6 +1379,7 @@ void scx_sub_enable_workfn(struct kthread_work *work)
set_cgroup_sched(sch_cgroup(sch), sch);
if (!(cgrp->self.flags & CSS_ONLINE)) {
scx_error(sch, "cgroup is not online");
+ ret = -ENODEV;
goto err_unlock_and_disable;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] sched_ext: Fix stale errno in scx_sub_enable_workfn()
2026-07-22 10:03 ` [PATCH v2] " Cui Jian
@ 2026-07-22 13:38 ` Andrea Righi
0 siblings, 0 replies; 4+ messages in thread
From: Andrea Righi @ 2026-07-22 13:38 UTC (permalink / raw)
To: Cui Jian; +Cc: Tejun Heo, David Vernet, Changwoo Min, sched-ext, linux-kernel
Hi Cui,
On Wed, Jul 22, 2026 at 06:03:52PM +0800, Cui Jian wrote:
> The nesting depth check and the cgroup online check in
> scx_sub_enable_workfn() reach err_disable without setting ret, so
> the fallback error added by commit db4e9defd2e8 ("sched_ext: Record
> an error on errno-only sub-enable failure") reports
> "scx_sub_enable() failed (0)".
>
> This is currently harmless because both paths record their own
> scx_error() first and the first error wins, but it leaves the
> fallback broken for these paths. Set -EINVAL and -ENODEV there
> so the fallback always reports a real errno.
>
> v2: The validate_ops() path from v1 is already fixed in for-7.3
> (sub.c already has ret = scx_validate_ops()), so only the two
> remaining paths are addressed.
>
> Signed-off-by: Cui Jian <cjian720@163.com>
Looks good to me now.
Reviewed-by: Andrea Righi <arighi@nvidia.com>
Thanks,
-Andrea
> ---
> kernel/sched/ext/sub.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index d7842a609d96..6da6c91e4287 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -1331,6 +1331,7 @@ void scx_sub_enable_workfn(struct kthread_work *work)
> if (sch->level >= SCX_SUB_MAX_DEPTH) {
> scx_error(sch, "max nesting depth %d violated",
> SCX_SUB_MAX_DEPTH);
> + ret = -EINVAL;
> goto err_disable;
> }
>
> @@ -1378,6 +1379,7 @@ void scx_sub_enable_workfn(struct kthread_work *work)
> set_cgroup_sched(sch_cgroup(sch), sch);
> if (!(cgrp->self.flags & CSS_ONLINE)) {
> scx_error(sch, "cgroup is not online");
> + ret = -ENODEV;
> goto err_unlock_and_disable;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-22 13:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-18 20:17 [PATCH] sched_ext: Fix stale errno in scx_sub_enable_workfn() Cui Jian
2026-07-21 17:32 ` Andrea Righi
2026-07-22 10:03 ` [PATCH v2] " Cui Jian
2026-07-22 13:38 ` Andrea Righi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.