Re: [PATCH 3.12 118/127] cgroup: use an ordered workqueue for cgroup destruction
From: Hugh Dickins <hughd@google.com>
Date: 2016-11-26 20:56:15
Also in:
lkml
On Fri, 25 Nov 2016, Jiri Slaby wrote:
From: Hugh Dickins <hughd@google.com>
3.12-stable review patch. If anyone has any objections, please let me know.
===============
commit ab3f5faa6255a0eb4f832675507d9e295ca7e9ba upstream.
Sometimes the cleanup after memcg hierarchy testing gets stuck in
mem_cgroup_reparent_charges(), unable to bring non-kmem usage down to 0.
There may turn out to be several causes, but a major cause is this: the
workitem to offline parent can get run before workitem to offline child;
parent's mem_cgroup_reparent_charges() circles around waiting for the
child's pages to be reparented to its lrus, but it's holding cgroup_mutex
which prevents the child from reaching its mem_cgroup_reparent_charges().
Just use an ordered workqueue for cgroup_destroy_wq.
tj: Committing as the temporary fix until the reverse dependency can
be removed from memcg. Comment updated accordingly.
Fixes: e5fca243abae ("cgroup: use a dedicated workqueue for cgroup destruction")
Suggested-by: Filipe Brandenburger <redacted>
Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Jiri Slaby <redacted>No, please drop this one. It was indeed marked for stable at the time, but then reverted by 1a11533fbd71792e8c5d36f6763fbce8df0d231d; and you already have in 3.12-stable the commit which in the end we used to fix the issue, 4fb1a86fb5e4209a7d4426d4e586c58e9edc74ac "memcg: reparent charges of children before processing parent". My fault for inadequate testing originally, sorry for the confusion, Hugh
quoted hunk ↗ jump to hunk
--- kernel/cgroup.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 5d9d542c0bb5..e89f6cec01c9 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c@@ -5168,12 +5168,16 @@ static int __init cgroup_wq_init(void) /* * There isn't much point in executing destruction path in * parallel. Good chunk is serialized with cgroup_mutex anyway. - * Use 1 for @max_active. + * + * XXX: Must be ordered to make sure parent is offlined after + * children. The ordering requirement is for memcg where a + * parent's offline may wait for a child's leading to deadlock. In + * the long term, this should be fixed from memcg side. * * We would prefer to do this in cgroup_init() above, but that * is called before init_workqueues(): so leave this until after. */ - cgroup_destroy_wq = alloc_workqueue("cgroup_destroy", 0, 1); + cgroup_destroy_wq = alloc_ordered_workqueue("cgroup_destroy", 0); BUG_ON(!cgroup_destroy_wq); return 0; }-- 2.10.2