On Fri 11-01-13 13:45:24, Glauber Costa wrote:
[...]
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 2ac2808..aa4e258 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4723,6 +4723,30 @@ static void mem_cgroup_reparent_charges(struct mem_cgroup *memcg)
> }
>
> /*
> + * must be called with memcg_mutex held, unless the cgroup is guaranteed to be
This one doesn't exist yet.
> + * already dead (like in mem_cgroup_force_empty, for instance). This is
> + * different than mem_cgroup_count_children, in the sense that we don't really
> + * care how many children we have, we only need to know if we have any. It is
> + * also count any memcg without hierarchy as infertile for that matter.
> + */
> +static inline bool memcg_has_children(struct mem_cgroup *memcg)
> +{
> + struct mem_cgroup *iter;
> +
> + if (!memcg->use_hierarchy)
> + return false;
> +
> + /* 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.
> + if ((iter == memcg) || !mem_cgroup_online(iter))
> + continue;
> + return true;
mem_cgroup_iter_break here if you _really_ insist on
for_each_mem_cgroup_tree.
I still think that the hammer is too big for what we need here.
> + }
> +
> + return false;
> +}
> +
> +/*
> * Reclaims as many pages from the given memcg as possible and moves
> * the rest to the parent.
> *
[...]
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>