perf: kernel memory leak when inherit enabled

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
[email protected]

/* 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;
}

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

Em Mon, Mar 14, 2011 at 06:27:19PM -0400, Vince Weaver escreveu:

commit 4fd38e4595e2f6c9d27732c042a0e16b2753049c

Author: Peter Zijlstra <[email protected]>
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.

LKML-Reference: <1273160566.5605.404.camel@twins>

- Arnaldo

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

Vince
[email protected]

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
[email protected]

Subject: perf: Fix tear-down of inherited group events
From: Peter Zijlstra <[email protected]>
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.

---
 kernel/perf_event.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Index: 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);
 	}

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.

Vince
[email protected]

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

Thanks Vince!

Commit-ID:  38b435b16c36b0d863efcf3f07b34a6fac9873fd
Gitweb:     http://git.kernel.org/tip/38b435b16c36b0d863efcf3f07b34a6fac9873fd
Author:     Peter Zijlstra <[email protected]>
AuthorDate: Tue, 15 Mar 2011 14:37:10 +0100
Committer:  Ingo Molnar <[email protected]>
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.

LKML-Reference: <1300196470.2203.61.camel@twins>

---
 kernel/perf_event.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

index 533f715..3472bb1 100644
@@ -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);
 	}

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.