Skip to contentSkip to conversation

perf session: Fill in the missing freeing a session after an error occur

PATCH Superseded Taeung Song 3 participants 14 messages Jun 30, 2015 – Jul 3, 2015 perf events

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover 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(-)
Signature · 1 line
1.9.1
#1 lore raw

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.

---
 tools/perf/builtin-inject.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
tools/perf/builtin-inject.c +4 -3
index 52ec66b..01b0649 100644
@@ -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;
 }
Signature · 1 line
1.9.1
Acked-by on v1 1/5
#2 lore raw

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

Quoted 5 lines · Taeung Song

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.

thanks,
jirka

Quoted 28 lines
---
 tools/perf/builtin-inject.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
tools/perf/builtin-inject.c +4 -3
index 52ec66b..01b0649 100644
@@ -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;
 }
Signature · 1 line
1.9.1
#3 lore raw

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

In reply to Taeung Song
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.

---
 tools/perf/builtin-inject.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
tools/perf/builtin-inject.c +4 -3
index 52ec66b..01b0649 100644
@@ -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;
 }
#4 lore raw

PATCH 2/5 perf kmem: Fill in the missing freeing a session after an error occur

In reply to Taeung Song

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.

---
 tools/perf/builtin-kmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-kmem.c +2 -2
index 950f296..23b1faa 100644
@@ -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);
Signature · 1 line
1.9.1
Acked-by on v1 2/5
#5 lore raw

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

Quoted 3 lines · Taeung Song

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.

thanks,
jirka

Quoted 31 lines
---
 tools/perf/builtin-kmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-kmem.c +2 -2
index 950f296..23b1faa 100644
@@ -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);
Signature · 1 line
1.9.1
#6 lore raw

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

In reply to Taeung Song
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.

---
 tools/perf/builtin-kmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-kmem.c +2 -2
index 950f296..23b1faa 100644
@@ -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);
#7 lore raw

PATCH 3/5 perf kvm: Fill in the missing freeing a session after an error occur

In reply to Taeung Song

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.

---
 tools/perf/builtin-kvm.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
tools/perf/builtin-kvm.c +12 -4
index 74878cd..5fa96a0 100644
@@ -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)
Signature · 1 line
1.9.1
#8 lore raw

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

Quoted 27 lines · Taeung Song

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.

---
 tools/perf/builtin-kvm.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
tools/perf/builtin-kvm.c +4 -2
index 74878cd..5fa96a0 100644
@@ -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

tools/perf/builtin-kvm.c static int read_events(struct perf_kvm_stat *kvm) · line 1079

Quoted 10 lines
tools/perf/builtin-kvm.c +4 -2
@@ -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

Quoted 10 lines
tools/perf/builtin-kvm.c +4 -0
+
+out_delete:
+	perf_session__delete(kvm->session);
+	return ret;
 }
 
 static int parse_target_str(struct perf_kvm_stat *kvm)
Signature · 1 line
1.9.1
#9 lore raw

PATCH 4/5 perf mem: Fill in the missing freeing a session after an error occur

In reply to Taeung Song

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.

---
 tools/perf/builtin-mem.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
tools/perf/builtin-mem.c +10 -7
index da2ec06..8b6d473 100644
@@ -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)
Signature · 1 line
1.9.1
#10 lore raw

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

Quoted 30 lines · Taeung Song

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.

---
 tools/perf/builtin-mem.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
tools/perf/builtin-mem.c +6 -3
index da2ec06..8b6d473 100644
@@ -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;
 

tools/perf/builtin-mem.c static int report_raw_events(struct perf_mem *mem) · line 154

Quoted 10 lines
tools/perf/builtin-mem.c +3 -3
 	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

Quoted 11 lines
tools/perf/builtin-mem.c +1 -1
 out_delete:
 	perf_session__delete(session);
-	return err;
+	return ret;
 }
 
 static int report_events(int argc, const char **argv, struct perf_mem *mem)
Signature · 1 line
1.9.1
#11 lore raw

PATCH 5/5 perf report: Fill in the missing freeing a session after an error occur

In reply to Taeung Song

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.

---
 tools/perf/builtin-report.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +4 -2
index 32626ea..610d056 100644
@@ -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);
Signature · 1 line
1.9.1
Acked-by on v1 5/5
#12 lore raw

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

Quoted 5 lines · Taeung Song

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.

thanks,
jirka

Quoted 24 lines
---
 tools/perf/builtin-report.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +4 -2
index 32626ea..610d056 100644
@@ -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);
Signature · 1 line
1.9.1
#13 lore raw

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

In reply to Taeung Song
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.

---
 tools/perf/builtin-report.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
tools/perf/builtin-report.c +4 -2
index 348bed4..95a4771 100644
@@ -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);