Hello, Michal.

On Mon, Oct 22, 2012 at 12:30:21PM +0200, Michal Hocko wrote:

quoted

quoted

We can still fail inn #3 without this patch becasuse there are is no
guarantee that a new task is attached to the group. And I wanted to keep
memcg and generic cgroup parts separated.

Yes, but all other controllers are broken that way too

It's just hugetlb and memcg that have pre_destroy.

quoted

and the worst thing which will hapen is triggering WARN_ON_ONCE().

The patch does BUG_ON(ss->pre_destroy(cgrp)). I am not sure WARN_ON_ONCE is
appropriate here because we would like to have it at least per
controller warning. I do not see any reason why to make this more
complicated but I am open to suggestions.

Once it's dropped from memcg, the next patch can update cgroup core
accordingly and the bug will exist for a single commit and the failure
mode would be triggering of WARN_ON_ONCE().  Seems pretty simple to
me.

quoted

Let's note the failure in the commit and remove
DEPREDATED_clear_css_refs in the previous patch.  Then, I can pull
from you, clean up pre_destroy mess and then you can pull back for
further cleanups.

Well this will get complicated as there are dependencies between memcg
parts (based on Andrew's tree) and your tree. My tree is not pullable as
all the patches go via Andrew. I am not sure how to get out of this.
There is only one cgroup patch so what about pushing all of this via
Andrew and do the follow up cleanups once they get merged? We are not in
hurry, are we?

Let's create a cgroup branch and build things there.  I don't think
cgroup changes are gonna be a single patch and expect to see at least
some bug fixes afterwards and don't wanna keep them floating separate
from other cgroup changes.  mm being based on top of -next, that
should work, right?

Anyway does it really make sense to drop DEPREDATED_clear_css_refs
already in the previous patch when it is _not_ guaranteed that
pre_destroy succeeds?

It makes things simpler here by decoupling memcg change with core
cgroup changes and the introduced bug isn't too easy to trigger and
even when triggered the failure mode isn't critical.  It's not gonna
break normal common operations or bisection.  As long as the issue is
clearly documented, I think it should be fine.  Just note that this
opens up a race window from deficient cgroup API and the following
commits will address it.

Thanks.

--
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>