2015-06-30 08:15:56

by Taeung Song

[permalink] [raw]
Subject: [PATCH 0/5] perf session: Fill in the missing freeing a session after an error occur

In some cases some sessions aren't freed.
For example, a session is allocated and then
if an error occur, just a error value is returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Taeung Song (5):
perf inject: Fill in the missing freeing a session after an error
occur
perf kmem: Fill in the missing freeing a session after an error occur
perf kvm: Fill in the missing freeing a session after an error occur
perf mem: Fill in the missing freeing a session after an error occur
perf report: Fill in the missing freeing a session after an error
occur

tools/perf/builtin-inject.c | 7 ++++---
tools/perf/builtin-kmem.c | 4 ++--
tools/perf/builtin-kvm.c | 16 ++++++++++++----
tools/perf/builtin-mem.c | 17 ++++++++++-------
tools/perf/builtin-report.c | 6 ++++--
5 files changed, 32 insertions(+), 18 deletions(-)

--
1.9.1


2015-06-30 08:16:02

by Taeung Song

[permalink] [raw]
Subject: [PATCH 1/5] perf inject: Fill in the missing freeing a session after an error occur

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-inject.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 52ec66b..01b0649 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -630,12 +630,13 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
if (inject.session == NULL)
return -1;

- if (symbol__init(&inject.session->header.env) < 0)
- return -1;
+ ret = symbol__init(&inject.session->header.env);
+ if (ret < 0)
+ goto out_delete;

ret = __cmd_inject(&inject);

+out_delete:
perf_session__delete(inject.session);
-
return ret;
}
--
1.9.1

2015-06-30 08:17:13

by Taeung Song

[permalink] [raw]
Subject: [PATCH 2/5] perf kmem: Fill in the missing freeing a session after an error occur

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-kmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 950f296..23b1faa 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1916,7 +1916,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
if (!perf_evlist__find_tracepoint_by_name(session->evlist,
"kmem:kmalloc")) {
pr_err(errmsg, "slab", "slab");
- return -1;
+ goto out_delete;
}
}

@@ -1927,7 +1927,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
"kmem:mm_page_alloc");
if (evsel == NULL) {
pr_err(errmsg, "page", "page");
- return -1;
+ goto out_delete;
}

kmem_page_size = pevent_get_page_size(evsel->tp_format->pevent);
--
1.9.1

2015-06-30 08:16:24

by Taeung Song

[permalink] [raw]
Subject: [PATCH 3/5] perf kvm: Fill in the missing freeing a session after an error occur

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-kvm.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 74878cd..5fa96a0 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1061,8 +1061,10 @@ static int read_events(struct perf_kvm_stat *kvm)

symbol__init(&kvm->session->header.env);

- if (!perf_session__has_traces(kvm->session, "kvm record"))
- return -EINVAL;
+ if (!perf_session__has_traces(kvm->session, "kvm record")) {
+ ret = -EINVAL;
+ goto out_delete;
+ }

/*
* Do not use 'isa' recorded in kvm_exit tracepoint since it is not
@@ -1070,9 +1072,15 @@ static int read_events(struct perf_kvm_stat *kvm)
*/
ret = cpu_isa_config(kvm);
if (ret < 0)
- return ret;
+ goto out_delete;

- return perf_session__process_events(kvm->session);
+ ret = perf_session__process_events(kvm->session);
+ if (ret < 0)
+ goto out_delete;
+
+out_delete:
+ perf_session__delete(kvm->session);
+ return ret;
}

static int parse_target_str(struct perf_kvm_stat *kvm)
--
1.9.1

2015-06-30 08:16:46

by Taeung Song

[permalink] [raw]
Subject: [PATCH 4/5] perf mem: Fill in the missing freeing a session after an error occur

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-mem.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index da2ec06..8b6d473 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -135,24 +135,27 @@ static int report_raw_events(struct perf_mem *mem)
if (mem->cpu_list) {
ret = perf_session__cpu_bitmap(session, mem->cpu_list,
mem->cpu_bitmap);
- if (ret)
+ if (ret) {
+ ret = err;
goto out_delete;
+ }
}

- if (symbol__init(&session->header.env) < 0)
- return -1;
+ ret = symbol__init(&session->header.env);
+ if (ret < 0)
+ goto out_delete;

printf("# PID, TID, IP, ADDR, LOCAL WEIGHT, DSRC, SYMBOL\n");

err = perf_session__process_events(session);
if (err)
- return err;
-
- return 0;
+ ret = err;
+ else
+ ret = 0;

out_delete:
perf_session__delete(session);
- return err;
+ return ret;
}

static int report_events(int argc, const char **argv, struct perf_mem *mem)
--
1.9.1

2015-06-30 08:16:37

by Taeung Song

[permalink] [raw]
Subject: [PATCH 5/5] perf report: Fill in the missing freeing a session after an error occur

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-report.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 32626ea..610d056 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -828,8 +828,10 @@ repeat:
if (report.header || report.header_only) {
perf_session__fprintf_info(session, stdout,
report.show_full_info);
- if (report.header_only)
- return 0;
+ if (report.header_only) {
+ ret = 0;
+ goto error;
+ }
} else if (use_browser == 0) {
fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
stdout);
--
1.9.1

2015-06-30 11:14:52

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/5] perf kvm: Fill in the missing freeing a session after an error occur

On Tue, Jun 30, 2015 at 05:15:22PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/builtin-kvm.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 74878cd..5fa96a0 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -1061,8 +1061,10 @@ static int read_events(struct perf_kvm_stat *kvm)
>
> symbol__init(&kvm->session->header.env);
>
> - if (!perf_session__has_traces(kvm->session, "kvm record"))
> - return -EINVAL;
> + if (!perf_session__has_traces(kvm->session, "kvm record")) {
> + ret = -EINVAL;
> + goto out_delete;
> + }
>
> /*
> * Do not use 'isa' recorded in kvm_exit tracepoint since it is not
> @@ -1070,9 +1072,15 @@ static int read_events(struct perf_kvm_stat *kvm)
> */
> ret = cpu_isa_config(kvm);
> if (ret < 0)
> - return ret;
> + goto out_delete;
>
> - return perf_session__process_events(kvm->session);
> + ret = perf_session__process_events(kvm->session);
> + if (ret < 0)
> + goto out_delete;

above 2 lines are not needed.. just fall through

jirka

> +
> +out_delete:
> + perf_session__delete(kvm->session);
> + return ret;
> }
>
> static int parse_target_str(struct perf_kvm_stat *kvm)
> --
> 1.9.1
>

2015-06-30 11:18:00

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 4/5] perf mem: Fill in the missing freeing a session after an error occur

On Tue, Jun 30, 2015 at 05:15:23PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/builtin-mem.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> index da2ec06..8b6d473 100644
> --- a/tools/perf/builtin-mem.c
> +++ b/tools/perf/builtin-mem.c
> @@ -135,24 +135,27 @@ static int report_raw_events(struct perf_mem *mem)
> if (mem->cpu_list) {
> ret = perf_session__cpu_bitmap(session, mem->cpu_list,
> mem->cpu_bitmap);
> - if (ret)
> + if (ret) {
> + ret = err;
> goto out_delete;
> + }
> }
>
> - if (symbol__init(&session->header.env) < 0)
> - return -1;
> + ret = symbol__init(&session->header.env);
> + if (ret < 0)
> + goto out_delete;
>
> printf("# PID, TID, IP, ADDR, LOCAL WEIGHT, DSRC, SYMBOL\n");
>
> err = perf_session__process_events(session);
> if (err)
> - return err;
> -
> - return 0;
> + ret = err;
> + else
> + ret = 0;

hum.. how about just:
ret = perf_session__process_events(session);

and fall through..

also I guess we could get rid of one of those 'ret' and 'err' and use just one?

jirka

>
> out_delete:
> perf_session__delete(session);
> - return err;
> + return ret;
> }
>
> static int report_events(int argc, const char **argv, struct perf_mem *mem)
> --
> 1.9.1
>

2015-06-30 11:20:27

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 5/5] perf report: Fill in the missing freeing a session after an error occur

On Tue, Jun 30, 2015 at 05:15:24PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
>
> Signed-off-by: Taeung Song <[email protected]>

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

> ---
> tools/perf/builtin-report.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 32626ea..610d056 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -828,8 +828,10 @@ repeat:
> if (report.header || report.header_only) {
> perf_session__fprintf_info(session, stdout,
> report.show_full_info);
> - if (report.header_only)
> - return 0;
> + if (report.header_only) {
> + ret = 0;
> + goto error;
> + }
> } else if (use_browser == 0) {
> fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
> stdout);
> --
> 1.9.1
>

2015-06-30 11:21:55

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 1/5] perf inject: Fill in the missing freeing a session after an error occur

On Tue, Jun 30, 2015 at 05:15:20PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
>
> Signed-off-by: Taeung Song <[email protected]>

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

> ---
> tools/perf/builtin-inject.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 52ec66b..01b0649 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -630,12 +630,13 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
> if (inject.session == NULL)
> return -1;
>
> - if (symbol__init(&inject.session->header.env) < 0)
> - return -1;
> + ret = symbol__init(&inject.session->header.env);
> + if (ret < 0)
> + goto out_delete;
>
> ret = __cmd_inject(&inject);
>
> +out_delete:
> perf_session__delete(inject.session);
> -
> return ret;
> }
> --
> 1.9.1
>

2015-06-30 11:22:59

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 2/5] perf kmem: Fill in the missing freeing a session after an error occur

On Tue, Jun 30, 2015 at 05:15:21PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/builtin-kmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index 950f296..23b1faa 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -1916,7 +1916,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
> if (!perf_evlist__find_tracepoint_by_name(session->evlist,
> "kmem:kmalloc")) {
> pr_err(errmsg, "slab", "slab");
> - return -1;
> + goto out_delete;
> }
> }
>
> @@ -1927,7 +1927,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
> "kmem:mm_page_alloc");
> if (evsel == NULL) {
> pr_err(errmsg, "page", "page");
> - return -1;
> + goto out_delete;
> }
>
> kmem_page_size = pevent_get_page_size(evsel->tp_format->pevent);
> --
> 1.9.1
>

Subject: [tip:perf/urgent] perf inject: Fill in the missing session freeing after an error occurs

Commit-ID: 9fedfb0c5b05ae6c315de722a0548bb1f1328bf5
Gitweb: http://git.kernel.org/tip/9fedfb0c5b05ae6c315de722a0548bb1f1328bf5
Author: Taeung Song <[email protected]>
AuthorDate: Tue, 30 Jun 2015 17:15:20 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 1 Jul 2015 17:53:49 -0300

perf inject: Fill in the missing session freeing after an error occurs

When an error occur an error value is just returned without freeing the
session. So allocating and freeing session have to be matched as a pair
even if an error occurs.

Signed-off-by: Taeung Song <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-inject.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 52ec66b..01b0649 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -630,12 +630,13 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
if (inject.session == NULL)
return -1;

- if (symbol__init(&inject.session->header.env) < 0)
- return -1;
+ ret = symbol__init(&inject.session->header.env);
+ if (ret < 0)
+ goto out_delete;

ret = __cmd_inject(&inject);

+out_delete:
perf_session__delete(inject.session);
-
return ret;
}

Subject: [tip:perf/urgent] perf kmem: Fill in the missing session freeing after an error occurs

Commit-ID: 249ca1a86067e6a4198f7b2b7e19b505e2f41864
Gitweb: http://git.kernel.org/tip/249ca1a86067e6a4198f7b2b7e19b505e2f41864
Author: Taeung Song <[email protected]>
AuthorDate: Tue, 30 Jun 2015 17:15:21 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 1 Jul 2015 17:53:49 -0300

perf kmem: Fill in the missing session freeing after an error occurs

When an error occurs an error value is just returned without freeing the
session. So allocating and freeing session have to be matched as a pair
even if an error occurs.

Signed-off-by: Taeung Song <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-kmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 950f296..23b1faa 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1916,7 +1916,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
if (!perf_evlist__find_tracepoint_by_name(session->evlist,
"kmem:kmalloc")) {
pr_err(errmsg, "slab", "slab");
- return -1;
+ goto out_delete;
}
}

@@ -1927,7 +1927,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
"kmem:mm_page_alloc");
if (evsel == NULL) {
pr_err(errmsg, "page", "page");
- return -1;
+ goto out_delete;
}

kmem_page_size = pevent_get_page_size(evsel->tp_format->pevent);

Subject: [tip:perf/urgent] perf report: Fill in the missing session freeing after an error occurs

Commit-ID: 07a716fff25b826461baa2a07faa2df8c171f220
Gitweb: http://git.kernel.org/tip/07a716fff25b826461baa2a07faa2df8c171f220
Author: Taeung Song <[email protected]>
AuthorDate: Tue, 30 Jun 2015 17:15:24 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 1 Jul 2015 17:53:49 -0300

perf report: Fill in the missing session freeing after an error occurs

When an error occurs an error value is just returned without freeing the
session. So allocating and freeing session have to be matched as a pair
even if an error occurs.

Signed-off-by: Taeung Song <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-report.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 348bed4..95a4771 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -839,8 +839,10 @@ repeat:
if (report.header || report.header_only) {
perf_session__fprintf_info(session, stdout,
report.show_full_info);
- if (report.header_only)
- return 0;
+ if (report.header_only) {
+ ret = 0;
+ goto error;
+ }
} else if (use_browser == 0) {
fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
stdout);