From: Oleg Nesterov <oleg@redhat.com>
To: Brenden Blanco <bblanco@plumgrid.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Michal Hocko <mhocko@kernel.org>,
Vladimir Davydov <vdavydov@virtuozzo.com>
Cc: linux-kernel@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>
Subject: Re: uprobes: memory leak in enable/disable loop
Date: Tue, 16 Aug 2016 16:34:08 +0200 [thread overview]
Message-ID: <20160816143407.GC17006@redhat.com> (raw)
In-Reply-To: <20160816142511.GB17006@redhat.com>
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;
next prev parent reply other threads:[~2016-08-16 14:36 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-15 20:58 uprobes: memory leak in enable/disable loop Brenden Blanco
2016-08-16 14:13 ` Oleg Nesterov
2016-08-16 14:25 ` Oleg Nesterov
2016-08-16 14:34 ` Oleg Nesterov [this message]
2016-08-16 16:55 ` Brenden Blanco
2016-08-17 15:36 ` [PATCH 0/2] " Oleg Nesterov
2016-08-17 15:36 ` [PATCH 1/2] uprobes: fix the memcg accounting Oleg Nesterov
2016-08-18 8:07 ` [tip:perf/urgent] uprobes: Fix " tip-bot for Oleg Nesterov
2016-08-17 15:37 ` [PATCH 2/2] uprobes: rename the "struct page *" args of __replace_page() Oleg Nesterov
2016-08-18 10:49 ` [tip:perf/core] uprobes: Rename " tip-bot for Oleg Nesterov
2016-08-16 17:36 ` uprobes: memory leak in enable/disable loop Johannes Weiner
2016-08-16 14:37 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160816143407.GC17006@redhat.com \
--to=oleg@redhat.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=alexei.starovoitov@gmail.com \
--cc=bblanco@plumgrid.com \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhocko@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=vdavydov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).