* [PATCH] perf session: Fill in the missing freeing a session after an error occur
@ 2015-06-27 14:08 taeung
2015-06-29 13:09 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 2+ messages in thread
From: taeung @ 2015-06-27 14:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, jolsa, namhyung, Ingo Molnar, taeung
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.
Signed-off-by: taeung <treeze.taeung@gmail.com>
---
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(-)
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;
}
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);
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)
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)
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] perf session: Fill in the missing freeing a session after an error occur
2015-06-27 14:08 [PATCH] perf session: Fill in the missing freeing a session after an error occur taeung
@ 2015-06-29 13:09 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-29 13:09 UTC (permalink / raw)
To: taeung; +Cc: linux-kernel, jolsa, namhyung, Ingo Molnar
Em Sat, Jun 27, 2015 at 11:08:52PM +0900, taeung escreveu:
> 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.
>
> Signed-off-by: taeung <treeze.taeung@gmail.com>
> ---
> 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(-)
<SNIP>
> +++ 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;
How come?
Can you break this into per-tool patches? If that would be the case I
would have applied some now.
- Arnaldo
> + }
> }
>
> - 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)
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-06-29 13:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-27 14:08 [PATCH] perf session: Fill in the missing freeing a session after an error occur taeung
2015-06-29 13:09 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).