Re: [PATCH v2 4/7] memcg: fast hierarchy-aware child test.

[Michal Hocko <mhocko@suse.cz>]

On Mon 21-01-13 11:58:49, Glauber Costa wrote:
> On 01/18/2013 08:06 PM, Michal Hocko wrote:
> >> +	/* bounce at first found */
> >> > +	for_each_mem_cgroup_tree(iter, memcg) {
> > This will not work. Consider you will see a !online memcg. What happens?
> > mem_cgroup_iter will css_get group that it returns and css_put it when
> > it visits another one or finishes the loop. So your poor iter will be
> > released before it gets born. Not good.
> > 
> Reading this again, I don't really follow. The iterator is not supposed
> to put() anything it hasn't get()'d before, so we will never release the
> group. Note that if it ever appears in here, the css refcnt is expected
> to be at least 1 already.
> 
> The online test relies on the memcg refcnt, not on the css refcnt.

Bahh, yeah, sorry about the confusion. Damn, it's not the first time I
managed to mix those two...

> Actually, now that the value setting is all done in css_online, the css
> refcnt should be enough to denote if the cgroup already has children,
> without a memcg-specific test. The css refcnt is bumped somewhere
> between alloc and online. 

Yes, in init_cgroup_css.

> Unless Tejun objects it, I think I will just get rid of the online
> test, and rely on the fact that if the iterator sees any children, we
> should already online.

Which means that we are back to list_empty(&cgrp->children) test, aren't
we. We just call it a different name. 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;

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).
-- 
Michal Hocko
SUSE Labs

--
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>