On 08/16, Oleg Nesterov wrote:
>
> On 08/16, Oleg Nesterov wrote:
> >
> > On 08/15, Brenden Blanco wrote:
> > >
> > > Hi folks,
> > >
> > > I think I have come across a memory leak in uprobes, which is fairly easy
> > > to
> > > reproduce.
> >
> > At first glance this looks as a problem in memcg, add CC's...
> >
> > put_page(old_page) looks properly balanced, and I assume we do not need
> > the additional "uncharge", we can rely on __page_cache_release().
> >
> > And I do not see any leak if I try to reproduce with CONFIG_MEMCG=n.
>
> Heh. it seems that mem_cgroup_*() logic was always wrong in __replace_page().
Yes, it seems this was broken by 00501b53 "mm: memcontrol: rewrite charge API".
> Could you try the patch below?
Please see v2 below. We don't need "cancel_charge" under "unlock:" at all.
Johannes, could you review?
Oleg.
---
--- x/kernel/events/uprobes.c
+++ x/kernel/events/uprobes.c
@@ -172,8 +172,10 @@ static int __replace_page(struct vm_area
mmu_notifier_invalidate_range_start(mm, mmun_start, mmun_end);
err = -EAGAIN;
ptep = page_check_address(page, mm, addr, &ptl, 0);
- if (!ptep)
+ if (!ptep) {
+ mem_cgroup_cancel_charge(kpage, memcg, false);
goto unlock;
+ }
get_page(kpage);
page_add_new_anon_rmap(kpage, vma, addr, false);
@@ -200,7 +202,6 @@ static int __replace_page(struct vm_area
err = 0;
unlock:
- mem_cgroup_cancel_charge(kpage, memcg, false);
mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end);
unlock_page(page);
return err;
