On Mon 29-12-14 18:54:43, Vladimir Davydov wrote:
> We are supposed to take one css reference per each memory page and per
> each swap entry accounted to a memory cgroup. However, during task
> charges migration we take a reference to the destination cgroup twice
> per each swap entry: first in mem_cgroup_do_precharge()->try_charge()
> and then in mem_cgroup_move_swap_account(), permanently leaking the
> destination cgroup.
Very well spotted!
> The hunk taking the second reference seems to be a leftover from the
> pre-00501b531c472 ("mm: memcontrol: rewrite charge API") era. Remove it
> to fix the leak.
This seems to be a fallout from e8ea14cc6ead (mm: memcontrol: take a
css reference for each charged page) because we only took per-charge
reference for swapped out pages before. In order to keep the balance
correct we had to do that ugly css_get() in mem_cgroup_move_swap_account
and uncharge the origin later on in __mem_cgroup_clear_mc.
The uncharge part for the from memcg should be OK because we do so from
the page counter directly and that doesn't involve reference counting
and then we do css_put_many explicitly.
So unless I have missed something the culrpit is different and so it
doesn't have to go to stable just should appear in a later 3.19 rc.
Fixes: e8ea14cc6ead (mm: memcontrol: take a css reference for each charged page)
> Signed-off-by: Vladimir Davydov <vdavy...@parallels.com>
Acked-by: Michal Hocko <mho...@suse.cz>
> ---
> mm/memcontrol.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ef91e856c7e4..d62c335dfef4 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3043,18 +3043,6 @@ static int mem_cgroup_move_swap_account(swp_entry_t
> entry,
> if (swap_cgroup_cmpxchg(entry, old_id, new_id) == old_id) {
> mem_cgroup_swap_statistics(from, false);
> mem_cgroup_swap_statistics(to, true);
> - /*
> - * This function is only called from task migration context now.
> - * It postpones page_counter and refcount handling till the end
> - * of task migration(mem_cgroup_clear_mc()) for performance
> - * improvement. But we cannot postpone css_get(to) because if
> - * the process that has been moved to @to does swap-in, the
> - * refcount of @to might be decreased to 0.
> - *
> - * We are in attach() phase, so the cgroup is guaranteed to be
> - * alive, so we can just call css_get().
> - */
> - css_get(&to->css);
> return 0;
> }
> return -EINVAL;
> --
> 1.7.10.4
>
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
