Re: [PATCH sched_ext/for-7.4-fixes] sched_ext: Call ops.sub_attach() only while the cgroup is online

[Andrea Righi]

On Thu, Oct 29, 2026 at 12:28:59PM -1000, Tejun Heo wrote:
> The parent's ops.sub_attach() runs before the enable work checks that the
> cgroup is still online, so the parent can get an attach for a cgroup whose
> rmdir already returned. That complicates parents where cgroup cleanup and
> sub-scheduler attach race.
> 
> Check CSS_ONLINE and call ops.sub_attach() under cgroup_lock(), so that the
> parent sees an attach only for a cgroup that is online.
> 
> Fixes: ebeca1f930ea ("sched_ext: Introduce cgroup sub-sched support")
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>

Reviewed-by: Andrea Righi <arighi@xxxxxxxxxx>

Thanks,
-Andrea

> ---
>  kernel/sched/ext/internal.h |  4 ++++
>  kernel/sched/ext/sub.c      | 13 +++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
> index ff0a3d508533..58652370da5a 100644
> --- a/kernel/sched/ext/internal.h
> +++ b/kernel/sched/ext/internal.h
> @@ -869,6 +869,10 @@ struct sched_ext_ops {
>  	 * @sub_attach: Attach a sub-scheduler
>  	 * @args: argument container, see the struct definition
>  	 *
> +	 * The sub-scheduler's cgroup is online for the whole call. If it goes
> +	 * offline after the sub-scheduler is accepted, ops.sub_detach()
> +	 * follows.
> +	 *
>  	 * Return 0 to accept the sub-scheduler. -errno to reject.
>  	 */
>  	s32 (*sub_attach)(struct scx_sub_attach_args *args);
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index b5d92d89629b..6bc6b93ca63a 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -2340,8 +2340,21 @@ void scx_sub_enable_workfn(struct kthread_work *work)
>  		.cgroup_path = sch->cgrp_path,
>  	};
>  
> +	/*
> +	 * The cgroup stays online across ops.sub_attach(), so the parent never
> +	 * sees an attach for a cgroup that rmdir already removed.
> +	 */
> +	cgroup_lock();
> +	if (!(cgrp->self.flags & CSS_ONLINE)) {
> +		cgroup_unlock();
> +		scx_error(sch, "cgroup is not online");
> +		ret = -ENODEV;
> +		goto err_disable;
> +	}
>  	ret = SCX_CALL_OP_RET(parent, sub_attach, NULL,
>  			      &sub_attach_args);
> +	cgroup_unlock();
> +
>  	if (ret) {
>  		ret = scx_ops_sanitize_err(sch, "sub_attach", ret);
>  		scx_error(sch, "parent rejected (%d)", ret);
> -- 
> 2.56.0
>