Re: [Powertop] [PATCH] Fix running failure when > 69 CPUs for open file limitation

[Sergey Senozhatsky <sergey.senozhatsky at gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

On (05/29/13 11:44), Youquan Song wrote:
> Powertop 2.3 fails to run on machine with > 69 CPUs because system open files
> limitation of one process is 1024 default,  While powertop will open one file
> for every monitored perf_event (at least 15) each CPU.
> 
> Like on 80 CPUs Westmere-EX machine, powertop will fail to run with below:
> 
> PowerTOP v2.3 needs the kernel to support the 'perf' subsystem
> as well as support for trace points in the kernel:
> CONFIG_PERF_EVENTS=y
> CONFIG_PERF_COUNTERS=y
> CONFIG_TRACEPOINTS=y
> CONFIG_TRACING=y
> 
> This patch is to change RLIMIT_NOFILE from default (1024) to max limition.
> 
> 

Hello,
I'd like to ask you to try out the following patch. We leak fd in read_file().




------8<--------8<--------

- Do not leak file descriptor in perf_bundle read_file().
- Clear perf event (unmap memory and close fd) on event destruction.


Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>

---

diff --git a/src/perf/perf.cpp b/src/perf/perf.cpp
index 35b4017..e919741 100644
--- a/src/perf/perf.cpp
+++ b/src/perf/perf.cpp
@@ -167,6 +167,7 @@ perf_event::~perf_event(void)
 	if (perf_event::pevent->ref_count == 1) {
 		pevent_free(perf_event::pevent);
 		perf_event::pevent = NULL;
+		clear();
 	} else
 		pevent_unref(perf_event::pevent);
 }
diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
index 38e1e91..8d480e8 100644
--- a/src/perf/perf_bundle.cpp
+++ b/src/perf/perf_bundle.cpp
@@ -123,6 +123,7 @@ static char * read_file(const char *file)
 		buffer[len] = '\0';
 	}
 out:
+	close(fd);
 	return buffer;
 }