2013-10-28 14:45:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/2] perf/urgent fixes

From: Arnaldo Carvalho de Melo <[email protected]>

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit e4f8eaad70ea186b8da290c99239dce721a34f88:

Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2013-10-20 10:51:35 +0200)

are available in the git repository at:


git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo

for you to fetch changes up to 2fd869f08aec5a8e4cbf01bc3fa345c4e53342d7:

perf tools: Fix up /proc/PID/maps parsing (2013-10-28 09:38:12 -0300)

----------------------------------------------------------------
perf/urgent fixes:

. Fix up /proc/PID/maps parsing, where perfectly fine mmap entries
were being trown away when synthesizing PERF_RECORD_MMAP for
preexisting threads, prevenging symbol resolution to work
for those threads, broken in the MMAP2 removal. Reported and
pinpointed by Markus Trippelsdorf,

. Fix mem leak in the python 'perf script' backend, due to missing Py_DECREFs
on dict entries, fix from Joseph Schuchart.

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):
perf tools: Fix up /proc/PID/maps parsing

Joseph Schuchart (1):
perf script python: Fix mem leak due to missing Py_DECREFs on dict entries

tools/perf/util/event.c | 2 +-
.../util/scripting-engines/trace-event-python.c | 37 ++++++++++++++--------
2 files changed, 25 insertions(+), 14 deletions(-)


2013-10-28 14:45:44

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/2] perf tools: Fix up /proc/PID/maps parsing

From: Arnaldo Carvalho de Melo <[email protected]>

When introducing support for MMAP2 we considered more parts of each map
representation in /proc/PID/maps, and when disabling it we forgot to
reduce the number of expected parsed/assigned entries in the sscanf
call, fix it to expect the right number of desired fields, 5.

Reported-by: Markus Trippelsdorf <[email protected]>
Based-on-a-patch-by: Markus Trippelsdorf <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 63df031fc9c7..49096ea58a15 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -213,7 +213,7 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
&event->mmap.pgoff,
execname);

- if (n != 8)
+ if (n != 5)
continue;

if (prot[2] != 'x')
--
1.8.1.4

2013-10-28 14:45:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/2] perf script python: Fix mem leak due to missing Py_DECREFs on dict entries

From: Joseph Schuchart <[email protected]>

We are using the Python scripting interface in perf to extract kernel
events relevant for performance analysis of HPC codes. We noticed that
the "perf script" call allocates a significant amount of memory (in the
order of several 100 MiB) during it's run, e.g. 125 MiB for a 25 MiB
input file:

$> perf record -o perf.data -a -R -g fp \
-e power:cpu_frequency -e sched:sched_switch \
-e sched:sched_migrate_task -e sched:sched_process_exit \
-e sched:sched_process_fork -e sched:sched_process_exec \
-e cycles -m 4096 --freq 4000
$> /usr/bin/time perf script -i perf.data -s dummy_script.py
0.84user 0.13system 0:01.92elapsed 51%CPU (0avgtext+0avgdata
125532maxresident)k
73072inputs+0outputs (57major+33086minor)pagefaults 0swaps

Upon further investigation using the valgrind massif tool, we noticed
that Python objects that are created in trace-event-python.c via
PyString_FromString*() (and their Integer and Long counterparts) are
never free'd.

The reason for this seem to be missing Py_DECREF calls on the objects
that are returned by these functions and stored in the Python
dictionaries. The Python dictionaries do not steal references (as
opposed to Python tuples and lists) but instead add their own reference.

Hence, the reference that is returned by these object creation functions
is never released and the memory is leaked. (see [1,2])

The attached patch fixes this by wrapping all relevant calls to
PyDict_SetItemString() and decrementing the reference counter
immediately after the Python function call.

This reduces the allocated memory to a reasonable amount:

$> /usr/bin/time perf script -i perf.data -s dummy_script.py
0.73user 0.05system 0:00.79elapsed 99%CPU (0avgtext+0avgdata
49132maxresident)k
0inputs+0outputs (0major+14045minor)pagefaults 0swaps

For comparison, with a 120 MiB input file the memory consumption
reported by time drops from almost 600 MiB to 146 MiB.

The patch has been tested using Linux 3.8.2 with Python 2.7.4 and Linux
3.11.6 with Python 2.7.5.

Please let me know if you need any further information.

[1] http://docs.python.org/2/c-api/tuple.html#PyTuple_SetItem
[2] http://docs.python.org/2/c-api/dict.html#PyDict_SetItemString

Signed-off-by: Joseph Schuchart <[email protected]>
Reviewed-by: Tom Zanussi <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Tom Zanussi <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
.../util/scripting-engines/trace-event-python.c | 37 ++++++++++++++--------
1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index cc75a3cef388..95d91a0b23af 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -56,6 +56,17 @@ static void handler_call_die(const char *handler_name)
Py_FatalError("problem in Python trace event handler");
}

+/*
+ * Insert val into into the dictionary and decrement the reference counter.
+ * This is necessary for dictionaries since PyDict_SetItemString() does not
+ * steal a reference, as opposed to PyTuple_SetItem().
+ */
+static void pydict_set_item_string_decref(PyObject *dict, const char *key, PyObject *val)
+{
+ PyDict_SetItemString(dict, key, val);
+ Py_DECREF(val);
+}
+
static void define_value(enum print_arg_type field_type,
const char *ev_name,
const char *field_name,
@@ -279,11 +290,11 @@ static void python_process_tracepoint(union perf_event *perf_event
PyTuple_SetItem(t, n++, PyInt_FromLong(pid));
PyTuple_SetItem(t, n++, PyString_FromString(comm));
} else {
- PyDict_SetItemString(dict, "common_cpu", PyInt_FromLong(cpu));
- PyDict_SetItemString(dict, "common_s", PyInt_FromLong(s));
- PyDict_SetItemString(dict, "common_ns", PyInt_FromLong(ns));
- PyDict_SetItemString(dict, "common_pid", PyInt_FromLong(pid));
- PyDict_SetItemString(dict, "common_comm", PyString_FromString(comm));
+ pydict_set_item_string_decref(dict, "common_cpu", PyInt_FromLong(cpu));
+ pydict_set_item_string_decref(dict, "common_s", PyInt_FromLong(s));
+ pydict_set_item_string_decref(dict, "common_ns", PyInt_FromLong(ns));
+ pydict_set_item_string_decref(dict, "common_pid", PyInt_FromLong(pid));
+ pydict_set_item_string_decref(dict, "common_comm", PyString_FromString(comm));
}
for (field = event->format.fields; field; field = field->next) {
if (field->flags & FIELD_IS_STRING) {
@@ -313,7 +324,7 @@ static void python_process_tracepoint(union perf_event *perf_event
if (handler)
PyTuple_SetItem(t, n++, obj);
else
- PyDict_SetItemString(dict, field->name, obj);
+ pydict_set_item_string_decref(dict, field->name, obj);

}
if (!handler)
@@ -370,21 +381,21 @@ static void python_process_general_event(union perf_event *perf_event
if (!handler || !PyCallable_Check(handler))
goto exit;

- PyDict_SetItemString(dict, "ev_name", PyString_FromString(perf_evsel__name(evsel)));
- PyDict_SetItemString(dict, "attr", PyString_FromStringAndSize(
+ pydict_set_item_string_decref(dict, "ev_name", PyString_FromString(perf_evsel__name(evsel)));
+ pydict_set_item_string_decref(dict, "attr", PyString_FromStringAndSize(
(const char *)&evsel->attr, sizeof(evsel->attr)));
- PyDict_SetItemString(dict, "sample", PyString_FromStringAndSize(
+ pydict_set_item_string_decref(dict, "sample", PyString_FromStringAndSize(
(const char *)sample, sizeof(*sample)));
- PyDict_SetItemString(dict, "raw_buf", PyString_FromStringAndSize(
+ pydict_set_item_string_decref(dict, "raw_buf", PyString_FromStringAndSize(
(const char *)sample->raw_data, sample->raw_size));
- PyDict_SetItemString(dict, "comm",
+ pydict_set_item_string_decref(dict, "comm",
PyString_FromString(thread->comm));
if (al->map) {
- PyDict_SetItemString(dict, "dso",
+ pydict_set_item_string_decref(dict, "dso",
PyString_FromString(al->map->dso->name));
}
if (al->sym) {
- PyDict_SetItemString(dict, "symbol",
+ pydict_set_item_string_decref(dict, "symbol",
PyString_FromString(al->sym->name));
}

--
1.8.1.4

2013-10-28 14:59:30

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/2] perf/urgent fixes


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> From: Arnaldo Carvalho de Melo <[email protected]>
>
> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit e4f8eaad70ea186b8da290c99239dce721a34f88:
>
> Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2013-10-20 10:51:35 +0200)
>
> are available in the git repository at:
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
>
> for you to fetch changes up to 2fd869f08aec5a8e4cbf01bc3fa345c4e53342d7:
>
> perf tools: Fix up /proc/PID/maps parsing (2013-10-28 09:38:12 -0300)
>
> ----------------------------------------------------------------
> perf/urgent fixes:
>
> . Fix up /proc/PID/maps parsing, where perfectly fine mmap entries
> were being trown away when synthesizing PERF_RECORD_MMAP for
> preexisting threads, prevenging symbol resolution to work
> for those threads, broken in the MMAP2 removal. Reported and
> pinpointed by Markus Trippelsdorf,
>
> . Fix mem leak in the python 'perf script' backend, due to missing Py_DECREFs
> on dict entries, fix from Joseph Schuchart.
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (1):
> perf tools: Fix up /proc/PID/maps parsing
>
> Joseph Schuchart (1):
> perf script python: Fix mem leak due to missing Py_DECREFs on dict entries
>
> tools/perf/util/event.c | 2 +-
> .../util/scripting-engines/trace-event-python.c | 37 ++++++++++++++--------
> 2 files changed, 25 insertions(+), 14 deletions(-)

Pulled, thanks Arnaldo!

Ingo