perf record: fix memory leak on AIO objects deallocation

PATCH In review Alexey Budankov 3 participants 5 messages Dec 5, 2018 – Dec 18, 2018 perf events

Share & tools

Alexey Budankov

author

2018-12-05 17:19 UTC #0 lore raw

PATCH 1/1 perf record: fix memory leak on AIO objects deallocation

Sending a part which was missed between v12 and v13 of the patch set introducing AIO trace streaming for perf record mode.

The part is essential to avoid memory leakage during deallocation of AIO related trace data buffers.

It is applied on top of acme perf/core repo.

Signed-off-by Alexey Budankov <[email protected]>

---
tools/perf/util/mmap.c | 10 ++++++++++

 1 file changed, 10 insertions(+)

tools/perf/util/mmap.c +10 -0

index ab30555d2afc..169d02973757 100644
@@ -207,8 +207,18 @@ static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp)
 
 static void perf_mmap__aio_munmap(struct perf_mmap *map)
 {
+	int i;
+
+	for (i = 0; i < map->aio.nr_cblocks; ++i) {
+		if (map->aio.data[i])
+			zfree(&map->aio.data[i]);
+	}
 	if (map->aio.data)
 		zfree(&map->aio.data);
+	if (map->aio.cblocks)
+		zfree(&map->aio.cblocks);
+	if (map->aio.aiocb)
+		zfree(&map->aio.aiocb);
 }
 
 int perf_mmap__aio_push(struct perf_mmap *md, void *to, int idx,

Arnaldo Carvalho de Melo

maintainer

2018-12-06 17:05 UTC #1 lore raw

Em Wed, Dec 05, 2018 at 08:19:41PM +0300, Alexey Budankov escreveu:

Quoted 24 lines

Sending a part which was missed between v12 and v13 of the patch set introducing AIO trace streaming for perf record mode.
The part is essential to avoid memory leakage during deallocation of AIO related trace data buffers.
It is applied on top of acme perf/core repo.
Signed-off-by Alexey Budankov <[email protected]>
---
tools/perf/util/mmap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
tools/perf/util/mmap.c +2 -0
index ab30555d2afc..169d02973757 100644
@@ -207,8 +207,18 @@ static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp)
 
 static void perf_mmap__aio_munmap(struct perf_mmap *map)
 {
+	int i;
+

tools/perf/util/mmap.c static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp) · line 221

Quoted 10 lines

tools/perf/util/mmap.c +8 -0

+	for (i = 0; i < map->aio.nr_cblocks; ++i) {
+		if (map->aio.data[i])
+			zfree(&map->aio.data[i]);
+	}
 	if (map->aio.data)
 		zfree(&map->aio.data);
+	if (map->aio.cblocks)
+		zfree(&map->aio.cblocks);
+	if (map->aio.aiocb)
+		zfree(&map->aio.aiocb);

There is no need to check for NULL before calling zfree(), as it is just a wrapper for free that sets that variable to NULL after calling it.

And free(NULL) is valid, just a noop to avoid having all those tests before each call to free(). I'm making those plain zfree() and later will do a sweep at removing other cases.

Quoted 3 lines

tools/perf/util/mmap.c +0 -0

 }
 
 int perf_mmap__aio_push(struct perf_mmap *md, void *to, int idx,

Signature · 1 line

- Arnaldo

Alexey Budankov

author

2018-12-06 18:10 UTC #2 lore raw

Quoted 39 lines · Arnaldo Carvalho de Melo

Em Wed, Dec 05, 2018 at 08:19:41PM +0300, Alexey Budankov escreveu:
Quoted 24 lines
Sending a part which was missed between v12 and v13 of the patch set introducing AIO trace streaming for perf record mode.
The part is essential to avoid memory leakage during deallocation of AIO related trace data buffers.
It is applied on top of acme perf/core repo.
Signed-off-by Alexey Budankov <[email protected]>
---
tools/perf/util/mmap.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
tools/perf/util/mmap.c +2 -0
index ab30555d2afc..169d02973757 100644
@@ -207,8 +207,18 @@ static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp)
 
 static void perf_mmap__aio_munmap(struct perf_mmap *map)
 {
+	int i;
+
tools/perf/util/mmap.c static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp) · line 221
Quoted 10 lines
tools/perf/util/mmap.c +8 -0
+	for (i = 0; i < map->aio.nr_cblocks; ++i) {
+		if (map->aio.data[i])
+			zfree(&map->aio.data[i]);
+	}
 	if (map->aio.data)
 		zfree(&map->aio.data);
+	if (map->aio.cblocks)
+		zfree(&map->aio.cblocks);
+	if (map->aio.aiocb)
+		zfree(&map->aio.aiocb);
There is no need to check for NULL before calling zfree(), as it is just a wrapper for free that sets that variable to NULL after calling it.

Quoted 3 lines

And free(NULL) is valid, just a noop to avoid having all those tests before each call to free(). I'm making those plain zfree() and later will do a sweep at removing other cases.

Thank you.
-Alexey

Quoted 5 lines

Quoted 4 lines
tools/perf/util/mmap.c +0 -0
 }
 
 int perf_mmap__aio_push(struct perf_mmap *md, void *to, int idx,

tip-bot for Alexey Budankov

merge bot

2018-12-14 20:59 UTC #3 lore raw

[tip:perf/core] perf record: Fix memory leak on AIO objects deallocation

In reply to Alexey Budankov

perf record: Fix memory leak on AIO objects deallocation

Commit-ID:  c713acc7090b32ca1c157f6528d607a087e807a8
Gitweb:     https://git.kernel.org/tip/c713acc7090b32ca1c157f6528d607a087e807a8
Author:     Alexey Budankov <[email protected]>
AuthorDate: Wed, 5 Dec 2018 20:19:41 +0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 6 Dec 2018 16:43:27 -0300

perf record: Fix memory leak on AIO objects deallocation

Sending a part which was missed between v12 and v13 of the patch set introducing AIO trace streaming for perf record mode.

The part is essential to avoid memory leakage during deallocation of AIO related trace data buffers.

Signed-off-by Alexey Budankov <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ No need to test for NULL before calling zfree() ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/mmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/mmap.c +6 -0

index ab30555d2afc..8fc39311a30d 100644
@@ -207,8 +207,14 @@ static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp)
 
 static void perf_mmap__aio_munmap(struct perf_mmap *map)
 {
+	int i;
+
+	for (i = 0; i < map->aio.nr_cblocks; ++i)
+		zfree(&map->aio.data[i]);
 	if (map->aio.data)
 		zfree(&map->aio.data);
+	zfree(&map->aio.cblocks);
+	zfree(&map->aio.aiocb);
 }
 
 int perf_mmap__aio_push(struct perf_mmap *md, void *to, int idx,

tip-bot for Alexey Budankov

merge bot

2018-12-18 14:28 UTC #4 lore raw

[tip:perf/core] perf record: Fix memory leak on AIO objects deallocation

In reply to Alexey Budankov

perf record: Fix memory leak on AIO objects deallocation

Commit-ID:  c8dd6ee51a4d0a119c8b4121d83008215e3209ed
Gitweb:     https://git.kernel.org/tip/c8dd6ee51a4d0a119c8b4121d83008215e3209ed
Author:     Alexey Budankov <[email protected]>
AuthorDate: Wed, 5 Dec 2018 20:19:41 +0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 17 Dec 2018 14:59:34 -0300

perf record: Fix memory leak on AIO objects deallocation

Sending a part which was missed between v12 and v13 of the patch set introducing AIO trace streaming for perf record mode.

The part is essential to avoid memory leakage during deallocation of AIO related trace data buffers.

Signed-off-by Alexey Budankov <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ No need to test for NULL before calling zfree() ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/mmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/mmap.c +6 -0

index ab30555d2afc..8fc39311a30d 100644
@@ -207,8 +207,14 @@ static int perf_mmap__aio_mmap(struct perf_mmap *map, struct mmap_params *mp)
 
 static void perf_mmap__aio_munmap(struct perf_mmap *map)
 {
+	int i;
+
+	for (i = 0; i < map->aio.nr_cblocks; ++i)
+		zfree(&map->aio.data[i]);
 	if (map->aio.data)
 		zfree(&map->aio.data);
+	zfree(&map->aio.cblocks);
+	zfree(&map->aio.aiocb);
 }
 
 int perf_mmap__aio_push(struct perf_mmap *md, void *to, int idx,