On Mon 03-12-12 18:15:32, Michal Hocko wrote:
[...]
> > @@ -3915,7 +3926,7 @@ static int mem_cgroup_hierarchy_write(struct cgroup *cont, struct cftype *cft,
> > */
> > if ((!parent_memcg || !parent_memcg->use_hierarchy) &&
> > (val == 1 || val == 0)) {
> > - if (list_empty(&cont->children))
> > + if (!memcg_has_children(memcg))
> > memcg->use_hierarchy = val;
> > else
> > retval = -EBUSY;
>
> Nothing prevents from a race when a task is on the way to be attached to
> the group. This means that we might miss some charges up the way to the
> parent.
>
> mem_cgroup_hierarchy_write
> cgroup_attach_task
> ss->can_attach() = mem_cgroup_can_attach
> mutex_lock(&memcg_lock)
> memcg->attach_in_progress++
> mutex_unlock(&memcg_lock)
> __mem_cgroup_can_attach
> mem_cgroup_precharge_mc (*)
> mutex_lock(memcg_lock)
> memcg_has_children(memcg)==false
Dohh, retard alert. I obviously mixed tasks and children cgroups here.
Why I thought we also do check for no tasks in the group? Ahh, because
we should, at least here otherwise parent could see more uncharges than
charges.
But that deserves a separate patch. Sorry, for the confusion.
> cgroup_task_migrate
> memcg->use_hierarchy = val;
> ss->attach()
>
> (*) All the charches here are not propagated upwards.
>
> Fixable simply by testing attach_in_progress as well. The same applies
> to all other cases so it would be much better to prepare a common helper
> which does the whole magic.
>
> [...]
>
> Thanks
> --
> Michal Hocko
> SUSE Labs
> --
> 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
--
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>