On 12/04/2012 12:23 PM, Michal Hocko wrote:

On Tue 04-12-12 11:58:48, Glauber Costa wrote:

quoted

On 12/03/2012 09:15 PM, Michal Hocko wrote:

quoted

On Fri 30-11-12 17:31:26, Glauber Costa wrote:
[...]

quoted

+/*
+ * must be called with memcg_lock held, unless the cgroup is guaranteed to be
+ * already dead (like in mem_cgroup_force_empty, for instance).
+ */
+static inline bool memcg_has_children(struct mem_cgroup *memcg)
+{
+	return mem_cgroup_count_children(memcg) != 1;
+}

Why not just keep list_empty(&cgrp->children) which is much simpler much
more effective and correct here as well because cgroup cannot vanish
while we are at the call because all callers come from cgroup fs?

Because it depends on cgroup's internal representation, which I think
we're better off not depending upon, even if this is not as serious a
case as the locking stuff. But also, technically, cgrp->children is
protected by the cgroup_lock(), while since we'll hold the memcg_lock
during creation and also around the iterators, we cover everything with
the same lock.

The list is RCU safe so we do not have to use cgroup_lock there for this
kind of test.

quoted

That said, of course we don't need to do the full iteration here, and
mem_cgroup_count_children is indeed overkill. We could just as easily
verify if any child exist - it is just an emptiness test after all. But
it is not living in any fast path, though, and I just assumed code reuse
to win over efficiency in this particular case -
mem_cgroup_count_children already existed...

Yes but the function name suggests a more generic usage and the test is
really an overkill. Maybe we can get a cgroup generic helper
cgroup_as_children which would do the thing without exhibiting cgroup
internals. What do you think?

I will give it another round of thinking, but I still don't see the
reason for calling to cgroup core with this. 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;

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