Skip to content

Commit 79f919a

Browse files
Chen Ridonghtejun
authored andcommitted
cgroup: split cgroup_destroy_wq into 3 workqueues
A hung task can occur during [1] LTP cgroup testing when repeatedly mounting/unmounting perf_event and net_prio controllers with systemd.unified_cgroup_hierarchy=1. The hang manifests in cgroup_lock_and_drain_offline() during root destruction. Related case: cgroup_fj_function_perf_event cgroup_fj_function.sh perf_event cgroup_fj_function_net_prio cgroup_fj_function.sh net_prio Call Trace: cgroup_lock_and_drain_offline+0x14c/0x1e8 cgroup_destroy_root+0x3c/0x2c0 css_free_rwork_fn+0x248/0x338 process_one_work+0x16c/0x3b8 worker_thread+0x22c/0x3b0 kthread+0xec/0x100 ret_from_fork+0x10/0x20 Root Cause: CPU0 CPU1 mount perf_event umount net_prio cgroup1_get_tree cgroup_kill_sb rebind_subsystems // root destruction enqueues // cgroup_destroy_wq // kill all perf_event css // one perf_event css A is dying // css A offline enqueues cgroup_destroy_wq // root destruction will be executed first css_free_rwork_fn cgroup_destroy_root cgroup_lock_and_drain_offline // some perf descendants are dying // cgroup_destroy_wq max_active = 1 // waiting for css A to die Problem scenario: 1. CPU0 mounts perf_event (rebind_subsystems) 2. CPU1 unmounts net_prio (cgroup_kill_sb), queuing root destruction work 3. A dying perf_event CSS gets queued for offline after root destruction 4. Root destruction waits for offline completion, but offline work is blocked behind root destruction in cgroup_destroy_wq (max_active=1) Solution: Split cgroup_destroy_wq into three dedicated workqueues: cgroup_offline_wq – Handles CSS offline operations cgroup_release_wq – Manages resource release cgroup_free_wq – Performs final memory deallocation This separation eliminates blocking in the CSS free path while waiting for offline operations to complete. [1] https://github.com/linux-test-project/ltp/blob/master/runtest/controllers Fixes: 334c367 ("cgroup: reimplement rebind_subsystems() using cgroup_apply_control() and friends") Reported-by: Gao Yingjie <gaoyingjie@uniontech.com> Signed-off-by: Chen Ridong <chenridong@huawei.com> Suggested-by: Teju Heo <tj@kernel.org> Signed-off-by: Tejun Heo <tj@kernel.org>
1 parent 6563623 commit 79f919a

File tree

1 file changed

+36
-7
lines changed

1 file changed

+36
-7
lines changed

kernel/cgroup/cgroup.c

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,31 @@ DEFINE_PERCPU_RWSEM(cgroup_threadgroup_rwsem);
126126
* of concurrent destructions. Use a separate workqueue so that cgroup
127127
* destruction work items don't end up filling up max_active of system_wq
128128
* which may lead to deadlock.
129+
*
130+
* A cgroup destruction should enqueue work sequentially to:
131+
* cgroup_offline_wq: use for css offline work
132+
* cgroup_release_wq: use for css release work
133+
* cgroup_free_wq: use for free work
134+
*
135+
* Rationale for using separate workqueues:
136+
* The cgroup root free work may depend on completion of other css offline
137+
* operations. If all tasks were enqueued to a single workqueue, this could
138+
* create a deadlock scenario where:
139+
* - Free work waits for other css offline work to complete.
140+
* - But other css offline work is queued after free work in the same queue.
141+
*
142+
* Example deadlock scenario with single workqueue (cgroup_destroy_wq):
143+
* 1. umount net_prio
144+
* 2. net_prio root destruction enqueues work to cgroup_destroy_wq (CPUx)
145+
* 3. perf_event CSS A offline enqueues work to same cgroup_destroy_wq (CPUx)
146+
* 4. net_prio cgroup_destroy_root->cgroup_lock_and_drain_offline.
147+
* 5. net_prio root destruction blocks waiting for perf_event CSS A offline,
148+
* which can never complete as it's behind in the same queue and
149+
* workqueue's max_active is 1.
129150
*/
130-
static struct workqueue_struct *cgroup_destroy_wq;
151+
static struct workqueue_struct *cgroup_offline_wq;
152+
static struct workqueue_struct *cgroup_release_wq;
153+
static struct workqueue_struct *cgroup_free_wq;
131154

132155
/* generate an array of cgroup subsystem pointers */
133156
#define SUBSYS(_x) [_x ## _cgrp_id] = &_x ## _cgrp_subsys,
@@ -5558,7 +5581,7 @@ static void css_release_work_fn(struct work_struct *work)
55585581
cgroup_unlock();
55595582

55605583
INIT_RCU_WORK(&css->destroy_rwork, css_free_rwork_fn);
5561-
queue_rcu_work(cgroup_destroy_wq, &css->destroy_rwork);
5584+
queue_rcu_work(cgroup_free_wq, &css->destroy_rwork);
55625585
}
55635586

55645587
static void css_release(struct percpu_ref *ref)
@@ -5567,7 +5590,7 @@ static void css_release(struct percpu_ref *ref)
55675590
container_of(ref, struct cgroup_subsys_state, refcnt);
55685591

55695592
INIT_WORK(&css->destroy_work, css_release_work_fn);
5570-
queue_work(cgroup_destroy_wq, &css->destroy_work);
5593+
queue_work(cgroup_release_wq, &css->destroy_work);
55715594
}
55725595

55735596
static void init_and_link_css(struct cgroup_subsys_state *css,
@@ -5701,7 +5724,7 @@ static struct cgroup_subsys_state *css_create(struct cgroup *cgrp,
57015724
list_del_rcu(&css->sibling);
57025725
err_free_css:
57035726
INIT_RCU_WORK(&css->destroy_rwork, css_free_rwork_fn);
5704-
queue_rcu_work(cgroup_destroy_wq, &css->destroy_rwork);
5727+
queue_rcu_work(cgroup_free_wq, &css->destroy_rwork);
57055728
return ERR_PTR(err);
57065729
}
57075730

@@ -5939,7 +5962,7 @@ static void css_killed_ref_fn(struct percpu_ref *ref)
59395962

59405963
if (atomic_dec_and_test(&css->online_cnt)) {
59415964
INIT_WORK(&css->destroy_work, css_killed_work_fn);
5942-
queue_work(cgroup_destroy_wq, &css->destroy_work);
5965+
queue_work(cgroup_offline_wq, &css->destroy_work);
59435966
}
59445967
}
59455968

@@ -6325,8 +6348,14 @@ static int __init cgroup_wq_init(void)
63256348
* We would prefer to do this in cgroup_init() above, but that
63266349
* is called before init_workqueues(): so leave this until after.
63276350
*/
6328-
cgroup_destroy_wq = alloc_workqueue("cgroup_destroy", 0, 1);
6329-
BUG_ON(!cgroup_destroy_wq);
6351+
cgroup_offline_wq = alloc_workqueue("cgroup_offline", 0, 1);
6352+
BUG_ON(!cgroup_offline_wq);
6353+
6354+
cgroup_release_wq = alloc_workqueue("cgroup_release", 0, 1);
6355+
BUG_ON(!cgroup_release_wq);
6356+
6357+
cgroup_free_wq = alloc_workqueue("cgroup_free", 0, 1);
6358+
BUG_ON(!cgroup_free_wq);
63306359
return 0;
63316360
}
63326361
core_initcall(cgroup_wq_init);

0 commit comments

Comments
 (0)