On Tue, Jun 26, 2012 at 05:30:28PM +0400, Glauber Costa wrote: > 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 <glommer@xxxxxxxxxxxxx> > CC: Michal Hocko <mhocko@xxxxxxx> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> > CC: Johannes Weiner <hannes@xxxxxxxxxxx> > CC: Tejun Heo <tj@xxxxxxxxxx> > --- > mm/memcontrol.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 9e710bc..037ddd4 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -3949,6 +3949,8 @@ 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 && val == false), > + "Non-hierarchical memcg is considered for deprecation"); Agreed, this is a much better first step than the global switch. Nitpicks: Should the warning be emitted for any memcg, not just the parent? If somebody takes notice of the changed semantics, it's better to print the warning on the first try to disable hierarchies instead of holding back until they walk up the tree and try to change it in the root. Still forbid disabling at lower levels, just be more eager to inform the people trying it. The memcg->use_hierarchy check should not be needed as you make sure it's different from val, so checking val == false should suffice? Also, why the extra parens around the condition? I find the warning message a bit terse. Maybe include something like "restructure the cgroup directory structure to match your accounting requirements or complain to (linux-mm, cgroups list etc.) if not possible" > @@ -5175,6 +5177,7 @@ mem_cgroup_create(struct cgroup *cont) > INIT_WORK(&stock->work, drain_local_stock); > } > hotcpu_notifier(memcg_cpu_hotplug_callback, 0); > + memcg->use_hierarchy = true; Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx> Let's try this. We have crappy semantics all over the place and no evidence, only fear, that someone may rely on them. Pushing and watching for fallout seems like the most sensible solution to get us anywhere at this point. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>