[PATCH] perf: close the opened directories.

[PATCH] perf: close the opened directories.

[Gui Jianfeng]

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 <guijianfeng@cn.fujitsu.com>
---
 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

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

[Gui Jianfeng]

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 <guijianfeng@cn.fujitsu.com>
> ---
>  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)

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

[Zhang, Yanmin]

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 <guijianfeng@cn.fujitsu.com>
> ---
>  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)

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

[Gui Jianfeng]

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 <guijianfeng@cn.fujitsu.com>
>> ---
>>  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)
> 
> 
> 
> 

[PATCH v2] perf: close the opened directories.

[Gui Jianfeng]

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 <guijianfeng@cn.fujitsu.com>
---
 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

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

[Peter Zijlstra]

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 <guijianfeng@cn.fujitsu.com>

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

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

[Arnaldo Carvalho de Melo]

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 <guijianfeng@cn.fujitsu.com>

Thanks, will apply.

- Arnaldo

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

[Gui Jianfeng]

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 <guijianfeng@cn.fujitsu.com>
> 
> Thanks, will apply.

Hi Arnaldo,

Would you pick up this fix?

Thanks,
Gui

> 
> - Arnaldo
> 
> 

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

[Arnaldo Carvalho de Melo]

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 <guijianfeng@cn.fujitsu.com>
> > 
> > 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

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

[tip-bot for Gui Jianfeng]

Commit-ID:  74534341c1214ac5993904680616afe698dde3b6
Gitweb:     http://git.kernel.org/tip/74534341c1214ac5993904680616afe698dde3b6
Author:     Gui Jianfeng <guijianfeng@cn.fujitsu.com>
AuthorDate: Thu, 24 Jun 2010 15:04:02 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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: <4C230362.5080704@cn.fujitsu.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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)