linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* perf: kernel memory leak when inherit enabled
@ 2011-03-11 22:54 Vince Weaver
  2011-03-14 22:27 ` Vince Weaver
  0 siblings, 1 reply; 10+ messages in thread
From: Vince Weaver @ 2011-03-11 22:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: TEXT/PLAIN, Size: 761 bytes --]

Hello

While trying to use perf events with inherit enabled to profile some 
multi-threaded BLAS routines (using PAPI) I ended up out-of-memorying my 
machine.  It turns out you can quickly leak gigabytes of kernel memory
that isn't freed when the process exits.

I've attached a test case that reproduces this.

It leaks on 2.6.37 and 2.6.35.9.  It does not on 2.6.32.

I would like to try on current git and maybe even bisect, but my devel 
machine currently is broken with the ".size expression does not evaluate 
to a constant" binutils nonsense.

Unfortunately I won't have much chance to test any more until Monday, but 
I thought I'd send this in case it's something obvious that can be easily 
reproduced and fixed.

Thanks,

Vince
vweaver1@eecs.utk.edu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-csrc; name=pe_inherit_memleak.c, Size: 1749 bytes --]

/* pe_inherit_memleak.c  */
/* by Vince Weaver   vweaver1 _at_ eecs.utk.edu */

/* Each time run will leak hundreds of megabytes of kernel memory */

/* Compile with gcc -O2 -Wall -o pe_inherit_memleak pe_inherit_memleak.c -lpthread */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <pthread.h>

#include <unistd.h>
#include <asm/unistd.h>
#include <sys/prctl.h>
#include <linux/perf_event.h>

void *thread_work(void *blah) {
    
   return NULL;
}

int perf_event_open(struct perf_event_attr *hw_event_uptr,
		    pid_t pid, int cpu, int group_fd, unsigned long flags) {
 
   return syscall(__NR_perf_event_open, hw_event_uptr, pid, cpu, group_fd,flags);
}

int main(int argc, char** argv) {
   
   int i,fd1,fd2;

   struct perf_event_attr pe;
   
   memset(&pe,0,sizeof(struct perf_event_attr));

   pe.type=PERF_TYPE_HARDWARE;
   pe.config=PERF_COUNT_HW_CPU_CYCLES;
   pe.disabled=1;
   pe.inherit=1;
   pe.exclude_kernel=1;
   pe.exclude_hv=1;
   
   fd1=perf_event_open(&pe,0,-1,-1,0);
   if (fd1<0) {
      fprintf(stderr,"Error opening\n");
      exit(1);
   }
   
   pe.type=PERF_TYPE_HARDWARE;
   pe.config=PERF_COUNT_HW_INSTRUCTIONS;
   pe.disabled=0;
   pe.inherit=1;
   pe.exclude_kernel=1;
   pe.exclude_hv=1;
   
   fd2=perf_event_open(&pe,0,-1,fd1,0);
   if (fd1<0) {
      fprintf(stderr,"Error opening\n");
      exit(1);
   }
       
   for(i=0;i<10000;i++) {
      
      int j;
      pthread_t our_thread[8];
      
      for(j=0;j<8;j++) {
         pthread_create(&our_thread[j],NULL,thread_work,0);	 
      }
      
      for(j=0;j<8;j++) {
         pthread_join(our_thread[j],NULL);	 
      }
      
   }
   
   return 0;
}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: perf: kernel memory leak when inherit enabled
  2011-03-11 22:54 perf: kernel memory leak when inherit enabled Vince Weaver
@ 2011-03-14 22:27 ` Vince Weaver
  2011-03-14 22:32   ` Arnaldo Carvalho de Melo
  2011-03-15  9:07   ` perf: kernel memory leak when inherit enabled Peter Zijlstra
  0 siblings, 2 replies; 10+ messages in thread
From: Vince Weaver @ 2011-03-14 22:27 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo

On Fri, 11 Mar 2011, Vince Weaver wrote:
> 
> While trying to use perf events with inherit enabled to profile some 
> multi-threaded BLAS routines (using PAPI) I ended up out-of-memorying my 
> machine.  It turns out you can quickly leak gigabytes of kernel memory
> that isn't freed when the process exits.

I've bisected this.  There's a whole day I'll never see again. binutils 
2.21 and gcc-4.5 for the lose :(

Anyway this memory leak with inherit was introduced in 
  4fd38e4595e

Vince


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: perf: kernel memory leak when inherit enabled
  2011-03-14 22:27 ` Vince Weaver
@ 2011-03-14 22:32   ` Arnaldo Carvalho de Melo
  2011-03-15  0:27     ` Vince Weaver
  2011-03-15  9:07   ` perf: kernel memory leak when inherit enabled Peter Zijlstra
  1 sibling, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-03-14 22:32 UTC (permalink / raw)
  To: Vince Weaver
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Corey Ashford, Stephane Eranian

Em Mon, Mar 14, 2011 at 06:27:19PM -0400, Vince Weaver escreveu:
> On Fri, 11 Mar 2011, Vince Weaver wrote:
> > 
> > While trying to use perf events with inherit enabled to profile some 
> > multi-threaded BLAS routines (using PAPI) I ended up out-of-memorying my 
> > machine.  It turns out you can quickly leak gigabytes of kernel memory
> > that isn't freed when the process exits.
> 
> I've bisected this.  There's a whole day I'll never see again. binutils 
> 2.21 and gcc-4.5 for the lose :(
> 
> Anyway this memory leak with inherit was introduced in 
>   4fd38e4595e


commit 4fd38e4595e2f6c9d27732c042a0e16b2753049c
Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date:   Thu May 6 17:31:38 2010 +0200

    perf: Fix exit() vs PERF_FORMAT_GROUP
    
    Both Stephane and Corey reported that PERF_FORMAT_GROUP didn't work
    as expected if the task the counters were attached to quit before
    the read() call.
    
    The cause is that we unconditionally destroy the grouping when we
    remove counters from their context. Fix this by only doing this when
    we free the counter itself.
    
    Reported-by: Corey Ashford <cjashfor@linux.vnet.ibm.com>
    Reported-by: Stephane Eranian <eranian@google.com>
    Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
    LKML-Reference: <1273160566.5605.404.camel@twins>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

- Arnaldo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: perf: kernel memory leak when inherit enabled
  2011-03-14 22:32   ` Arnaldo Carvalho de Melo
@ 2011-03-15  0:27     ` Vince Weaver
  2011-03-15  2:26       ` Vince Weaver
  0 siblings, 1 reply; 10+ messages in thread
From: Vince Weaver @ 2011-03-15  0:27 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Corey Ashford, Stephane Eranian

On Mon, 14 Mar 2011, Arnaldo Carvalho de Melo wrote:

> Em Mon, Mar 14, 2011 at 06:27:19PM -0400, Vince Weaver escreveu:
> > On Fri, 11 Mar 2011, Vince Weaver wrote:
> > > 
> > > While trying to use perf events with inherit enabled to profile some 
> > > multi-threaded BLAS routines (using PAPI) I ended up out-of-memorying my 
> > > machine.  It turns out you can quickly leak gigabytes of kernel memory
> > > that isn't freed when the process exits.
> > 
> > I've bisected this.  There's a whole day I'll never see again. binutils 
> > 2.21 and gcc-4.5 for the lose :(
> > 
> > Anyway this memory leak with inherit was introduced in 
> >   4fd38e4595e
> 
> 
> commit 4fd38e4595e2f6c9d27732c042a0e16b2753049c
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date:   Thu May 6 17:31:38 2010 +0200
> 
>     perf: Fix exit() vs PERF_FORMAT_GROUP

This changeset was reverted already in 
e3174cfd2a1e28fff774681f00a0eef3d31da970
yet somehow that didn't fix the inherit mem-leak.

Vince
vweaver1@eecs.utk.edu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: perf: kernel memory leak when inherit enabled
  2011-03-15  0:27     ` Vince Weaver
@ 2011-03-15  2:26       ` Vince Weaver
  2011-03-15 13:41         ` [PATCH] perf: Fix tear-down of inherited group events Peter Zijlstra
  0 siblings, 1 reply; 10+ messages in thread
From: Vince Weaver @ 2011-03-15  2:26 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Corey Ashford, Stephane Eranian

On Mon, 14 Mar 2011, Vince Weaver wrote:

> On Mon, 14 Mar 2011, Arnaldo Carvalho de Melo wrote:
> 
> > Em Mon, Mar 14, 2011 at 06:27:19PM -0400, Vince Weaver escreveu:
> > > On Fri, 11 Mar 2011, Vince Weaver wrote:
> > > > 
> > > > While trying to use perf events with inherit enabled to profile some 
> > > > multi-threaded BLAS routines (using PAPI) I ended up out-of-memorying my 
> > > > machine.  It turns out you can quickly leak gigabytes of kernel memory
> > > > that isn't freed when the process exits.
> > > 
> > > I've bisected this.  There's a whole day I'll never see again. binutils 
> > > 2.21 and gcc-4.5 for the lose :(
> > > 
> > > Anyway this memory leak with inherit was introduced in 
> > >   4fd38e4595e
> 
> This changeset was reverted already in 
> e3174cfd2a1e28fff774681f00a0eef3d31da970
> yet somehow that didn't fix the inherit mem-leak.

I see, a new fix was made immediately after the revert, 
   050735b08ca8a016bbace4445fa025b88fee770b
which probably immediately re-introduced the problem, which is why
git bisect didn't catch this.  I'm away from my test machine so I'll
have to wait until tomorrow before I can investigate more.

Vince
vweaver1@eecs.utk.edu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: perf: kernel memory leak when inherit enabled
  2011-03-14 22:27 ` Vince Weaver
  2011-03-14 22:32   ` Arnaldo Carvalho de Melo
@ 2011-03-15  9:07   ` Peter Zijlstra
  1 sibling, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2011-03-15  9:07 UTC (permalink / raw)
  To: Vince Weaver
  Cc: linux-kernel, Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo

On Mon, 2011-03-14 at 18:27 -0400, Vince Weaver wrote:
> On Fri, 11 Mar 2011, Vince Weaver wrote:
> > 
> > While trying to use perf events with inherit enabled to profile some 
> > multi-threaded BLAS routines (using PAPI) I ended up out-of-memorying my 
> > machine.  It turns out you can quickly leak gigabytes of kernel memory
> > that isn't freed when the process exits.
> 
> I've bisected this.  There's a whole day I'll never see again. binutils 
> 2.21 and gcc-4.5 for the lose :(
> 
> Anyway this memory leak with inherit was introduced in 
>   4fd38e4595e

Thanks, managed to get some time yesterday and got as far as to see that
our perf_event_context refcounting is indeed screwy, but didn't get
around to actually catching the culprit.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] perf: Fix tear-down of inherited group events
  2011-03-15  2:26       ` Vince Weaver
@ 2011-03-15 13:41         ` Peter Zijlstra
  2011-03-15 16:09           ` Vince Weaver
  2011-03-16 13:59           ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
  0 siblings, 2 replies; 10+ messages in thread
From: Peter Zijlstra @ 2011-03-15 13:41 UTC (permalink / raw)
  To: Vince Weaver
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Paul Mackerras,
	Ingo Molnar, Corey Ashford, Stephane Eranian

Subject: perf: Fix tear-down of inherited group events
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue Mar 15 14:37:10 CET 2011

When destroying inherited events, we need to destroy groups too,
otherwise the event iteration in perf_event_exit_task_context() will
miss group siblings and we leak events with all the consequences.

Reported-by: Vince Weaver <vweaver1@eecs.utk.edu>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: stable@kernel.org # .35+
---
 kernel/perf_event.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Index: linux-2.6/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/kernel/perf_event.c
+++ linux-2.6/kernel/perf_event.c
@@ -6722,17 +6722,20 @@ __perf_event_exit_task(struct perf_event
 			 struct perf_event_context *child_ctx,
 			 struct task_struct *child)
 {
-	struct perf_event *parent_event;
+	if (child_event->parent) {
+		raw_spin_lock_irq(&child_ctx->lock);
+		perf_group_detach(child_event);
+		raw_spin_unlock_irq(&child_ctx->lock);
+	}
 
 	perf_remove_from_context(child_event);
 
-	parent_event = child_event->parent;
 	/*
-	 * It can happen that parent exits first, and has events
+	 * It can happen that the parent exits first, and has events
 	 * that are still around due to the child reference. These
-	 * events need to be zapped - but otherwise linger.
+	 * events need to be zapped.
 	 */
-	if (parent_event) {
+	if (child_event->parent) {
 		sync_child_event(child_event, child);
 		free_event(child_event);
 	}


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] perf: Fix tear-down of inherited group events
  2011-03-15 13:41         ` [PATCH] perf: Fix tear-down of inherited group events Peter Zijlstra
@ 2011-03-15 16:09           ` Vince Weaver
  2011-03-15 16:41             ` Peter Zijlstra
  2011-03-16 13:59           ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
  1 sibling, 1 reply; 10+ messages in thread
From: Vince Weaver @ 2011-03-15 16:09 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Paul Mackerras,
	Ingo Molnar, Corey Ashford, Stephane Eranian

On Tue, 15 Mar 2011, Peter Zijlstra wrote:

> Subject: perf: Fix tear-down of inherited group events
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Tue Mar 15 14:37:10 CET 2011
> 
> When destroying inherited events, we need to destroy groups too,
> otherwise the event iteration in perf_event_exit_task_context() will
> miss group siblings and we leak events with all the consequences.

Thanks for the fix!  I can verify that when applied against current 
linus-git kernel that my original test case no longer leaks memory.

I've also run the full PAPI regression tests, plus the BLAS/PAPI benchmark 
code that originally showed the problem and everything checks out fine.

It's a shame this fix didn't make it in before 2.6.38.

Tested-by: Vince Weaver <vweaver1@eecs.utk.edu>

Vince
vweaver1@eecs.utk.edu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] perf: Fix tear-down of inherited group events
  2011-03-15 16:09           ` Vince Weaver
@ 2011-03-15 16:41             ` Peter Zijlstra
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2011-03-15 16:41 UTC (permalink / raw)
  To: Vince Weaver
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Paul Mackerras,
	Ingo Molnar, Corey Ashford, Stephane Eranian

On Tue, 2011-03-15 at 12:09 -0400, Vince Weaver wrote:
> On Tue, 15 Mar 2011, Peter Zijlstra wrote:
> 
> > Subject: perf: Fix tear-down of inherited group events
> > From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Date: Tue Mar 15 14:37:10 CET 2011
> > 
> > When destroying inherited events, we need to destroy groups too,
> > otherwise the event iteration in perf_event_exit_task_context() will
> > miss group siblings and we leak events with all the consequences.
> 
> Thanks for the fix!  I can verify that when applied against current 
> linus-git kernel that my original test case no longer leaks memory.
> 
> I've also run the full PAPI regression tests, plus the BLAS/PAPI benchmark 
> code that originally showed the problem and everything checks out fine.
> 
> It's a shame this fix didn't make it in before 2.6.38.

It sure is, sadly there's still only 24h in a day, less if you consider
this .jp earthquake speeding up our planets spin :-)

> Tested-by: Vince Weaver <vweaver1@eecs.utk.edu>

Thanks Vince!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tip:perf/urgent] perf: Fix tear-down of inherited group events
  2011-03-15 13:41         ` [PATCH] perf: Fix tear-down of inherited group events Peter Zijlstra
  2011-03-15 16:09           ` Vince Weaver
@ 2011-03-16 13:59           ` tip-bot for Peter Zijlstra
  1 sibling, 0 replies; 10+ messages in thread
From: tip-bot for Peter Zijlstra @ 2011-03-16 13:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, tglx, vweaver1, mingo

Commit-ID:  38b435b16c36b0d863efcf3f07b34a6fac9873fd
Gitweb:     http://git.kernel.org/tip/38b435b16c36b0d863efcf3f07b34a6fac9873fd
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Tue, 15 Mar 2011 14:37:10 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 16 Mar 2011 14:04:14 +0100

perf: Fix tear-down of inherited group events

When destroying inherited events, we need to destroy groups too,
otherwise the event iteration in perf_event_exit_task_context() will
miss group siblings and we leak events with all the consequences.

Reported-and-tested-by: Vince Weaver <vweaver1@eecs.utk.edu>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: <stable@kernel.org> # .35+
LKML-Reference: <1300196470.2203.61.camel@twins>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/perf_event.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 533f715..3472bb1 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -6722,17 +6722,20 @@ __perf_event_exit_task(struct perf_event *child_event,
 			 struct perf_event_context *child_ctx,
 			 struct task_struct *child)
 {
-	struct perf_event *parent_event;
+	if (child_event->parent) {
+		raw_spin_lock_irq(&child_ctx->lock);
+		perf_group_detach(child_event);
+		raw_spin_unlock_irq(&child_ctx->lock);
+	}
 
 	perf_remove_from_context(child_event);
 
-	parent_event = child_event->parent;
 	/*
-	 * It can happen that parent exits first, and has events
+	 * It can happen that the parent exits first, and has events
 	 * that are still around due to the child reference. These
-	 * events need to be zapped - but otherwise linger.
+	 * events need to be zapped.
 	 */
-	if (parent_event) {
+	if (child_event->parent) {
 		sync_child_event(child_event, child);
 		free_event(child_event);
 	}

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-03-16 14:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-11 22:54 perf: kernel memory leak when inherit enabled Vince Weaver
2011-03-14 22:27 ` Vince Weaver
2011-03-14 22:32   ` Arnaldo Carvalho de Melo
2011-03-15  0:27     ` Vince Weaver
2011-03-15  2:26       ` Vince Weaver
2011-03-15 13:41         ` [PATCH] perf: Fix tear-down of inherited group events Peter Zijlstra
2011-03-15 16:09           ` Vince Weaver
2011-03-15 16:41             ` Peter Zijlstra
2011-03-16 13:59           ` [tip:perf/urgent] " tip-bot for Peter Zijlstra
2011-03-15  9:07   ` perf: kernel memory leak when inherit enabled Peter Zijlstra

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).