Re: [PATCH] sched_ext: Fix deadlock with PSI trigger creation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fixes an ABBA deadlock between `cgroup_mutex` and `scx_fork_rwsem` in `scx_root_enable_workfn()`, but leaves the exact same lock inversion unpatched in `scx_root_disable()`, `scx_sub_disable()`, and `scx_sub_enable_workfn()`.
--

--- Patch [1]: [PATCH] sched_ext: Fix deadlock with PSI trigger creation ---
commit aacffeaa1e10b05888e4970e698ee8d1863dbbbd
Author: Matt Fleming <[email protected]>

sched_ext: Fix deadlock with PSI trigger creation

This commit fixes an ABBA deadlock between cgroup_mutex and scx_fork_rwsem
in scx_root_enable_workfn(). It prevents deadlocks that can occur when
concurrent writes to cgroup PSI trigger files attempt to create kthreads.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 691d53fe0f648..ba89eafe79643 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -7193,7 +7193,10 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  	/*
>  	 * Lock out forks, cgroup on/offlining and moves before opening the
>  	 * floodgate so that they don't wander into the operations prematurely.
> +	 * cgroup_mutex must nest outside scx_fork_rwsem because cgroup file
> +	 * operations may create kthreads while holding cgroup_mutex.
>  	 */
> +	scx_cgroup_lock();
>  	percpu_down_write(&scx_fork_rwsem);

[Severity: High]
This is a pre-existing issue, but does this same lock inversion still exist
in other scheduler functions?

For example, scx_sub_disable() takes scx_fork_rwsem before scx_cgroup_lock():

kernel/sched/ext/ext.c:scx_sub_disable() {
    ...
    percpu_down_write(&scx_fork_rwsem);
    scx_cgroup_lock();
    ...
}

scx_root_disable() also takes the locks in the old order:

kernel/sched/ext/ext.c:scx_root_disable() {
    ...
    percpu_down_write(&scx_fork_rwsem);
    ...
    scx_cgroup_lock();
    ...
}

And scx_sub_enable_workfn() appears to do the same:

kernel/sched/ext/ext.c:scx_sub_enable_workfn() {
    ...
    percpu_down_write(&scx_fork_rwsem);
    scx_cgroup_lock();
    ...
}

Could a concurrent PSI trigger write still cause a deadlock if it races
with these other paths?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1