2010-06-19 03:56:10

by Gui, Jianfeng/归 剑峰

Subject: [PATCH] perf: close the opened directories.

When I ran "perf kvm ... top", I encountered the following error output.

Error: perfcounter syscall returned with -1 (Too many open files)

Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?

Looking into perf, I found perf opens too many direcotries at initialization
time, but forgets to close them. Here is the fix.

Signed-off-by: Gui Jianfeng <[email protected]>
---
tools/perf/util/symbol.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7fd6b15..a00dcad 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1443,6 +1443,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
{
struct dirent *dent;
DIR *dir = opendir(dir_name);
+ int ret = 0;

if (!dir) {
pr_debug("%s: cannot open %s dir\n", __func__, dir_name);
@@ -1465,8 +1466,10 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,

snprintf(path, sizeof(path), "%s/%s",
dir_name, dent->d_name);
- if (map_groups__set_modules_path_dir(self, path) < 0)
- goto failure;
+ if (map_groups__set_modules_path_dir(self, path) < 0) {
+ ret = -1;
+ goto out;
+ }
} else {
char *dot = strrchr(dent->d_name, '.'),
dso_name[PATH_MAX];
@@ -1487,17 +1490,18 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
dir_name, dent->d_name);

long_name = strdup(path);
- if (long_name == NULL)
- goto failure;
+ if (long_name == NULL) {
+ ret = -1;
+ goto out;
+ }
dso__set_long_name(map->dso, long_name);
dso__kernel_module_get_build_id(map->dso, "");
}
}

- return 0;
-failure:
+out:
closedir(dir);
- return -1;
+ return ret;
}

static char *get_kernel_version(const char *root_dir)
--
1.6.5.2

2010-06-24 06:17:56

by Gui, Jianfeng/归 剑峰

Subject: Re: [PATCH] perf: close the opened directories.

Gui Jianfeng wrote:
> When I ran "perf kvm ... top", I encountered the following error output.
>
> Error: perfcounter syscall returned with -1 (Too many open files)
>
> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
>
> Looking into perf, I found perf opens too many direcotries at initialization
> time, but forgets to close them. Here is the fix.

Ping.

>
> Signed-off-by: Gui Jianfeng <[email protected]>
> ---
> tools/perf/util/symbol.c | 18 +++++++++++-------
> 1 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 7fd6b15..a00dcad 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1443,6 +1443,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
> {
> struct dirent *dent;
> DIR *dir = opendir(dir_name);
> + int ret = 0;
>
> if (!dir) {
> pr_debug("%s: cannot open %s dir\n", __func__, dir_name);
> @@ -1465,8 +1466,10 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
>
> snprintf(path, sizeof(path), "%s/%s",
> dir_name, dent->d_name);
> - if (map_groups__set_modules_path_dir(self, path) < 0)
> - goto failure;
> + if (map_groups__set_modules_path_dir(self, path) < 0) {
> + ret = -1;
> + goto out;
> + }
> } else {
> char *dot = strrchr(dent->d_name, '.'),
> dso_name[PATH_MAX];
> @@ -1487,17 +1490,18 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
> dir_name, dent->d_name);
>
> long_name = strdup(path);
> - if (long_name == NULL)
> - goto failure;
> + if (long_name == NULL) {
> + ret = -1;
> + goto out;
> + }
> dso__set_long_name(map->dso, long_name);
> dso__kernel_module_get_build_id(map->dso, "");
> }
> }
>
> - return 0;
> -failure:
> +out:
> closedir(dir);
> - return -1;
> + return ret;
> }
>
> static char *get_kernel_version(const char *root_dir)

2010-06-24 06:29:10

by Yanmin Zhang

Subject: Re: [PATCH] perf: close the opened directories.

On Sat, 2010-06-19 at 11:54 +0800, Gui Jianfeng wrote:
> When I ran "perf kvm ... top", I encountered the following error output.
>
> Error: perfcounter syscall returned with -1 (Too many open files)
>
> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
>
> Looking into perf, I found perf opens too many direcotries at initialization
> time, but forgets to close them. Here is the fix.
Good catch!

>
> Signed-off-by: Gui Jianfeng <[email protected]>
> ---
> tools/perf/util/symbol.c | 18 +++++++++++-------
> 1 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 7fd6b15..a00dcad 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1443,6 +1443,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
> {
> struct dirent *dent;
> DIR *dir = opendir(dir_name);
> + int ret = 0;
>
> if (!dir) {
> pr_debug("%s: cannot open %s dir\n", __func__, dir_name);
> @@ -1465,8 +1466,10 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
>
> snprintf(path, sizeof(path), "%s/%s",
> dir_name, dent->d_name);
> - if (map_groups__set_modules_path_dir(self, path) < 0)
> - goto failure;
> + if (map_groups__set_modules_path_dir(self, path) < 0) {
> + ret = -1;
> + goto out;
> + }
How about changing above to:
ret = map_groups__set_modules_path_dir(self, path);
if (ret < 0)
goto out;



> } else {
> char *dot = strrchr(dent->d_name, '.'),
> dso_name[PATH_MAX];
> @@ -1487,17 +1490,18 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
> dir_name, dent->d_name);
>
> long_name = strdup(path);
> - if (long_name == NULL)
> - goto failure;
> + if (long_name == NULL) {
> + ret = -1;
> + goto out;
> + }
> dso__set_long_name(map->dso, long_name);
> dso__kernel_module_get_build_id(map->dso, "");
> }
> }
>
> - return 0;
> -failure:
> +out:
> closedir(dir);
> - return -1;
> + return ret;
> }
>
> static char *get_kernel_version(const char *root_dir)

2010-06-24 06:50:54

by Gui, Jianfeng/归 剑峰

Subject: Re: [PATCH] perf: close the opened directories.

Zhang, Yanmin wrote:
> On Sat, 2010-06-19 at 11:54 +0800, Gui Jianfeng wrote:
>> When I ran "perf kvm ... top", I encountered the following error output.
>>
>> Error: perfcounter syscall returned with -1 (Too many open files)
>>
>> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
>>
>> Looking into perf, I found perf opens too many direcotries at initialization
>> time, but forgets to close them. Here is the fix.
> Good catch!
>
>> Signed-off-by: Gui Jianfeng <[email protected]>
>> ---
>> tools/perf/util/symbol.c | 18 +++++++++++-------
>> 1 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 7fd6b15..a00dcad 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -1443,6 +1443,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
>> {
>> struct dirent *dent;
>> DIR *dir = opendir(dir_name);
>> + int ret = 0;
>>
>> if (!dir) {
>> pr_debug("%s: cannot open %s dir\n", __func__, dir_name);
>> @@ -1465,8 +1466,10 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
>>
>> snprintf(path, sizeof(path), "%s/%s",
>> dir_name, dent->d_name);
>> - if (map_groups__set_modules_path_dir(self, path) < 0)
>> - goto failure;
>> + if (map_groups__set_modules_path_dir(self, path) < 0) {
>> + ret = -1;
>> + goto out;
>> + }
> How about changing above to:
> ret = map_groups__set_modules_path_dir(self, path);
> if (ret < 0)
> goto out;

Sure, will change.

Thanks,
Gui

>
>
>
>> } else {
>> char *dot = strrchr(dent->d_name, '.'),
>> dso_name[PATH_MAX];
>> @@ -1487,17 +1490,18 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
>> dir_name, dent->d_name);
>>
>> long_name = strdup(path);
>> - if (long_name == NULL)
>> - goto failure;
>> + if (long_name == NULL) {
>> + ret = -1;
>> + goto out;
>> + }
>> dso__set_long_name(map->dso, long_name);
>> dso__kernel_module_get_build_id(map->dso, "");
>> }
>> }
>>
>> - return 0;
>> -failure:
>> +out:
>> closedir(dir);
>> - return -1;
>> + return ret;
>> }
>>
>> static char *get_kernel_version(const char *root_dir)
>
>
>
>

2010-06-24 07:05:41

by Gui, Jianfeng/归 剑峰

Subject: [PATCH v2] perf: close the opened directories.

When I ran "perf kvm ... top", I encountered the following error output.

Error: perfcounter syscall returned with -1 (Too many open files)

Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?

Looking into perf, I found perf opens too many direcotries at initialization
time, but forgets to close them. Here is the fix.

Signef-off-by: Gui Jianfeng <[email protected]>
---
tools/perf/util/symbol.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7fd6b15..3609a20 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1443,6 +1443,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
{
struct dirent *dent;
DIR *dir = opendir(dir_name);
+ int ret = 0;

if (!dir) {
pr_debug("%s: cannot open %s dir\n", __func__, dir_name);
@@ -1465,8 +1466,9 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,

snprintf(path, sizeof(path), "%s/%s",
dir_name, dent->d_name);
- if (map_groups__set_modules_path_dir(self, path) < 0)
- goto failure;
+ ret = map_groups__set_modules_path_dir(self, path);
+ if (ret < 0)
+ goto out;
} else {
char *dot = strrchr(dent->d_name, '.'),
dso_name[PATH_MAX];
@@ -1487,17 +1489,18 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
dir_name, dent->d_name);

long_name = strdup(path);
- if (long_name == NULL)
- goto failure;
+ if (long_name == NULL) {
+ ret = -1;
+ goto out;
+ }
dso__set_long_name(map->dso, long_name);
dso__kernel_module_get_build_id(map->dso, "");
}
}

- return 0;
-failure:
+out:
closedir(dir);
- return -1;
+ return ret;
}

static char *get_kernel_version(const char *root_dir)
--
1.6.5.2

2010-06-24 07:21:48

by Peter Zijlstra

Subject: Re: [PATCH v2] perf: close the opened directories.

On Thu, 2010-06-24 at 15:04 +0800, Gui Jianfeng wrote:
> When I ran "perf kvm ... top", I encountered the following error output.
>
> Error: perfcounter syscall returned with -1 (Too many open files)
>
> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
>
> Looking into perf, I found perf opens too many direcotries at initialization
> time, but forgets to close them. Here is the fix.
>
> Signef-off-by: Gui Jianfeng <[email protected]>

Looks good to me, Arnaldo, will you pick this up?

2010-06-24 13:53:10

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH v2] perf: close the opened directories.

Em Thu, Jun 24, 2010 at 03:04:02PM +0800, Gui Jianfeng escreveu:
> When I ran "perf kvm ... top", I encountered the following error output.
>
> Error: perfcounter syscall returned with -1 (Too many open files)
>
> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
>
> Looking into perf, I found perf opens too many direcotries at initialization
> time, but forgets to close them. Here is the fix.
>
> Signef-off-by: Gui Jianfeng <[email protected]>

Thanks, will apply.

- Arnaldo

2010-07-16 07:39:22

by Gui, Jianfeng/归 剑峰

Subject: Re: [PATCH v2] perf: close the opened directories.

Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 24, 2010 at 03:04:02PM +0800, Gui Jianfeng escreveu:
>> When I ran "perf kvm ... top", I encountered the following error output.
>>
>> Error: perfcounter syscall returned with -1 (Too many open files)
>>
>> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
>>
>> Looking into perf, I found perf opens too many direcotries at initialization
>> time, but forgets to close them. Here is the fix.
>>
>> Signef-off-by: Gui Jianfeng <[email protected]>
>
> Thanks, will apply.

Hi Arnaldo,

Would you pick up this fix?

Thanks,
Gui

>
> - Arnaldo
>
>

2010-07-16 15:42:43

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH v2] perf: close the opened directories.

Em Fri, Jul 16, 2010 at 03:37:07PM +0800, Gui Jianfeng escreveu:
> Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jun 24, 2010 at 03:04:02PM +0800, Gui Jianfeng escreveu:
> >> When I ran "perf kvm ... top", I encountered the following error output.
> >>
> >> Error: perfcounter syscall returned with -1 (Too many open files)
> >>
> >> Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?
> >>
> >> Looking into perf, I found perf opens too many direcotries at initialization
> >> time, but forgets to close them. Here is the fix.
> >>
> >> Signef-off-by: Gui Jianfeng <[email protected]>
> >
> > Thanks, will apply.
>
> Hi Arnaldo,
>
> Would you pick up this fix?

It was already, no? Nope, I thought about that other one about . and ..,
will apply.

- Arnaldo

2010-07-17 11:12:10

by Gui, Jianfeng/归 剑峰

Subject: [tip:perf/urgent] perf symbols: Fix directory descriptor leaking

Commit-ID: 74534341c1214ac5993904680616afe698dde3b6
Gitweb: http://git.kernel.org/tip/74534341c1214ac5993904680616afe698dde3b6
Author: Gui Jianfeng <[email protected]>
AuthorDate: Thu, 24 Jun 2010 15:04:02 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 16 Jul 2010 14:16:47 -0300

perf symbols: Fix directory descriptor leaking

When I ran "perf kvm ... top", I encountered the following error output.

Error: perfcounter syscall returned with -1 (Too many open files)

Fatal: No CONFIG_PERF_EVENTS=y kernel support configured?

Looking into perf, I found perf opens too many directories at
initialization time, but forgets to close them. Here is the fix.

LKML-Reference: <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Gui Jianfeng <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/symbol.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b63e571..5b27683 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1443,6 +1443,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
{
struct dirent *dent;
DIR *dir = opendir(dir_name);
+ int ret = 0;

if (!dir) {
pr_debug("%s: cannot open %s dir\n", __func__, dir_name);
@@ -1465,8 +1466,9 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,

snprintf(path, sizeof(path), "%s/%s",
dir_name, dent->d_name);
- if (map_groups__set_modules_path_dir(self, path) < 0)
- goto failure;
+ ret = map_groups__set_modules_path_dir(self, path);
+ if (ret < 0)
+ goto out;
} else {
char *dot = strrchr(dent->d_name, '.'),
dso_name[PATH_MAX];
@@ -1487,17 +1489,18 @@ static int map_groups__set_modules_path_dir(struct map_groups *self,
dir_name, dent->d_name);

long_name = strdup(path);
- if (long_name == NULL)
- goto failure;
+ if (long_name == NULL) {
+ ret = -1;
+ goto out;
+ }
dso__set_long_name(map->dso, long_name);
dso__kernel_module_get_build_id(map->dso, "");
}
}

- return 0;
-failure:
+out:
closedir(dir);
- return -1;
+ return ret;
}

static char *get_kernel_version(const char *root_dir)