On 12/03/2012 09:15 PM, Michal Hocko wrote:
> On Fri 30-11-12 17:31:26, Glauber Costa wrote:
> [...]
>> +/*
>> + * 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.
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...
--
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
