Re: [PATCH] sched_ext: Delete sub-scheduler kobjects before releasing scx_enable_mutex

[Andrea Righi <arighi@nvidia.com>]

Hello,

On Tue, Sep 01, 2026 at 06:22:40PM +0800, Qiurong Fang wrote:
> From: fangqiurong <fangqiurong@kylinos.cn>
> 
> The sub-scheduler disable path deletes the scheduler's kobjects after
> releasing scx_enable_mutex, while the root path deletes them before
> releasing it. A concurrent enable on the same cgroup can therefore hit
> kobject_add() with the same "sub-%llu" name still in the hierarchy and
> fail with -EEXIST, tearing down an otherwise healthy scheduler.

This race seems legit to me, but I think the commit message should clarify that
the concurrent enable must use a different struct_ops instance. Re-enabling the
same instance is rejected by the ops->priv check and a normal sequential
unregister/register is serialized.

With that:

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Bonus: would it also be possible to add a sched_ext kselftest for this? The race
should be reproducible deterministically by blocking the parent's
ops.sub_detach() callback after the first child has been unlinked, and then
attaching a second struct_ops instance to the same cgroup. Without this change,
the second scheduler should exit because scx_sched_sysfs_add() returns -EEXIST;
with the change, it should attach successfully.

Thanks,
-Andrea

> 
> Move the two kobject_del() calls above mutex_unlock() to match the root
> path.
> 
> Fixes: ebeca1f930ea ("sched_ext: Introduce cgroup sub-sched support")
> Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>
> ---
>  kernel/sched/ext/sub.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index a17d84db93bd..1923e3023bff 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -1631,6 +1631,12 @@ void scx_sub_disable(struct scx_sched *sch)
>  
>  	scx_unlink_sched(sch);
>  
> +	if (sch->sub_kset)
> +		kobject_del(&sch->sub_kset->kobj);
> +	/* not added if enable failed before scx_sched_sysfs_add() */
> +	if (sch->kobj.state_in_sysfs)
> +		kobject_del(&sch->kobj);
> +
>  	mutex_unlock(&scx_enable_mutex);
>  
>  	/*
> @@ -1661,12 +1667,6 @@ void scx_sub_disable(struct scx_sched *sch)
>  	 */
>  	WRITE_ONCE(sch->dead, true);
>  	synchronize_rcu();
> -
> -	if (sch->sub_kset)
> -		kobject_del(&sch->sub_kset->kobj);
> -	/* not added if enable failed before scx_sched_sysfs_add() */
> -	if (sch->kobj.state_in_sysfs)
> -		kobject_del(&sch->kobj);
>  }
>  
>  /* verify that a scheduler can be attached to @cgrp and return the parent */
> -- 
> 2.43.0
>