Skip to content

Commit

Permalink
scx: Reorder scx_fork_rwsem, cpu_hotplug_lock and scx_cgroup_rwsem
Browse files Browse the repository at this point in the history
scx_cgroup_rwsem and scx_fork_rwsem, respectively, are in the following
locking dependency chain.

  cpu_hotplug_lock --> cgroup_threadgroup_rwsem --> scx_cgroup_rwsem
  scx_fork_rwsem --> pernet_ops_rwsem --> cpu_hotplug_lock

And we need to flip static_key which requires CPUs stable. The only locking
order which satifies all three requirements is

  scx_fork_rwsem --> cpu_hotplug_lock --> scx_cgroup_rwsem

Reorder locking in scx_ops_enable() and scx_ops_disable_workfn().

Signed-off-by: Tejun Heo <tj@kernel.org>
  • Loading branch information
htejun committed Jan 10, 2024
1 parent 4bbb07c commit 1225a90
Showing 1 changed file with 22 additions and 5 deletions.
27 changes: 22 additions & 5 deletions kernel/sched/ext.c
Original file line number Diff line number Diff line change
Expand Up @@ -3201,9 +3201,12 @@ static void scx_ops_disable_workfn(struct kthread_work *work)
static_branch_disable(&__scx_switched_all);
WRITE_ONCE(scx_switching_all, false);

/* avoid racing against fork and cgroup changes */
cpus_read_lock();
/*
* Avoid racing against fork and cgroup changes. See scx_ops_enable()
* for explanation on the locking order.
*/
percpu_down_write(&scx_fork_rwsem);
cpus_read_lock();
scx_cgroup_lock();

spin_lock_irq(&scx_tasks_lock);
Expand Down Expand Up @@ -3244,8 +3247,8 @@ static void scx_ops_disable_workfn(struct kthread_work *work)
scx_cgroup_exit();

scx_cgroup_unlock();
percpu_up_write(&scx_fork_rwsem);
cpus_read_unlock();
percpu_up_write(&scx_fork_rwsem);

if (ei->kind >= SCX_EXIT_ERROR) {
printk(KERN_ERR "sched_ext: BPF scheduler \"%s\" errored, disabling\n", scx_ops.name);
Expand Down Expand Up @@ -3460,9 +3463,23 @@ static int scx_ops_enable(struct sched_ext_ops *ops)
/*
* Lock out forks, cgroup on/offlining and moves before opening the
* floodgate so that they don't wander into the operations prematurely.
*
* We don't need to keep the CPUs stable but static_branch_*() requires
* cpus_read_lock() and scx_cgroup_rwsem must nest inside
* cpu_hotplug_lock because of the following dependency chain:
*
* cpu_hotplug_lock --> cgroup_threadgroup_rwsem --> scx_cgroup_rwsem
*
* So, we need to do cpus_read_lock() before scx_cgroup_lock() and use
* static_branch_*_cpuslocked().
*
* Note that cpu_hotplug_lock must nest inside scx_fork_rwsem due to the
* following dependency chain:
*
* scx_fork_rwsem --> pernet_ops_rwsem --> cpu_hotplug_lock
*/
cpus_read_lock();
percpu_down_write(&scx_fork_rwsem);
cpus_read_lock();
scx_cgroup_lock();

for (i = SCX_OPI_NORMAL_BEGIN; i < SCX_OPI_NORMAL_END; i++)
Expand Down Expand Up @@ -3575,8 +3592,8 @@ static int scx_ops_enable(struct sched_ext_ops *ops)
spin_unlock_irq(&scx_tasks_lock);
preempt_enable();
scx_cgroup_unlock();
percpu_up_write(&scx_fork_rwsem);
cpus_read_unlock();
percpu_up_write(&scx_fork_rwsem);

if (!scx_ops_tryset_enable_state(SCX_OPS_ENABLED, SCX_OPS_ENABLING)) {
ret = -EBUSY;
Expand Down

0 comments on commit 1225a90

Please sign in to comment.