Messages in this thread | | | Date | Mon, 12 Dec 2011 14:11:18 +0100 | From | Michal Hocko <> | Subject | Re: [PATCH] mm: memcg: keep root group unchanged if fail to create new |
| |
On Sun 11-12-11 15:39:43, Hugh Dickins wrote: > On Sun, 11 Dec 2011, Hillf Danton wrote: > > > If the request is not to create root group and we fail to meet it, > > we'd leave the root unchanged. > > I didn't understand that at first: please say "we should" rather > than "we'd", which I take to be an abbreviation for "we would". > > > > > Signed-off-by: Hillf Danton <dhillf@gmail.com> > > Yes indeed, well caught: > Acked-by: Hugh Dickins <hughd@google.com> > > I wonder what was going through the author's mind when he wrote it > that way? I wonder if it's one of those bugs that creeps in when > you start from a perfectly functional patch, then make refinements > to suit feedback from reviewers. > > On which topic: wouldn't this patch be better just to move the > "root_mem_cgroup = memcg;" two lines lower down (and of course > remove free_out's "root_mem_cgroup = NULL;" as you already did)?
Yes would look nicer.
> I can't see mem_cgroup_soft_limit_tree_init() relying on > root_mem_cgroup at all.
It doesn't but it still needs some love to handle error case properly AFAICS. We do not deallocate softlimit trees for nodes that succeeded.
[...]
Hilf could you update the patch please? -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic
| |