Good morning,
We have developed a patch for the perf Python scripting interface to
provide additional information about the pid, tid, and cpu of generic
events as well as information about the call-stack and resolved symbol
names. This provides scripts with a greater level of detail. The
mentioned information is already available to the scripting engine and
just has to be handed down. This is done by the attached patch. The
patch is based on Linux-3.13.3.
Please let me know if you have any questions on this.
Thanks
Joseph
--
Dipl. Inf. Joseph Schuchart
Computer Scientist
Technische Universit?t Dresden
Center for Information Services and High Performance Computing (ZIH)
01062 Dresden, Germany
Phone: +49 351 463-36494
Fax: +49 351 463-3773
E-Mail: [email protected]
On 07.03.2014 15:18, Arnaldo Carvalho de Melo wrote:
> Can you please resend, against the perf/core branch in
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git, and as an
> attachement or making sure that the patch is not mangled?
Arnaldo,
Please find attached our changes. I am sending you two patches since I
came across possible memory leaks while working on the original patch.
The first patch (perf_python_retval_decref.patch) adds calls to
Py_DECREF for the references returned by PyObject_CallObject().
The second patch (perf_python_add_sample.patch) contains our changes to
hand down the sample information for generic events. In addition, the
call-chain of the samples are constructed into a list and passed on. In
the case of generic events, this is just another entry in the dictionary
that is passed to the script as sole argument. For tracepoint events,
this adds another argument and hence changes the scripting interface.
Please feel free to remove these lines in python_process_tracepoint() if
you think that this is problematic.
I hope it is no problem that I am sending you two patches in one mail.
The patches are based on the git repository you pointed out, last
updated on April 2nd (commit 675b44bdf5f2a245f4479c5a8c40abf591007f36).
Please let me know if you have any questions.
Thanks,
Joseph
Hi Namhyung,
Thanks for your valuable feedback. Sorry for having sent the patches not inlined
again, I guess I was relying too much on Thunderbird to inline them. I tried to
address your points below and will send the new patch set in separate mails.
Thanks,
Joseph
On 29.05.2014 08:01, Namhyung Kim wrote:
> Hi Joseph,
>
> Sorry for late review, this looks very useful.. But please send a
> separate email for each patch and make it inlined (not attached) in the
> next version.
>
>
> On Thu, 03 Apr 2014 10:57:39 +0200, Joseph Schuchart wrote:
>
> [SNIP]
>> static void python_process_tracepoint(struct perf_sample *sample,
>> struct perf_evsel *evsel,
>> struct thread *thread,
>> struct addr_location *al)
>> {
>> - PyObject *handler, *retval, *context, *t, *obj, *dict = NULL;
>> + PyObject *handler, *retval, *context, *t, *obj, *callchain;
>> + PyObject *dict = NULL;
>> static char handler_name[256];
>> struct format_field *field;
>> unsigned long long val;
>> @@ -327,6 +406,14 @@ static void python_process_tracepoint(struct perf_sample *sample,
>> pydict_set_item_string_decref(dict, field->name, obj);
>>
>> }
>> +
>> + /* ip unwinding */
>> + callchain = python_process_callchain(sample, evsel, al);
>> + if (handler)
>> + PyTuple_SetItem(t, n++, callchain);
>> + else
>> + pydict_set_item_string_decref(dict, "callchain", callchain);
>
> But this makes the script not runnable anymore due to argument number
> mismatch:
>
> $ perf script -s perf-script.py
> ...
> TypeError: sched__sched_wakeup() takes exactly 12 arguments (13 given)
> Fatal Python error: problem in Python trace event handler
> Aborted (core dumped)
>
>
> So you need to change to pass callchain when generating the handler.
>
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf
> index 255d45177613..94e395c9a875 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -716,7 +716,7 @@ static int python_generate_script(struct pevent *pevent, con
>
> fprintf(ofp, "%s", f->name);
> }
> - fprintf(ofp, "):\n");
> + fprintf(ofp, ", callchain):\n");
>
> fprintf(ofp, "\t\tprint_header(event_name, common_cpu, "
> "common_secs, common_nsecs,\n\t\t\t"
>
>
> Also I think it's very useful to generate code to print callchain by
> default if exists - something like below?
>
> for node in callchain:
> if 'sym' in node:
> print "\t[%x] %s" % (node['ip'], node['sym']['name'])
> else:
> print "\t[%x]" % (node['ip'])
>
>
> $ perf script -s perf-script.py
> in trace_begin
> sched__sched_wakeup 8 5021904.056096444 19713 :19713 comm=perf, pid=19714, prio=120, success=1, target_cpu=9
> [ffffffff81091512] ttwu_do_wakeup
> [ffffffff810916d6] ttwu_do_activate.constprop.87
> [ffffffff81093b64] try_to_wake_up
> [ffffffff81093c22] default_wake_function
> [ffffffff8108348d] autoremove_wake_function
> [ffffffff8108b215] __wake_up_common
> [ffffffff8108e933] __wake_up_sync_key
> [ffffffff811a5b20] pipe_write
> [ffffffff8119cc07] do_sync_write
> [ffffffff8119d2cc] vfs_write
> [ffffffff8119d762] sys_write
> [ffffffff816640d9] system_call_fastpath
> sched__sched_wakeup 8 5021904.056100334 19713 :19713 comm=perf, pid=19714, prio=120, success=1, target_cpu=9
> [ffffffff81091512] ttwu_do_wakeup
> [ffffffff810916d6] ttwu_do_activate.constprop.87
> [ffffffff81093b64] try_to_wake_up
> [ffffffff81093c22] default_wake_function
> [ffffffff8108348d] autoremove_wake_function
> [ffffffff8108b215] __wake_up_common
> [ffffffff8108e933] __wake_up_sync_key
> [ffffffff811a5b20] pipe_write
> [ffffffff8119cc07] do_sync_write
> [ffffffff8119d2cc] vfs_write
> [ffffffff8119d762] sys_write
> [ffffffff816640d9] system_call_fastpath
> ...
>
>
>> +
>> if (!handler)
>> PyTuple_SetItem(t, n++, dict);
>>
>
> [SNIP]
>> @@ -385,15 +476,30 @@ static void python_process_general_event(struct perf_sample *sample,
>> 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_set_item_string_decref(dict, "sample", PyString_FromStringAndSize(
>> - (const char *)sample, sizeof(*sample)));
>> +
>> + pydict_set_item_string_decref(dict_sample, "pid",
>> + PyInt_FromLong(sample->pid));
>> + pydict_set_item_string_decref(dict_sample, "tid",
>> + PyInt_FromLong(sample->tid));
>> + pydict_set_item_string_decref(dict_sample, "cpu",
>> + PyInt_FromLong(sample->cpu));
>> + pydict_set_item_string_decref(dict_sample, "time",
>> + PyLong_FromUnsignedLongLong(sample->time));
>> + pydict_set_item_string_decref(dict_sample, "period",
>> + PyLong_FromUnsignedLongLong(sample->period));
>> + pydict_set_item_string_decref(dict, "sample", dict_sample);
>
> And I think this part should be a separate patch.
>
>> +
>> + /* ip unwinding */
>> + callchain = python_process_callchain(sample, evsel, al);
>> + pydict_set_item_string_decref(dict, "callchain", callchain);
>> +
>> pydict_set_item_string_decref(dict, "raw_buf", PyString_FromStringAndSize(
>> (const char *)sample->raw_data, sample->raw_size));
>> pydict_set_item_string_decref(dict, "comm",
>> PyString_FromString(thread__comm_str(thread)));
>> if (al->map) {
>> pydict_set_item_string_decref(dict, "dso",
>> - PyString_FromString(al->map->dso->name));
>> + PyString_FromString(al->map->dso->name));
>
> It seems like an unnecessary change.
>
>
>> }
>> if (al->sym) {
>> pydict_set_item_string_decref(dict, "symbol",
>
>
>
>>
>> Perf: Fix possible memory leaks in Python interface
>>
>> The function PyObject_CallObject() returns a new PyObject reference
>> on which Py_DECREF has to be called to avoid memory leaks.
>> This patch adds these calls where necessary.
>>
>> Signed-off-by: Joseph Schuchart <[email protected]>
>>
>> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
>> index cd9774d..ee17f64 100644
>> --- a/tools/perf/util/scripting-engines/trace-event-python.c
>> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
>> @@ -97,6 +97,8 @@ static void define_value(enum print_arg_type field_type,
>> retval = PyObject_CallObject(handler, t);
>> if (retval == NULL)
>> handler_call_die(handler_name);
>> + else
>> + Py_DECREF(retval);
>
> It looks like the handler_call_die() is never returned. So we can get
> rid of the 'else' (like in the last hunk) for simplicity.
>
> Thanks,
> Namhyung
>
>
>> }
>>
>> Py_DECREF(t);
>> @@ -143,6 +145,8 @@ static void define_field(enum print_arg_type field_type,
>> retval = PyObject_CallObject(handler, t);
>> if (retval == NULL)
>> handler_call_die(handler_name);
>> + else
>> + Py_DECREF(retval);
>> }
>>
>> Py_DECREF(t);
>> @@ -333,6 +337,8 @@ static void python_process_tracepoint(struct perf_sample *sample,
>> retval = PyObject_CallObject(handler, t);
>> if (retval == NULL)
>> handler_call_die(handler_name);
>> + else
>> + Py_DECREF(retval);
>> } else {
>> handler = PyDict_GetItemString(main_dict, "trace_unhandled");
>> if (handler && PyCallable_Check(handler)) {
>> @@ -340,6 +346,8 @@ static void python_process_tracepoint(struct perf_sample *sample,
>> retval = PyObject_CallObject(handler, t);
>> if (retval == NULL)
>> handler_call_die("trace_unhandled");
>> + else
>> + Py_DECREF(retval);
>> }
>> Py_DECREF(dict);
>> }
>> @@ -399,6 +407,8 @@ static void python_process_general_event(struct perf_sample *sample,
>> retval = PyObject_CallObject(handler, t);
>> if (retval == NULL)
>> handler_call_die(handler_name);
>> + else
>> + Py_DECREF(retval);
>> exit:
>> Py_DECREF(dict);
>> Py_DECREF(t);
>> @@ -444,8 +454,8 @@ static int run_start_sub(void)
>> retval = PyObject_CallObject(handler, NULL);
>> if (retval == NULL)
>> handler_call_die("trace_begin");
>> -
>> - Py_DECREF(retval);
>> + else
>> + Py_DECREF(retval);
>> return err;
>> error:
>> Py_XDECREF(main_dict);