Re: [PATCH] cgroup: use an ordered workqueue for cgroup destruction

Tejun Heo
2014-02-07T14:04:10Z

Hello, Hugh.

On Thu, Feb 06, 2014 at 03:56:01PM -0800, Hugh Dickins wrote:
> 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.

Hmmm... I'm not really comfortable with this. This would seal shut
any possiblity of increasing concurrency in that path, which is okay
now but I find the combination of such long term commitment and the
non-obviousness (it's not apparent from looking at memcg code why it
wouldn't deadlock) very unappealing. Besides, the only reason
offline() is currently called under cgroup_mutex is history. We can
move it out of cgroup_mutex right now.

But even with offline being called outside cgroup_mutex, IIRC, the
described problem would still be able to deadlock as long as the tree
depth is deeper than max concurrency level of the destruction
workqueue. Sure, we can give it large enough number but it's
generally nasty.

One thing I don't get is why memcg has such reverse dependency at all.
Why does the parent wait for its descendants to do something during
offline? Shouldn't it be able to just bail and let whatever
descendant which is stil busy propagate things upwards? That's a
usual pattern we use to tree shutdowns anyway. Would that be nasty to
implement in memcg?

Thanks.

--
tejun