Re: [PATCH] sched_ext: Fix deadlock with PSI trigger creation
Andrea Righi <[email protected]>
| Newsgroups | dev.linux.lists.sched-ext,org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <alE46Lxq-gFyJcxZ@gpd4> |
On Fri, Jul 10, 2026 at 07:46:39PM +0200, Andrea Righi wrote: > Hi Matt, > > On Fri, Jul 10, 2026 at 11:04:41AM +0100, Matt Fleming wrote: > > From: Matt Fleming <[email protected]> > > > > scx_root_enable_workfn() currently takes scx_fork_rwsem for writing > > before acquiring cgroup_mutex. Since commit a5b98009f16d ("sched/psi: > > fix race between file release and pressure write"), pressure_write() > > holds cgroup_mutex across psi_trigger_create(), which may call > > kthread_create() for the psimon kthread. kthreadd's fork then enters > > scx_pre_fork() and waits for the read side of scx_fork_rwsem. > > > > This results in a deadlock. The enable worker holds scx_fork_rwsem and > > waits for cgroup_mutex, while the PSI writer holds cgroup_mutex and > > waits for psimon creation to complete. Any concurrent fork blocks on > > scx_pre_fork() behind the enable worker. > > > > The hung-task detector captured all three sides of the deadlock: > > > > scx_enable_help: > > __mutex_lock > > scx_enable_workfn > > kthread_worker_fn > > > > systemd: > > wait_for_completion_killable > > __kthread_create_on_node > > kthread_create_on_node > > psi_trigger_create > > pressure_write > > kernfs_fop_write_iter > > > > python3: > > percpu_rwsem_wait > > __percpu_down_read > > scx_pre_fork > > sched_fork > > copy_process > > kernel_clone > > > > It also identified systemd as the likely owner of the mutex on which > > scx_enable_help was blocked. > > > > We reproduced this on a 128-CPU AMD EPYC 7713 by enabling scx_lavd > > concurrently with writes to cgroup PSI trigger files. Unrelated tasks > > piled up in scx_pre_fork() and process creation on the box stopped. > > > > Fix the inversion by acquiring cgroup_mutex before scx_fork_rwsem in > > scx_root_enable_workfn() and releasing them in reverse order, while > > preserving the existing exclusion around cgroup and task initialisation. > > > > Fixes: a5b98009f16d ("sched/psi: fix race between file release and pressure write") > > Cc: [email protected] > > Signed-off-by: Matt Fleming <[email protected]> > > This seems to introduce the following (running the sched_ext kselftests): > > [ 28.963575] ====================================================== > [ 28.963670] WARNING: possible circular locking dependency detected > [ 28.963752] 7.1.0-virtme #1 Not tainted > [ 28.963804] ------------------------------------------------------ > [ 28.963887] sched_ext_helpe/2619 is trying to acquire lock: > [ 28.963954] ffffffff83685240 (scx_cgroup_ops_rwsem){+.+.}-{0:0}, at: scx_root_disable+0x45d/0x840 > [ 28.964071] > [ 28.964071] but task is already holding lock: > [ 28.964151] ffffffff83682910 (scx_fork_rwsem){++++}-{0:0}, at: scx_root_disable+0x160/0x840 > [ 28.964260] > [ 28.964260] which lock already depends on the new lock. Looking more at this, I think we should fix the PSI path instead, sched_ext locking order is consistently using: scx_fork_rwsem -> scx_cgroup_ops_rwsem -> cgroup_mutex Maybe we can rework pressure_write() to pin the cgroup and file context, then drop both cgroup_mutex and kernfs active protection before creating the PSI kthread? Thanks, -Andrea > [ 28.964260] > [ 28.964355] > [ 28.964355] the existing dependency chain (in reverse order) is: > [ 28.964455] > [ 28.964455] -> #2 (scx_fork_rwsem){++++}-{0:0}: > [ 28.964539] percpu_down_write+0x49/0x150 > [ 28.964610] scx_root_enable_workfn+0x5d0/0xd30 > [ 28.964679] kthread_worker_fn+0x121/0x360 > [ 28.964749] kthread+0x10c/0x140 > [ 28.964802] ret_from_fork+0x189/0x330 > [ 28.964872] ret_from_fork_asm+0x1a/0x30 > [ 28.964941] > [ 28.964941] -> #1 (cgroup_mutex){+.+.}-{4:4}: > [ 28.965025] __mutex_lock+0xbe/0xd80 > [ 28.965070] scx_root_enable_workfn+0x5c4/0xd30 > [ 28.965138] kthread_worker_fn+0x121/0x360 > [ 28.965210] kthread+0x10c/0x140 > [ 28.965263] ret_from_fork+0x189/0x330 > [ 28.965331] ret_from_fork_asm+0x1a/0x30 > [ 28.965402] > [ 28.965402] -> #0 (scx_cgroup_ops_rwsem){+.+.}-{0:0}: > [ 28.965485] __lock_acquire+0x14c5/0x2a40 > [ 28.965554] lock_acquire+0xd3/0x280 > [ 28.965608] percpu_down_write+0x49/0x150 > [ 28.965677] scx_root_disable+0x45d/0x840 > [ 28.965745] kthread_worker_fn+0x121/0x360 > [ 28.965815] kthread+0x10c/0x140 > [ 28.965867] ret_from_fork+0x189/0x330 > [ 28.965935] ret_from_fork_asm+0x1a/0x30 > [ 28.966002] > [ 28.966002] other info that might help us debug this: > [ 28.966002] > [ 28.966097] Chain exists of: > [ 28.966097] scx_cgroup_ops_rwsem --> cgroup_mutex --> scx_fork_rwsem > [ 28.966097] > [ 28.966233] Possible unsafe locking scenario: > [ 28.966233] > [ 28.966314] CPU0 CPU1 > [ 28.966379] ---- ---- > [ 28.966444] lock(scx_fork_rwsem); > [ 28.966496] lock(cgroup_mutex); > [ 28.966579] lock(scx_fork_rwsem); > [ 28.966661] lock(scx_cgroup_ops_rwsem); > [ 28.966713] > [ 28.966713] *** DEADLOCK *** > [ 28.966713] > [ 28.966794] 2 locks held by sched_ext_helpe/2619: > [ 28.966861] #0: ffffffff83682818 (scx_enable_mutex){+.+.}-{4:4}, at: scx_root_disable+0xba/0x840 > [ 28.966974] #1: ffffffff83682910 (scx_fork_rwsem){++++}-{0:0}, at: scx_root_disable+0x160/0x840 > [ 28.967090] > [ 28.967090] stack backtrace: > [ 28.967158] CPU: 8 UID: 0 PID: 2619 Comm: sched_ext_helpe Not tainted 7.1.0-virtme #1 PREEMPT(full) > [ 28.967164] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 > [ 28.967166] Call Trace: > [ 28.967169] <TASK> > [ 28.967172] dump_stack_lvl+0x6d/0xa0 > [ 28.967175] print_circular_bug+0x2e1/0x300 > [ 28.967179] check_noncircular+0x144/0x170 > [ 28.967182] __lock_acquire+0x14c5/0x2a40 > [ 28.967202] ? scx_root_disable+0x45d/0x840 > [ 28.967204] lock_acquire+0xd3/0x280 > [ 28.967207] ? scx_root_disable+0x45d/0x840 > [ 28.967209] percpu_down_write+0x49/0x150 > [ 28.967212] ? scx_root_disable+0x45d/0x840 > [ 28.967213] scx_root_disable+0x45d/0x840 > [ 28.967219] ? kthread_worker_fn+0x51/0x360 > [ 28.967221] kthread_worker_fn+0x121/0x360 > [ 28.967223] ? __pfx_scx_disable_workfn+0x10/0x10 > [ 28.967224] ? __pfx_kthread_worker_fn+0x10/0x10 > [ 28.967227] kthread+0x10c/0x140 > [ 28.967228] ? __pfx_kthread+0x10/0x10 > [ 28.967229] ret_from_fork+0x189/0x330 > [ 28.967231] ? __pfx_kthread+0x10/0x10 > [ 28.967232] ret_from_fork_asm+0x1a/0x30 > [ 28.967235] </TASK> > > Thanks, > -Andrea > > > --- > > kernel/sched/ext/ext.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c > > index 691d53fe0f64..ba89eafe7964 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); > > > > WARN_ON_ONCE(scx_init_task_enabled); > > @@ -7216,7 +7219,6 @@ static void scx_root_enable_workfn(struct kthread_work *work) > > * while tasks are being initialized so that scx_cgroup_can_attach() > > * never sees uninitialized tasks. > > */ > > - scx_cgroup_lock(); > > set_cgroup_sched(sch_cgroup(sch), sch); > > ret = scx_cgroup_init(sch); > > if (ret) > > @@ -7283,8 +7285,8 @@ static void scx_root_enable_workfn(struct kthread_work *work) > > put_task_struct(p); > > } > > scx_task_iter_stop(&sti); > > - scx_cgroup_unlock(); > > percpu_up_write(&scx_fork_rwsem); > > + scx_cgroup_unlock(); > > > > /* > > * All tasks are READY. It's safe to turn on scx_enabled() and switch > > @@ -7369,8 +7371,8 @@ static void scx_root_enable_workfn(struct kthread_work *work) > > return; > > > > err_disable_unlock_all: > > - scx_cgroup_unlock(); > > percpu_up_write(&scx_fork_rwsem); > > + scx_cgroup_unlock(); > > /* we'll soon enter disable path, keep bypass on */ > > err_disable: > > mutex_unlock(&scx_enable_mutex); > > -- > > 2.43.0 > >