On 01/21/2013 01:15 PM, Michal Hocko wrote: > On Mon 21-01-13 12:41:24, Glauber Costa wrote: >> On 01/21/2013 12:34 PM, Michal Hocko wrote: > [...] >>> If you really insist on not using >>> children directly then do something like: >>> struct cgroup *pos; >>> >>> if (!memcg->use_hierarchy) >>> cgroup_for_each_child(pos, memcg->css.cgroup) >>> return true; >>> >>> return false; >>> >> I don't oppose that. > > OK, I guess I could live with that ;) > >>> This still has an issue that a change (e.g. vm_swappiness) that requires >>> this check will fail even though the child creation fails after it is >>> made visible (e.g. during css_online). >>> >> Is it a problem ? > > I thought you were considering this a problem. Quoting from patch 3/7 > " >> This calls for troubles and I do not think the win you get is really >> worth it. All it gives you is basically that you can change an >> inheritable attribute while your child is between css_alloc and >> css_online and so your attribute change doesn't fail if the child >> creation fails between those two. Is this the case you want to >> handle? Does it really even matter? > > I think it matters a lot. Aside from the before vs after discussion to > which I've already conceded, without this protection we can't guarantee > that we won't end up with an inconsistent value of the tunables between > parent and child. > " > > Just to make it clear. I do not see this failure as a big problem. We > can fix it by adding an Online check later if somebody complains. > I am talking here about groups that appear between the alloc and online callbacks. You mentioned child creation failures. Those are very different scenarios. I am personally not a lot bothered if we deny a value change during child creation due to that child, and the allocation turns out to fail. IOW: denying a value change because we falsely believe there is a child is a lot less serious than allowing a value change due to our ignorance of child existence and ending up with an inconsistent value set. -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
