From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752758AbcKZU4P (ORCPT ); Sat, 26 Nov 2016 15:56:15 -0500 Received: from mail-pg0-f52.google.com ([74.125.83.52]:35269 "EHLO mail-pg0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752074AbcKZU4E (ORCPT ); Sat, 26 Nov 2016 15:56:04 -0500 Date: Sat, 26 Nov 2016 12:55:55 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Jiri Slaby cc: stable@vger.kernel.org, linux-kernel@vger.kernel.org, Hugh Dickins , Tejun Heo Subject: Re: [PATCH 3.12 118/127] cgroup: use an ordered workqueue for cgroup destruction In-Reply-To: <58563cf6ff3ad14d9f08cd7ccb0656e7fe3872ef.1480062521.git.jslaby@suse.cz> Message-ID: References: <58563cf6ff3ad14d9f08cd7ccb0656e7fe3872ef.1480062521.git.jslaby@suse.cz> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 25 Nov 2016, Jiri Slaby wrote: > From: Hugh Dickins > > 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 > Signed-off-by: Hugh Dickins > Signed-off-by: Tejun Heo > Signed-off-by: Jiri Slaby 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 > --- > 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