[PATCH 1/2] perf tools: Fix memory leak in read_ftrace_printk()

[PATCH 1/2] perf tools: Fix memory leak in read_ftrace_printk()

[Li Zefan]

get_tracing_file() should be paired with put_tracing_file().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 tools/perf/util/trace-event-info.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index 6c9302a..1fd824c 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -458,7 +458,7 @@ static void read_proc_kallsyms(void)
 static void read_ftrace_printk(void)
 {
 	unsigned int size, check_size;
-	const char *path;
+	char *path;
 	struct stat st;
 	int ret;
 
@@ -468,14 +468,15 @@ static void read_ftrace_printk(void)
 		/* not found */
 		size = 0;
 		write_or_die(&size, 4);
-		return;
+		goto out;
 	}
 	size = get_size(path);
 	write_or_die(&size, 4);
 	check_size = copy_file(path);
 	if (size != check_size)
 		die("error in size of file '%s'", path);
-
+out:
+	put_tracing_file(path);
 }
 
 static struct tracepoint_path *
-- 
1.6.3

[PATCH 2/2] perf tools: Increase MAX_EVENT_LENGTH

[Li Zefan]

The name length of some trace events is longer than 30, like
sys_enter_sched_get_priority_max and ext4_mb_discard_preallocations.

Passing those events to perf-record will fail, try:

  # ./perf record -f -e syscalls:sys_enter_sched_get_priority_max -F 1 -a

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 tools/perf/util/parse-events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 034245e..c9ef944 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -145,7 +145,7 @@ static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
 	   (strcmp(evt_dirent.d_name, "..")) &&				       \
 	   (!tp_event_has_id(&sys_dirent, &evt_dirent)))
 
-#define MAX_EVENT_LENGTH 30
+#define MAX_EVENT_LENGTH 40
 
 int valid_debugfs_mount(const char *debugfs)
 {
-- 
1.6.3

Re: [PATCH 1/2] perf tools: Fix memory leak in read_ftrace_printk()

[Frederic Weisbecker]

On Thu, Sep 17, 2009 at 04:34:23PM +0800, Li Zefan wrote:
> get_tracing_file() should be paired with put_tracing_file().
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>



Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


> ---
>  tools/perf/util/trace-event-info.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
> index 6c9302a..1fd824c 100644
> --- a/tools/perf/util/trace-event-info.c
> +++ b/tools/perf/util/trace-event-info.c
> @@ -458,7 +458,7 @@ static void read_proc_kallsyms(void)
>  static void read_ftrace_printk(void)
>  {
>  	unsigned int size, check_size;
> -	const char *path;
> +	char *path;
>  	struct stat st;
>  	int ret;
>  
> @@ -468,14 +468,15 @@ static void read_ftrace_printk(void)
>  		/* not found */
>  		size = 0;
>  		write_or_die(&size, 4);
> -		return;
> +		goto out;
>  	}
>  	size = get_size(path);
>  	write_or_die(&size, 4);
>  	check_size = copy_file(path);
>  	if (size != check_size)
>  		die("error in size of file '%s'", path);
> -
> +out:
> +	put_tracing_file(path);
>  }
>  
>  static struct tracepoint_path *
> -- 
> 1.6.3
> 

Re: [PATCH 2/2] perf tools: Increase MAX_EVENT_LENGTH

[Frederic Weisbecker]

On Thu, Sep 17, 2009 at 04:34:51PM +0800, Li Zefan wrote:
> The name length of some trace events is longer than 30, like
> sys_enter_sched_get_priority_max and ext4_mb_discard_preallocations.
> 
> Passing those events to perf-record will fail, try:
> 
>   # ./perf record -f -e syscalls:sys_enter_sched_get_priority_max -F 1 -a
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  tools/perf/util/parse-events.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 034245e..c9ef944 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -145,7 +145,7 @@ static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
>  	   (strcmp(evt_dirent.d_name, "..")) &&				       \
>  	   (!tp_event_has_id(&sys_dirent, &evt_dirent)))
>  
> -#define MAX_EVENT_LENGTH 30
> +#define MAX_EVENT_LENGTH 40



Ack too.

Thanks Li!

  
>  int valid_debugfs_mount(const char *debugfs)
>  {
> -- 
> 1.6.3
> 

Re: [PATCH 2/2] perf tools: Increase MAX_EVENT_LENGTH

[Peter Zijlstra]

On Thu, 2009-09-17 at 11:02 +0200, Frederic Weisbecker wrote:
> On Thu, Sep 17, 2009 at 04:34:51PM +0800, Li Zefan wrote:
> > The name length of some trace events is longer than 30, like
> > sys_enter_sched_get_priority_max and ext4_mb_discard_preallocations.
> > 
> > Passing those events to perf-record will fail, try:
> > 
> >   # ./perf record -f -e syscalls:sys_enter_sched_get_priority_max -F 1 -a
> > 
> > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > ---
> >  tools/perf/util/parse-events.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 034245e..c9ef944 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -145,7 +145,7 @@ static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
> >  	   (strcmp(evt_dirent.d_name, "..")) &&				       \
> >  	   (!tp_event_has_id(&sys_dirent, &evt_dirent)))
> >  
> > -#define MAX_EVENT_LENGTH 30
> > +#define MAX_EVENT_LENGTH 40

This is userspace, is there any reason to be cheap with memory like
this?

Why not stick in 1024 and be done for a while?

Re: [PATCH 2/2] perf tools: Increase MAX_EVENT_LENGTH

[Frederic Weisbecker]

On Thu, Sep 17, 2009 at 11:08:34AM +0200, Peter Zijlstra wrote:
> On Thu, 2009-09-17 at 11:02 +0200, Frederic Weisbecker wrote:
> > On Thu, Sep 17, 2009 at 04:34:51PM +0800, Li Zefan wrote:
> > > The name length of some trace events is longer than 30, like
> > > sys_enter_sched_get_priority_max and ext4_mb_discard_preallocations.
> > > 
> > > Passing those events to perf-record will fail, try:
> > > 
> > >   # ./perf record -f -e syscalls:sys_enter_sched_get_priority_max -F 1 -a
> > > 
> > > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > > ---
> > >  tools/perf/util/parse-events.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > > index 034245e..c9ef944 100644
> > > --- a/tools/perf/util/parse-events.c
> > > +++ b/tools/perf/util/parse-events.c
> > > @@ -145,7 +145,7 @@ static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
> > >  	   (strcmp(evt_dirent.d_name, "..")) &&				       \
> > >  	   (!tp_event_has_id(&sys_dirent, &evt_dirent)))
> > >  
> > > -#define MAX_EVENT_LENGTH 30
> > > +#define MAX_EVENT_LENGTH 40
> 
> This is userspace, is there any reason to be cheap with memory like
> this?
> 
> Why not stick in 1024 and be done for a while?
> 


Indeed, we may need to go even further than 40 in the future.
But I guess 512 would be already sufficient.

Re: [PATCH 2/2] perf tools: Increase MAX_EVENT_LENGTH

[Li Zefan]

>>>> The name length of some trace events is longer than 30, like
>>>> sys_enter_sched_get_priority_max and ext4_mb_discard_preallocations.
>>>>
>>>> Passing those events to perf-record will fail, try:
>>>>
>>>>   # ./perf record -f -e syscalls:sys_enter_sched_get_priority_max -F 1 -a
>>>>
>>>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>>>> ---
>>>>  tools/perf/util/parse-events.c |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>>>> index 034245e..c9ef944 100644
>>>> --- a/tools/perf/util/parse-events.c
>>>> +++ b/tools/perf/util/parse-events.c
>>>> @@ -145,7 +145,7 @@ static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
>>>>  	   (strcmp(evt_dirent.d_name, "..")) &&				       \
>>>>  	   (!tp_event_has_id(&sys_dirent, &evt_dirent)))
>>>>  
>>>> -#define MAX_EVENT_LENGTH 30
>>>> +#define MAX_EVENT_LENGTH 40
>> This is userspace, is there any reason to be cheap with memory like
>> this?
>>
>> Why not stick in 1024 and be done for a while?
>>
> 
> Indeed, we may need to go even further than 40 in the future.
> But I guess 512 would be already sufficient.
> 

We are not allocating memory for all tracepoints, but only
those to be profiled, so a larger limit is OK.

But as the sys_name:evnt_name will be displayed using perf-list
cmd, I don't think the names will be too long.

Anyway, we decide to make it 512? I'll resend the patch.

[tip:perfcounters/core] perf tools: Fix memory leak in read_ftrace_printk()

[tip-bot for Li Zefan]

Commit-ID:  6706ccf8e776e51e38ffa89fe7dd20e80eb1e860
Gitweb:     http://git.kernel.org/tip/6706ccf8e776e51e38ffa89fe7dd20e80eb1e860
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Thu, 17 Sep 2009 16:34:23 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 18 Sep 2009 00:30:24 +0200

perf tools: Fix memory leak in read_ftrace_printk()

get_tracing_file() should be paired with put_tracing_file().

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <4AB1F48F.4070807@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 tools/perf/util/trace-event-info.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index 6c9302a..1fd824c 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -458,7 +458,7 @@ static void read_proc_kallsyms(void)
 static void read_ftrace_printk(void)
 {
 	unsigned int size, check_size;
-	const char *path;
+	char *path;
 	struct stat st;
 	int ret;
 
@@ -468,14 +468,15 @@ static void read_ftrace_printk(void)
 		/* not found */
 		size = 0;
 		write_or_die(&size, 4);
-		return;
+		goto out;
 	}
 	size = get_size(path);
 	write_or_die(&size, 4);
 	check_size = copy_file(path);
 	if (size != check_size)
 		die("error in size of file '%s'", path);
-
+out:
+	put_tracing_file(path);
 }
 
 static struct tracepoint_path *

[tip:perfcounters/core] perf tools: Increase MAX_EVENT_LENGTH

[tip-bot for Li Zefan]

Commit-ID:  270bbbe80d82fad8b698d0b407eb3ad67cc3492b
Gitweb:     http://git.kernel.org/tip/270bbbe80d82fad8b698d0b407eb3ad67cc3492b
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Thu, 17 Sep 2009 16:34:51 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 18 Sep 2009 00:30:25 +0200

perf tools: Increase MAX_EVENT_LENGTH

The name length of some trace events is longer than 30, like
sys_enter_sched_get_priority_max and
ext4_mb_discard_preallocations.

Passing those events to perf-record will fail, try:

  # ./perf record -f -e syscalls:sys_enter_sched_get_priority_max -F 1 -a

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <4AB1F4AB.7050205@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 tools/perf/util/parse-events.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 034245e..910283c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -145,7 +145,7 @@ static int tp_event_has_id(struct dirent *sys_dir, struct dirent *evt_dir)
 	   (strcmp(evt_dirent.d_name, "..")) &&				       \
 	   (!tp_event_has_id(&sys_dirent, &evt_dirent)))
 
-#define MAX_EVENT_LENGTH 30
+#define MAX_EVENT_LENGTH 512
 
 int valid_debugfs_mount(const char *debugfs)
 {