Re: [PATCH 4/4] memcg: replace cgroup_lock with memcg specific memcg_lock

[Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

Hello, Michal, Glauber.

On Tue, Dec 04, 2012 at 09:45:44AM +0100, Michal Hocko wrote:
> Because such a helper might be useful in general? I didn't check if
> somebody does the same test elsewhere though.

The problem is that whether a cgroup has a child or not may differ
depending on the specific controller.  You can't tell whether
something exists or not at a given time without synchronization and
synchronization is per-controller.  IOW, if a controller cares about
when a cgroup comes online and goes offline, it should synchronize
those events in ->css_on/offline() and only consider cgroups marked
online as online.

> > If you really dislike doing a children count (I don't like as well, I
> > just don't dislike), maybe we can do something like:
> > 
> > i = 0;
> > for_each_mem_cgroup_tree(iter, memcg) {
> > 	if (i++ == 1)
> > 		return false;
> > }
> > return true;
> 
> I guess you meant:
> i = 0;
> for_each_mem_cgroup_tree(iter, memcg) {
> 	if (i++ == 1) {
> 		mem_cgroup_iter_break(iter);
> 		break;
> 	}
> }
> return i > 1;

Or sth like the following?

bool memcg_has_children(cgrp)
{
	lockdep_assert_held(memcg_lock);

	rcu_read_lock();
	cgroup_for_each_children(pos, cgrp) {
		if (memcg_is_online(pos)) {
			rcu_read_unlock();
			return true;
		}
	}
	rcu_read_unlock();
	return ret;
}

-- 
tejun