On Tue 26-06-12 20:37:16, Glauber Costa wrote:

On 06/26/2012 08:15 PM, Michal Hocko wrote:

quoted

On Tue 26-06-12 19:47:14, Glauber Costa wrote:

quoted

Okay, so after recent discussions, I am proposing the following
patch. It won't remove hierarchy, or anything like that. Just default
to true in the root cgroup, and print a warning once if you try
to set it back to 0.

I am not adding it to feature-removal-schedule.txt because I don't
view it as a consensus. Rather, changing the default would allow us
to give it a time around in the open, and see if people complain
and what we can learn about that.

Signed-off-by: Glauber Costa <redacted>
Acked-by: Johannes Weiner <redacted>
CC: Michal Hocko <redacted>
CC: Kamezawa Hiroyuki <redacted>
CC: Tejun Heo <redacted>
---
 mm/memcontrol.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 85f7790..c37e4c1 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c

@@ -3993,6 +3993,10 @@ static int mem_cgroup_hierarchy_write(struct cgroup *cont, struct cftype *cft,
 	if (memcg->use_hierarchy == val)
 		goto out;

+	WARN_ONCE(!parent_memcg && memcg->use_hierarchy,

Do you have to test anything here at all? The test above will get you
out without doing anything if you are not trying to change anything.
The default is true so you have to be trying to disable it.

If you omit !parent_memcg test as well you will get a bonus of the early
warning even if somebody has cgconfig.conf like this:

group a/b/c {
	memory {
		memory.use_hierarchy = 0;
		[...]
	}
}

which worked previously...
True there is a risk of a "false warning" when somebody just tries to
set disable hierarchy when it is (and never was) allowed but I do not
think this is that bad.

Well, a false warning is not that bad.
It is better to be vocal.

I will wait for Kame to put his comments, and I can resend with that change.

OK, you can resend with
Acked-by: Michal Hocko <redacted>

And thanks!
I will try to push this into OpenSUSE kernels to have a wider audience
for testing. I think that the approach is safe because there is a safe
fallback I mentioned earlier (maybe you can put it into the changelog)
and if there really exists a setting which cannot be converted to
non-hierarchical layout then we should better know as soon as possible.
-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic