On Fri, 25 Nov 2016, Jiri Slaby wrote:
> From: Hugh Dickins <hu...@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 <filbran...@google.com>
> Signed-off-by: Hugh Dickins <hu...@google.com>
> Signed-off-by: Tejun Heo <t...@kernel.org>
> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
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
