* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.