Re: [PATCH 29/34] sched_ext: Implement cgroup sub-sched enabling and disabling

[Cheng-Yang Chou]

Hi Tejun,

I've been reading through this patch and I think I may have spotted a
lock leak in the abort: error path of scx_sub_enable_workfn(), but I'm
not fully familiar with this code so please correct me if I'm wrong.

percpu_down_write(&scx_fork_rwsem) and scx_cgroup_lock() are acquired
before the first task iteration loop:

	percpu_down_write(&scx_fork_rwsem);
	scx_cgroup_lock();

On Wed, Mar 04, 2026 at 12:01:14PM -1000, Tejun Heo wrote:
> +abort:
> +	put_task_struct(p);
> +	scx_task_iter_stop(&sti);
> +	scx_enabling_sub_sched = NULL;
> +
> +	scx_task_iter_start(&sti, sch->cgrp);
> +	while ((p = scx_task_iter_next_locked(&sti))) {
> +		if (p->scx.flags & SCX_TASK_SUB_INIT) {
> +			__scx_disable_and_exit_task(sch, p);
> +			p->scx.flags &= ~SCX_TASK_SUB_INIT;
> +		}
> +	}
> +	scx_task_iter_stop(&sti);

/* scx_cgroup_unlock() and percpu_up_write() seem missing here? */

>  out_put_cgrp:
>  	cgroup_put(cgrp);
>  out_unlock:
> 

abort: can be reached when assert_task_ready_or_enabled() fails or
__scx_init_task() returns an error during the init loop. If I'm reading
this correctly, leaving those locks unreleased would deadlock the next
caller of scx_fork_rwsem or scx_cgroup_lock() (e.g. any fork or future
scheduler load attempt).

Would the fix be to add before out_put_cgrp: :

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index fd6e2173cefe..25d16d0f45d0 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -6389,6 +6389,8 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
                }
        }
        scx_task_iter_stop(&sti);
+       scx_cgroup_unlock();
+       percpu_up_write(&scx_fork_rwsem);
 out_put_cgrp:
        cgroup_put(cgrp);
 out_unlock:

mirroring what err_unlock_and_disable: already does? Or am I missing
something that handles this on the abort path?

-- 
Thanks,
Cheng-Yang