Re: [PATCH 3/3] cgroup: implement cgroup.subtree_populated for the default hierarchy

[Tejun Heo]

Hello,

On Thu, Apr 10, 2014 at 09:04:24AM -0500, Serge Hallyn wrote:
> Except for the keeping state.  If the userspace agent crashes when it
> was meant to drop 100 cgroups when they become empty, then when it
> restarts those 100 cgroups may never be freed.  Of course userspace
> can do things about this, but it just seems like it would be so
> trivial to handle this case in the kernel.  Like you say there is

But it's also trivial from userland.  You can write up a separate
backup agent or just make the main agent perform cleanup on restart,
which it should probably be doing anyway.

> no need for all the fancy agent spawning - just notice that the
> cgroup became empty, that releaseonempty was true, and so unlink the
> thing.  It'll be freed when anyone who has it held open closes it.

It'd be a superflous feature which require a separate control knob on
each cgroup and can lead to confusion too as there are now two
entities acting on it by default.  The manager has to worry about
keeping those knobs in sync and deal with cases where cgroups
disappear unexpectedly.  Think about it.  What should the default
value of the knob be?  What if the manager crashes before setting up
the knob to its desired state?  It needs to perform a cleanup pass
after crash *anyway*.  How is it gonna synchronize with the current
state of cgroup hierarchy otherwise?

It's adding complexity without any actual benefits.  This is kernel
API.  It should be concise and orthogonal where it can be.  There of
course are cases where convenience should be considered but what
you're suggesting doesn't seem beneficial in any substantial way.

> > The case where you move a task out of x1/y1 to another cgroup doesn't
> > generate an event.  One could say that that's unnecessary because the
> 
> Still confused.
> 
> If I create x1/y1 and x3/y3, set notify_on_release on x1 and x1/y1,
> move a task into x1/y1, then move it to x3/y3, then x1/y1 and x1 both
> do get removed.

Ah, you're right, cgroup_task_migrate() sets CGRP_RELEASABLE
explicitly.  I was confused because put_css_set_locked() sets
CGRP_RELEASABLE only if @taskexit is set.  Will drop that part from
the description.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/