This patch just follows Robert Richter's idea and the commit 37a058ea0
"perf script: Add generic perl handler to process events"
to similarly add a python handler for general events other than tracepoints.
For non-tracepoint events, this patch will try to find a function named
"process_general_event" in the python script, and pass the event
header, attribute, perf_sample, raw_data in format of raw string. And
the python script can use "struct" module's unpack function to disasemble
the needed info and process.
Signed-off-by: Feng Tang <[email protected]>
---
.../util/scripting-engines/trace-event-python.c | 60 +++++++++++++++++++-
1 files changed, 59 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index c2623c6..aaf2679 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -31,6 +31,7 @@
#include "../event.h"
#include "../thread.h"
#include "../trace-event.h"
+#include "../evsel.h"
PyMODINIT_FUNC initperf_trace_context(void);
@@ -205,7 +206,7 @@ static inline struct event *find_cache_event(int type)
return event;
}
-static void python_process_event(union perf_event *pevent __unused,
+static void python_process_tracepoint(union perf_event *pevent __unused,
struct perf_sample *sample,
struct perf_evsel *evsel __unused,
struct machine *machine __unused,
@@ -324,6 +325,63 @@ static void python_process_event(union perf_event *pevent __unused,
Py_DECREF(t);
}
+static void python_process_general_event(union perf_event *pevent __unused,
+ struct perf_sample *sample,
+ struct perf_evsel *evsel __unused,
+ struct machine *machine __unused,
+ struct thread *thread __unused)
+{
+ PyObject *handler, *retval, *t;
+ static char handler_name[64];
+ unsigned n = 0;
+ void *data = sample->raw_data;
+
+ t = PyTuple_New(MAX_FIELDS);
+ if (!t)
+ Py_FatalError("couldn't create Python tuple");
+
+ sprintf(handler_name, "process_general_event");
+
+ handler = PyDict_GetItemString(main_dict, handler_name);
+ if (handler && !PyCallable_Check(handler)) {
+ handler = NULL;
+ goto exit;
+ }
+
+ /* Pass 3 parameters: event_attr, perf_sample, raw data */
+ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
+ (const char *)&evsel->attr, sizeof(evsel->attr)));
+ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
+ (const char *)sample, sizeof(*sample)));
+ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
+ data, sample->raw_size));
+
+ if (_PyTuple_Resize(&t, n) == -1)
+ Py_FatalError("error resizing Python tuple");
+
+ retval = PyObject_CallObject(handler, t);
+ if (retval == NULL)
+ handler_call_die(handler_name);
+exit:
+ Py_DECREF(t);
+}
+
+static void python_process_event(union perf_event *pevent,
+ struct perf_sample *sample,
+ struct perf_evsel *evsel,
+ struct machine *machine,
+ struct thread *thread)
+{
+ switch (evsel->attr.type) {
+ case PERF_TYPE_TRACEPOINT:
+ python_process_tracepoint(pevent, sample, evsel, machine, thread);
+ break;
+ /* Reserve for future process_hw/sw/raw APIs */
+ default:
+ python_process_general_event(pevent, sample, evsel, machine, thread);
+ }
+}
+
static int run_start_sub(void)
{
PyObject *handler, *retval;
--
1.7.1
Both perl and python script start processing events other than trace
points, and it's useful to pass the resolved symbol and the dso info
to the event handler in script for better analysis and statistics.
Struct thread is already a member of struct addr_location, using
addr_location will keep the thread info, while providing additional
symbol and dso info if exist, so that the script itself doesn't need
to bother to do the symbol resolving and dso searching work.
Signed-off-by: Feng Tang <[email protected]>
---
tools/perf/builtin-script.c | 5 +++--
.../perf/util/scripting-engines/trace-event-perl.c | 11 ++++++-----
.../util/scripting-engines/trace-event-python.c | 16 ++++++++--------
tools/perf/util/trace-event-scripting.c | 2 +-
tools/perf/util/trace-event.h | 4 +++-
5 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index d4ce733..417fe7d 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -405,9 +405,10 @@ static void process_event(union perf_event *event __unused,
struct perf_sample *sample,
struct perf_evsel *evsel,
struct machine *machine,
- struct thread *thread)
+ struct addr_location *al __unused)
{
struct perf_event_attr *attr = &evsel->attr;
+ struct thread *thread = al->thread;
if (output[attr->type].fields == 0)
return;
@@ -520,7 +521,7 @@ static int process_sample_event(struct perf_tool *tool __used,
if (cpu_list && !test_bit(sample->cpu, cpu_bitmap))
return 0;
- scripting_ops->process_event(event, sample, evsel, machine, thread);
+ scripting_ops->process_event(event, sample, evsel, machine, &al);
evsel->hists.stats.total_period += sample->period;
return 0;
diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index e30749e..8f5eacc 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -252,7 +252,7 @@ static void perl_process_tracepoint(union perf_event *pevent __unused,
struct perf_sample *sample,
struct perf_evsel *evsel,
struct machine *machine __unused,
- struct thread *thread)
+ struct addr_location *al __unused)
{
struct format_field *field;
static char handler[256];
@@ -264,6 +264,7 @@ static void perl_process_tracepoint(union perf_event *pevent __unused,
int cpu = sample->cpu;
void *data = sample->raw_data;
unsigned long long nsecs = sample->time;
+ struct thread *thread = al->thread;
char *comm = thread->comm;
dSP;
@@ -342,7 +343,7 @@ static void perl_process_event_generic(union perf_event *pevent __unused,
struct perf_sample *sample,
struct perf_evsel *evsel __unused,
struct machine *machine __unused,
- struct thread *thread __unused)
+ struct addr_location *al __unused)
{
dSP;
@@ -368,10 +369,10 @@ static void perl_process_event(union perf_event *pevent,
struct perf_sample *sample,
struct perf_evsel *evsel,
struct machine *machine,
- struct thread *thread)
+ struct addr_location *al)
{
- perl_process_tracepoint(pevent, sample, evsel, machine, thread);
- perl_process_event_generic(pevent, sample, evsel, machine, thread);
+ perl_process_tracepoint(pevent, sample, evsel, machine, al);
+ perl_process_event_generic(pevent, sample, evsel, machine, al);
}
static void run_start_sub(void)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index aaf2679..a202881 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -210,7 +210,7 @@ static void python_process_tracepoint(union perf_event *pevent __unused,
struct perf_sample *sample,
struct perf_evsel *evsel __unused,
struct machine *machine __unused,
- struct thread *thread)
+ struct addr_location *al)
{
PyObject *handler, *retval, *context, *t, *obj, *dict = NULL;
static char handler_name[256];
@@ -224,6 +224,7 @@ static void python_process_tracepoint(union perf_event *pevent __unused,
int cpu = sample->cpu;
void *data = sample->raw_data;
unsigned long long nsecs = sample->time;
+ struct thread *thread = al->thread;
char *comm = thread->comm;
t = PyTuple_New(MAX_FIELDS);
@@ -327,9 +328,9 @@ static void python_process_tracepoint(union perf_event *pevent __unused,
static void python_process_general_event(union perf_event *pevent __unused,
struct perf_sample *sample,
- struct perf_evsel *evsel __unused,
+ struct perf_evsel *evsel,
struct machine *machine __unused,
- struct thread *thread __unused)
+ struct addr_location *al)
{
PyObject *handler, *retval, *t;
static char handler_name[64];
@@ -341,14 +342,13 @@ static void python_process_general_event(union perf_event *pevent __unused,
Py_FatalError("couldn't create Python tuple");
sprintf(handler_name, "process_general_event");
-
handler = PyDict_GetItemString(main_dict, handler_name);
if (handler && !PyCallable_Check(handler)) {
handler = NULL;
goto exit;
}
- /* Pass 3 parameters: event_attr, perf_sample, raw data */
+ /* Pass 3 parameters: event_attr, perf_sample, raw data, thread name */
PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
(const char *)&evsel->attr, sizeof(evsel->attr)));
PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
@@ -370,15 +370,15 @@ static void python_process_event(union perf_event *pevent,
struct perf_sample *sample,
struct perf_evsel *evsel,
struct machine *machine,
- struct thread *thread)
+ struct addr_location *al)
{
switch (evsel->attr.type) {
case PERF_TYPE_TRACEPOINT:
- python_process_tracepoint(pevent, sample, evsel, machine, thread);
+ python_process_tracepoint(pevent, sample, evsel, machine, al);
break;
/* Reserve for future process_hw/sw/raw APIs */
default:
- python_process_general_event(pevent, sample, evsel, machine, thread);
+ python_process_general_event(pevent, sample, evsel, machine, al);
}
}
diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
index 18ae6c1..b26459e 100644
--- a/tools/perf/util/trace-event-scripting.c
+++ b/tools/perf/util/trace-event-scripting.c
@@ -39,7 +39,7 @@ static void process_event_unsupported(union perf_event *event __unused,
struct perf_sample *sample __unused,
struct perf_evsel *evsel __unused,
struct machine *machine __unused,
- struct thread *thread __unused)
+ struct addr_location *al __unused)
{
}
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 58ae14c..dc20ca0 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -289,6 +289,8 @@ enum trace_flag_type {
TRACE_FLAG_SOFTIRQ = 0x10,
};
+struct addr_location;
+
struct scripting_ops {
const char *name;
int (*start_script) (const char *script, int argc, const char **argv);
@@ -297,7 +299,7 @@ struct scripting_ops {
struct perf_sample *sample,
struct perf_evsel *evsel,
struct machine *machine,
- struct thread *thread);
+ struct addr_location *al);
int (*generate_script) (const char *outfile);
};
--
1.7.1
Signed-off-by: Feng Tang <[email protected]>
---
.../util/scripting-engines/trace-event-python.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index a202881..c96f5e2 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -336,6 +336,7 @@ static void python_process_general_event(union perf_event *pevent __unused,
static char handler_name[64];
unsigned n = 0;
void *data = sample->raw_data;
+ struct thread *thread = al->thread;
t = PyTuple_New(MAX_FIELDS);
if (!t)
@@ -348,13 +349,21 @@ static void python_process_general_event(union perf_event *pevent __unused,
goto exit;
}
- /* Pass 3 parameters: event_attr, perf_sample, raw data, thread name */
+ /* Pass parameters: attr, perf_sample, raw data, thread and dso name */
PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
(const char *)&evsel->attr, sizeof(evsel->attr)));
PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
(const char *)sample, sizeof(*sample)));
PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
data, sample->raw_size));
+ PyTuple_SetItem(t, n++, PyString_FromString(thread->comm));
+ PyTuple_SetItem(t, n++, PyString_FromString(al->map->dso->name));
+
+ /* Pass the resolved symbol if there is, othersize pass "Unkown" */
+ if (al->sym)
+ PyTuple_SetItem(t, n++, PyString_FromString(al->sym->name));
+ else
+ PyTuple_SetItem(t, n++, PyString_FromString("Unknown"));
if (_PyTuple_Resize(&t, n) == -1)
Py_FatalError("error resizing Python tuple");
--
1.7.1
Em Wed, May 16, 2012 at 08:59:13PM +0800, Feng Tang escreveu:
> This patch just follows Robert Richter's idea and the commit 37a058ea0
> "perf script: Add generic perl handler to process events"
> to similarly add a python handler for general events other than tracepoints.
>
> For non-tracepoint events, this patch will try to find a function named
> "process_general_event" in the python script, and pass the event
But in perl isn't it named "process_event"? Can't we use the same
convention in the python case?
> header, attribute, perf_sample, raw_data in format of raw string. And
> the python script can use "struct" module's unpack function to disasemble
> the needed info and process.
>
> Signed-off-by: Feng Tang <[email protected]>
> ---
> .../util/scripting-engines/trace-event-python.c | 60 +++++++++++++++++++-
> 1 files changed, 59 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index c2623c6..aaf2679 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -324,6 +325,63 @@ static void python_process_event(union perf_event *pevent __unused,
> Py_DECREF(t);
> }
>
> +static void python_process_general_event(union perf_event *pevent __unused,
> + struct perf_sample *sample,
> + struct perf_evsel *evsel __unused,
> + struct machine *machine __unused,
> + struct thread *thread __unused)
> +{
> + PyObject *handler, *retval, *t;
> + static char handler_name[64];
> + unsigned n = 0;
> + void *data = sample->raw_data;
> +
> + t = PyTuple_New(MAX_FIELDS);
> + if (!t)
> + Py_FatalError("couldn't create Python tuple");
> +
> + sprintf(handler_name, "process_general_event");
Strange use of sprintf, if you think it is safe to not bounds check, use
strcpy(), if you still want to use sprintf, please instead use:
snprintf(handler_name, sizeof(handler_name), "%s", "process_event");
> + handler = PyDict_GetItemString(main_dict, handler_name);
> + if (handler && !PyCallable_Check(handler)) {
> + handler = NULL;
> + goto exit;
> + }
> +
> + /* Pass 3 parameters: event_attr, perf_sample, raw data */
> + PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> + (const char *)&evsel->attr, sizeof(evsel->attr)));
> + PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> + (const char *)sample, sizeof(*sample)));
> + PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> + data, sample->raw_size));
> +
> + if (_PyTuple_Resize(&t, n) == -1)
> + Py_FatalError("error resizing Python tuple");
> +
> + retval = PyObject_CallObject(handler, t);
> + if (retval == NULL)
> + handler_call_die(handler_name);
> +exit:
> + Py_DECREF(t);
> +}
> +
> +static void python_process_event(union perf_event *pevent,
> + struct perf_sample *sample,
> + struct perf_evsel *evsel,
> + struct machine *machine,
> + struct thread *thread)
> +{
> + switch (evsel->attr.type) {
> + case PERF_TYPE_TRACEPOINT:
> + python_process_tracepoint(pevent, sample, evsel, machine, thread);
> + break;
> + /* Reserve for future process_hw/sw/raw APIs */
> + default:
> + python_process_general_event(pevent, sample, evsel, machine, thread);
> + }
> +}
> +
> static int run_start_sub(void)
> {
> PyObject *handler, *retval;
> --
> 1.7.1
Em Wed, May 16, 2012 at 08:59:14PM +0800, Feng Tang escreveu:
> Both perl and python script start processing events other than trace
> points, and it's useful to pass the resolved symbol and the dso info
> to the event handler in script for better analysis and statistics.
>
> Struct thread is already a member of struct addr_location, using
> addr_location will keep the thread info, while providing additional
> symbol and dso info if exist, so that the script itself doesn't need
> to bother to do the symbol resolving and dso searching work.
This seems ok.
David, any objections or suggestions?
- Arnaldo
> Signed-off-by: Feng Tang <[email protected]>
> ---
> tools/perf/builtin-script.c | 5 +++--
> .../perf/util/scripting-engines/trace-event-perl.c | 11 ++++++-----
> .../util/scripting-engines/trace-event-python.c | 16 ++++++++--------
> tools/perf/util/trace-event-scripting.c | 2 +-
> tools/perf/util/trace-event.h | 4 +++-
> 5 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index d4ce733..417fe7d 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -405,9 +405,10 @@ static void process_event(union perf_event *event __unused,
> struct perf_sample *sample,
> struct perf_evsel *evsel,
> struct machine *machine,
> - struct thread *thread)
> + struct addr_location *al __unused)
> {
> struct perf_event_attr *attr = &evsel->attr;
> + struct thread *thread = al->thread;
>
> if (output[attr->type].fields == 0)
> return;
> @@ -520,7 +521,7 @@ static int process_sample_event(struct perf_tool *tool __used,
> if (cpu_list && !test_bit(sample->cpu, cpu_bitmap))
> return 0;
>
> - scripting_ops->process_event(event, sample, evsel, machine, thread);
> + scripting_ops->process_event(event, sample, evsel, machine, &al);
>
> evsel->hists.stats.total_period += sample->period;
> return 0;
> diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
> index e30749e..8f5eacc 100644
> --- a/tools/perf/util/scripting-engines/trace-event-perl.c
> +++ b/tools/perf/util/scripting-engines/trace-event-perl.c
> @@ -252,7 +252,7 @@ static void perl_process_tracepoint(union perf_event *pevent __unused,
> struct perf_sample *sample,
> struct perf_evsel *evsel,
> struct machine *machine __unused,
> - struct thread *thread)
> + struct addr_location *al __unused)
> {
> struct format_field *field;
> static char handler[256];
> @@ -264,6 +264,7 @@ static void perl_process_tracepoint(union perf_event *pevent __unused,
> int cpu = sample->cpu;
> void *data = sample->raw_data;
> unsigned long long nsecs = sample->time;
> + struct thread *thread = al->thread;
> char *comm = thread->comm;
>
> dSP;
> @@ -342,7 +343,7 @@ static void perl_process_event_generic(union perf_event *pevent __unused,
> struct perf_sample *sample,
> struct perf_evsel *evsel __unused,
> struct machine *machine __unused,
> - struct thread *thread __unused)
> + struct addr_location *al __unused)
> {
> dSP;
>
> @@ -368,10 +369,10 @@ static void perl_process_event(union perf_event *pevent,
> struct perf_sample *sample,
> struct perf_evsel *evsel,
> struct machine *machine,
> - struct thread *thread)
> + struct addr_location *al)
> {
> - perl_process_tracepoint(pevent, sample, evsel, machine, thread);
> - perl_process_event_generic(pevent, sample, evsel, machine, thread);
> + perl_process_tracepoint(pevent, sample, evsel, machine, al);
> + perl_process_event_generic(pevent, sample, evsel, machine, al);
> }
>
> static void run_start_sub(void)
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index aaf2679..a202881 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -210,7 +210,7 @@ static void python_process_tracepoint(union perf_event *pevent __unused,
> struct perf_sample *sample,
> struct perf_evsel *evsel __unused,
> struct machine *machine __unused,
> - struct thread *thread)
> + struct addr_location *al)
> {
> PyObject *handler, *retval, *context, *t, *obj, *dict = NULL;
> static char handler_name[256];
> @@ -224,6 +224,7 @@ static void python_process_tracepoint(union perf_event *pevent __unused,
> int cpu = sample->cpu;
> void *data = sample->raw_data;
> unsigned long long nsecs = sample->time;
> + struct thread *thread = al->thread;
> char *comm = thread->comm;
>
> t = PyTuple_New(MAX_FIELDS);
> @@ -327,9 +328,9 @@ static void python_process_tracepoint(union perf_event *pevent __unused,
>
> static void python_process_general_event(union perf_event *pevent __unused,
> struct perf_sample *sample,
> - struct perf_evsel *evsel __unused,
> + struct perf_evsel *evsel,
> struct machine *machine __unused,
> - struct thread *thread __unused)
> + struct addr_location *al)
> {
> PyObject *handler, *retval, *t;
> static char handler_name[64];
> @@ -341,14 +342,13 @@ static void python_process_general_event(union perf_event *pevent __unused,
> Py_FatalError("couldn't create Python tuple");
>
> sprintf(handler_name, "process_general_event");
> -
> handler = PyDict_GetItemString(main_dict, handler_name);
> if (handler && !PyCallable_Check(handler)) {
> handler = NULL;
> goto exit;
> }
>
> - /* Pass 3 parameters: event_attr, perf_sample, raw data */
> + /* Pass 3 parameters: event_attr, perf_sample, raw data, thread name */
> PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> (const char *)&evsel->attr, sizeof(evsel->attr)));
> PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> @@ -370,15 +370,15 @@ static void python_process_event(union perf_event *pevent,
> struct perf_sample *sample,
> struct perf_evsel *evsel,
> struct machine *machine,
> - struct thread *thread)
> + struct addr_location *al)
> {
> switch (evsel->attr.type) {
> case PERF_TYPE_TRACEPOINT:
> - python_process_tracepoint(pevent, sample, evsel, machine, thread);
> + python_process_tracepoint(pevent, sample, evsel, machine, al);
> break;
> /* Reserve for future process_hw/sw/raw APIs */
> default:
> - python_process_general_event(pevent, sample, evsel, machine, thread);
> + python_process_general_event(pevent, sample, evsel, machine, al);
> }
> }
>
> diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
> index 18ae6c1..b26459e 100644
> --- a/tools/perf/util/trace-event-scripting.c
> +++ b/tools/perf/util/trace-event-scripting.c
> @@ -39,7 +39,7 @@ static void process_event_unsupported(union perf_event *event __unused,
> struct perf_sample *sample __unused,
> struct perf_evsel *evsel __unused,
> struct machine *machine __unused,
> - struct thread *thread __unused)
> + struct addr_location *al __unused)
> {
> }
>
> diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
> index 58ae14c..dc20ca0 100644
> --- a/tools/perf/util/trace-event.h
> +++ b/tools/perf/util/trace-event.h
> @@ -289,6 +289,8 @@ enum trace_flag_type {
> TRACE_FLAG_SOFTIRQ = 0x10,
> };
>
> +struct addr_location;
> +
> struct scripting_ops {
> const char *name;
> int (*start_script) (const char *script, int argc, const char **argv);
> @@ -297,7 +299,7 @@ struct scripting_ops {
> struct perf_sample *sample,
> struct perf_evsel *evsel,
> struct machine *machine,
> - struct thread *thread);
> + struct addr_location *al);
> int (*generate_script) (const char *outfile);
> };
>
> --
> 1.7.1
Em Wed, May 16, 2012 at 08:59:15PM +0800, Feng Tang escreveu:
> Signed-off-by: Feng Tang <[email protected]>
> ---
> .../util/scripting-engines/trace-event-python.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index a202881..c96f5e2 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -336,6 +336,7 @@ static void python_process_general_event(union perf_event *pevent __unused,
> static char handler_name[64];
> unsigned n = 0;
> void *data = sample->raw_data;
> + struct thread *thread = al->thread;
>
> t = PyTuple_New(MAX_FIELDS);
> if (!t)
> @@ -348,13 +349,21 @@ static void python_process_general_event(union perf_event *pevent __unused,
> goto exit;
> }
>
> - /* Pass 3 parameters: event_attr, perf_sample, raw data, thread name */
> + /* Pass parameters: attr, perf_sample, raw data, thread and dso name */
> PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> (const char *)&evsel->attr, sizeof(evsel->attr)));
> PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> (const char *)sample, sizeof(*sample)));
> PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> data, sample->raw_size));
> + PyTuple_SetItem(t, n++, PyString_FromString(thread->comm));
> + PyTuple_SetItem(t, n++, PyString_FromString(al->map->dso->name));
> +
> + /* Pass the resolved symbol if there is, othersize pass "Unkown" */
> + if (al->sym)
> + PyTuple_SetItem(t, n++, PyString_FromString(al->sym->name));
> + else
> + PyTuple_SetItem(t, n++, PyString_FromString("Unknown"));
Isn't this getting a little bit convoluted?
I.e. python has dictionaries, perhaps we could pass a dict instead of a
tuple, in that case we would simply not add the "symbol" key.
> if (_PyTuple_Resize(&t, n) == -1)
> Py_FatalError("error resizing Python tuple");
> --
> 1.7.1
On 5/17/12 9:45 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 16, 2012 at 08:59:14PM +0800, Feng Tang escreveu:
>> Both perl and python script start processing events other than trace
>> points, and it's useful to pass the resolved symbol and the dso info
>> to the event handler in script for better analysis and statistics.
>>
>> Struct thread is already a member of struct addr_location, using
>> addr_location will keep the thread info, while providing additional
>> symbol and dso info if exist, so that the script itself doesn't need
>> to bother to do the symbol resolving and dso searching work.
>
> This seems ok.
>
> David, any objections or suggestions?
perf_event__preprocess_sample calls thread__find_addr_map which sets
al->thread and the preprocess_sample is invoked prior to process_event,
so it should be fine.
That said, I would like to test it. Unfortunately I am knee deep
debugging some local perf breakage. Give me a couple of days to get back
to this.
David
Em Thu, May 17, 2012 at 10:08:16AM -0600, David Ahern escreveu:
> On 5/17/12 9:45 AM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, May 16, 2012 at 08:59:14PM +0800, Feng Tang escreveu:
> >>Both perl and python script start processing events other than trace
> >>points, and it's useful to pass the resolved symbol and the dso info
> >>to the event handler in script for better analysis and statistics.
> >>
> >>Struct thread is already a member of struct addr_location, using
> >>addr_location will keep the thread info, while providing additional
> >>symbol and dso info if exist, so that the script itself doesn't need
> >>to bother to do the symbol resolving and dso searching work.
> >
> >This seems ok.
> >
> >David, any objections or suggestions?
>
> perf_event__preprocess_sample calls thread__find_addr_map which sets
> al->thread and the preprocess_sample is invoked prior to
> process_event, so it should be fine.
>
> That said, I would like to test it. Unfortunately I am knee deep
> debugging some local perf breakage. Give me a couple of days to get
> back to this.
Thanks!
- Arnaldo
Hi Arnaldo,
Thanks for your review!
On Thu, 17 May 2012 12:43:15 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, May 16, 2012 at 08:59:13PM +0800, Feng Tang escreveu:
> > This patch just follows Robert Richter's idea and the commit 37a058ea0
> > "perf script: Add generic perl handler to process events"
> > to similarly add a python handler for general events other than tracepoints.
> >
> > For non-tracepoint events, this patch will try to find a function named
> > "process_general_event" in the python script, and pass the event
>
> But in perl isn't it named "process_event"? Can't we use the same
> convention in the python case?
Good point, will comply with the perl code.
>
> > header, attribute, perf_sample, raw_data in format of raw string. And
> > the python script can use "struct" module's unpack function to disasemble
> > the needed info and process.
> >
> > Signed-off-by: Feng Tang <[email protected]>
> > ---
> > .../util/scripting-engines/trace-event-python.c | 60
> > +++++++++++++++++++- 1 files changed, 59 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/perf/util/scripting-engines/trace-event-python.c
> > b/tools/perf/util/scripting-engines/trace-event-python.c index
> > c2623c6..aaf2679 100644 ---
> > a/tools/perf/util/scripting-engines/trace-event-python.c +++
> > b/tools/perf/util/scripting-engines/trace-event-python.c @@ -324,6 +325,63
> > @@ static void python_process_event(union perf_event *pevent __unused,
> > Py_DECREF(t); }
> >
> > +static void python_process_general_event(union perf_event *pevent __unused,
> > + struct perf_sample *sample,
> > + struct perf_evsel *evsel __unused,
> > + struct machine *machine __unused,
> > + struct thread *thread __unused)
> > +{
> > + PyObject *handler, *retval, *t;
> > + static char handler_name[64];
> > + unsigned n = 0;
> > + void *data = sample->raw_data;
> > +
> > + t = PyTuple_New(MAX_FIELDS);
> > + if (!t)
> > + Py_FatalError("couldn't create Python tuple");
> > +
> > + sprintf(handler_name, "process_general_event");
>
> Strange use of sprintf, if you think it is safe to not bounds check, use
> strcpy(), if you still want to use sprintf, please instead use:
>
> snprintf(handler_name, sizeof(handler_name), "%s", "process_event");
Ok, will change it. Following is the update patch:
>From 87b855a7c85438f45e1fc89250e392c1d6365ec6 Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Fri, 11 May 2012 16:58:05 +0800
Subject: [PATCH] perf script: Add general python handler to process non-tracepoint events
This patch just follows Robert Richter's idea and the commit 37a058ea0
"perf script: Add generic perl handler to process events"
to similarly add a python handler for general events other than tracepoints.
For non-tracepoint events, this patch will try to find a function named
"process_event" in the python script, and pass the event attribute,
perf_sample, raw_data in format of raw string. And the python script can
use "struct" module's unpack function to disasemble the needed info and process.
Signed-off-by: Feng Tang <[email protected]>
---
.../util/scripting-engines/trace-event-python.c | 60 +++++++++++++++++++-
1 files changed, 59 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index c2623c6..6daa12f 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -31,6 +31,7 @@
#include "../event.h"
#include "../thread.h"
#include "../trace-event.h"
+#include "../evsel.h"
PyMODINIT_FUNC initperf_trace_context(void);
@@ -205,7 +206,7 @@ static inline struct event *find_cache_event(int type)
return event;
}
-static void python_process_event(union perf_event *pevent __unused,
+static void python_process_tracepoint(union perf_event *pevent __unused,
struct perf_sample *sample,
struct perf_evsel *evsel __unused,
struct machine *machine __unused,
@@ -324,6 +325,63 @@ static void python_process_event(union perf_event *pevent __unused,
Py_DECREF(t);
}
+static void python_process_general_event(union perf_event *pevent __unused,
+ struct perf_sample *sample,
+ struct perf_evsel *evsel,
+ struct machine *machine __unused,
+ struct thread *thread __unused)
+{
+ PyObject *handler, *retval, *t;
+ static char handler_name[64];
+ unsigned n = 0;
+ void *data = sample->raw_data;
+
+ t = PyTuple_New(MAX_FIELDS);
+ if (!t)
+ Py_FatalError("couldn't create Python tuple");
+
+ snprintf(handler_name, sizeof(handler_name), "%s", "process_event");
+
+ handler = PyDict_GetItemString(main_dict, handler_name);
+ if (handler && !PyCallable_Check(handler)) {
+ handler = NULL;
+ goto exit;
+ }
+
+ /* Pass 3 parameters: event_attr, perf_sample, raw data */
+ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
+ (const char *)&evsel->attr, sizeof(evsel->attr)));
+ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
+ (const char *)sample, sizeof(*sample)));
+ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
+ data, sample->raw_size));
+
+ if (_PyTuple_Resize(&t, n) == -1)
+ Py_FatalError("error resizing Python tuple");
+
+ retval = PyObject_CallObject(handler, t);
+ if (retval == NULL)
+ handler_call_die(handler_name);
+exit:
+ Py_DECREF(t);
+}
+
+static void python_process_event(union perf_event *pevent,
+ struct perf_sample *sample,
+ struct perf_evsel *evsel,
+ struct machine *machine,
+ struct thread *thread)
+{
+ switch (evsel->attr.type) {
+ case PERF_TYPE_TRACEPOINT:
+ python_process_tracepoint(pevent, sample, evsel, machine, thread);
+ break;
+ /* Reserve for future process_hw/sw/raw APIs */
+ default:
+ python_process_general_event(pevent, sample, evsel, machine, thread);
+ }
+}
+
static int run_start_sub(void)
{
PyObject *handler, *retval;
--
1.7.1
On Thu, 17 May 2012 12:47:26 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, May 16, 2012 at 08:59:15PM +0800, Feng Tang escreveu:
> > Signed-off-by: Feng Tang <[email protected]>
> > ---
> > .../util/scripting-engines/trace-event-python.c | 11 ++++++++++-
> > 1 files changed, 10 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/perf/util/scripting-engines/trace-event-python.c
> > b/tools/perf/util/scripting-engines/trace-event-python.c index
> > a202881..c96f5e2 100644 ---
> > a/tools/perf/util/scripting-engines/trace-event-python.c +++
> > b/tools/perf/util/scripting-engines/trace-event-python.c @@ -336,6 +336,7
> > @@ static void python_process_general_event(union perf_event *pevent
> > __unused, static char handler_name[64]; unsigned n = 0;
> > void *data = sample->raw_data;
> > + struct thread *thread = al->thread;
> >
> > t = PyTuple_New(MAX_FIELDS);
> > if (!t)
> > @@ -348,13 +349,21 @@ static void python_process_general_event(union
> > perf_event *pevent __unused, goto exit;
> > }
> >
> > - /* Pass 3 parameters: event_attr, perf_sample, raw data, thread
> > name */
> > + /* Pass parameters: attr, perf_sample, raw data, thread and dso
> > name */ PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> > (const char *)&evsel->attr, sizeof(evsel->attr)));
> > PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> > (const char *)sample, sizeof(*sample)));
> > PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> > data, sample->raw_size));
> > + PyTuple_SetItem(t, n++, PyString_FromString(thread->comm));
> > + PyTuple_SetItem(t, n++, PyString_FromString(al->map->dso->name));
> > +
> > + /* Pass the resolved symbol if there is, othersize pass "Unkown" */
> > + if (al->sym)
> > + PyTuple_SetItem(t, n++,
> > PyString_FromString(al->sym->name));
> > + else
> > + PyTuple_SetItem(t, n++, PyString_FromString("Unknown"));
>
> Isn't this getting a little bit convoluted?
>
> I.e. python has dictionaries, perhaps we could pass a dict instead of a
> tuple, in that case we would simply not add the "symbol" key.
IIRC, the PyObject_CallObjects() only accept "tuple" arguments, or did
you mean make the "symbol" a dict and an item of the "t" tuple?
I agree the symbol code is ugly, is following a little better?
PyTuple_SetItem(t, n++, PyString_FromString((al->sym) ? al->sym->name : "Unknown"));
Thanks,
Feng
>
> > if (_PyTuple_Resize(&t, n) == -1)
> > Py_FatalError("error resizing Python tuple");
> > --
> > 1.7.1
Em Fri, May 18, 2012 at 10:48:56AM +0800, Feng Tang escreveu:
> Hi Arnaldo,
>
> Thanks for your review!
you're welcome, keep sending patches ;-)
>
> Ok, will change it. Following is the update patch:
Please next time answer the suggestions/requests inline then send a
separate patchset, with "[PATCH N/M v2] patch summary", its the norm for
such cases and makes the scales things up for maintainers :-)
I'll process this one now.
- Arnaldo
>
> From 87b855a7c85438f45e1fc89250e392c1d6365ec6 Mon Sep 17 00:00:00 2001
> From: Feng Tang <[email protected]>
> Date: Fri, 11 May 2012 16:58:05 +0800
> Subject: [PATCH] perf script: Add general python handler to process non-tracepoint events
>
> This patch just follows Robert Richter's idea and the commit 37a058ea0
> "perf script: Add generic perl handler to process events"
> to similarly add a python handler for general events other than tracepoints.
>
> For non-tracepoint events, this patch will try to find a function named
> "process_event" in the python script, and pass the event attribute,
> perf_sample, raw_data in format of raw string. And the python script can
> use "struct" module's unpack function to disasemble the needed info and process.
>
> Signed-off-by: Feng Tang <[email protected]>
> ---
> .../util/scripting-engines/trace-event-python.c | 60 +++++++++++++++++++-
> 1 files changed, 59 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index c2623c6..6daa12f 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -31,6 +31,7 @@
> #include "../event.h"
> #include "../thread.h"
> #include "../trace-event.h"
> +#include "../evsel.h"
>
> PyMODINIT_FUNC initperf_trace_context(void);
>
> @@ -205,7 +206,7 @@ static inline struct event *find_cache_event(int type)
> return event;
> }
>
> -static void python_process_event(union perf_event *pevent __unused,
> +static void python_process_tracepoint(union perf_event *pevent __unused,
> struct perf_sample *sample,
> struct perf_evsel *evsel __unused,
> struct machine *machine __unused,
> @@ -324,6 +325,63 @@ static void python_process_event(union perf_event *pevent __unused,
> Py_DECREF(t);
> }
>
> +static void python_process_general_event(union perf_event *pevent __unused,
> + struct perf_sample *sample,
> + struct perf_evsel *evsel,
> + struct machine *machine __unused,
> + struct thread *thread __unused)
> +{
> + PyObject *handler, *retval, *t;
> + static char handler_name[64];
> + unsigned n = 0;
> + void *data = sample->raw_data;
> +
> + t = PyTuple_New(MAX_FIELDS);
> + if (!t)
> + Py_FatalError("couldn't create Python tuple");
> +
> + snprintf(handler_name, sizeof(handler_name), "%s", "process_event");
> +
> + handler = PyDict_GetItemString(main_dict, handler_name);
> + if (handler && !PyCallable_Check(handler)) {
> + handler = NULL;
> + goto exit;
> + }
> +
> + /* Pass 3 parameters: event_attr, perf_sample, raw data */
> + PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> + (const char *)&evsel->attr, sizeof(evsel->attr)));
> + PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> + (const char *)sample, sizeof(*sample)));
> + PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> + data, sample->raw_size));
> +
> + if (_PyTuple_Resize(&t, n) == -1)
> + Py_FatalError("error resizing Python tuple");
> +
> + retval = PyObject_CallObject(handler, t);
> + if (retval == NULL)
> + handler_call_die(handler_name);
> +exit:
> + Py_DECREF(t);
> +}
> +
> +static void python_process_event(union perf_event *pevent,
> + struct perf_sample *sample,
> + struct perf_evsel *evsel,
> + struct machine *machine,
> + struct thread *thread)
> +{
> + switch (evsel->attr.type) {
> + case PERF_TYPE_TRACEPOINT:
> + python_process_tracepoint(pevent, sample, evsel, machine, thread);
> + break;
> + /* Reserve for future process_hw/sw/raw APIs */
> + default:
> + python_process_general_event(pevent, sample, evsel, machine, thread);
> + }
> +}
> +
> static int run_start_sub(void)
> {
> PyObject *handler, *retval;
> --
> 1.7.1
>
Em Fri, May 18, 2012 at 10:55:27AM +0800, Feng Tang escreveu:
> On Thu, 17 May 2012 12:47:26 -0300
> Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> > > data, sample->raw_size));
> > > + PyTuple_SetItem(t, n++, PyString_FromString(thread->comm));
> > > + PyTuple_SetItem(t, n++, PyString_FromString(al->map->dso->name));
> > > +
> > > + /* Pass the resolved symbol if there is, othersize pass "Unkown" */
> > > + if (al->sym)
> > > + PyTuple_SetItem(t, n++,
> > > PyString_FromString(al->sym->name));
> > > + else
> > > + PyTuple_SetItem(t, n++, PyString_FromString("Unknown"));
> >
> > Isn't this getting a little bit convoluted?
> >
> > I.e. python has dictionaries, perhaps we could pass a dict instead of a
> > tuple, in that case we would simply not add the "symbol" key.
>
> IIRC, the PyObject_CallObjects() only accept "tuple" arguments, or did
> you mean make the "symbol" a dict and an item of the "t" tuple?
>
> I agree the symbol code is ugly, is following a little better?
> PyTuple_SetItem(t, n++, PyString_FromString((al->sym) ? al->sym->name : "Unknown"));
What I meant was: make it so that the process_event() python method we
look for receives as its first argument a dict.
I.e. pass a tuple to PyObject_CallObjects() that has just one entry: a
dict.
This way if we need to add more parameters in the future, some of which
may not exist (the "Unknown" ones). This way older scripts will continue
working with newer perf tools, they just won't process the new stuff.
I.e. when adding features make sure that old scripts works with newer
perf tools and vice versa as much as possible. If not possible, add
notes to the relevant tools/perf/Documentation/ file.
- Arnaldo
Hi Arnaldo,
On Fri, 18 May 2012 12:38:54 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Fri, May 18, 2012 at 10:55:27AM +0800, Feng Tang escreveu:
> > On Thu, 17 May 2012 12:47:26 -0300
> > Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > PyTuple_SetItem(t, n++, PyString_FromStringAndSize(
> > > > data, sample->raw_size));
> > > > + PyTuple_SetItem(t, n++, PyString_FromString(thread->comm));
> > > > + PyTuple_SetItem(t, n++,
> > > > PyString_FromString(al->map->dso->name)); +
> > > > + /* Pass the resolved symbol if there is, othersize pass
> > > > "Unkown" */
> > > > + if (al->sym)
> > > > + PyTuple_SetItem(t, n++,
> > > > PyString_FromString(al->sym->name));
> > > > + else
> > > > + PyTuple_SetItem(t, n++,
> > > > PyString_FromString("Unknown"));
> > >
> > > Isn't this getting a little bit convoluted?
> > >
> > > I.e. python has dictionaries, perhaps we could pass a dict instead of a
> > > tuple, in that case we would simply not add the "symbol" key.
> >
> > IIRC, the PyObject_CallObjects() only accept "tuple" arguments, or did
> > you mean make the "symbol" a dict and an item of the "t" tuple?
> >
> > I agree the symbol code is ugly, is following a little better?
> > PyTuple_SetItem(t, n++, PyString_FromString((al->sym) ?
> > al->sym->name : "Unknown"));
>
> What I meant was: make it so that the process_event() python method we
> look for receives as its first argument a dict.
>
> I.e. pass a tuple to PyObject_CallObjects() that has just one entry: a
> dict.
>
> This way if we need to add more parameters in the future, some of which
> may not exist (the "Unknown" ones). This way older scripts will continue
> working with newer perf tools, they just won't process the new stuff.
Got your point now. I've modified the code as you suggested and tested
them with my script which worked fine. Will send out a V2 for the whole
patch set after I got comments from David Ahern for my [PATCH 2/3]. Thanks,
- Feng
>
> I.e. when adding features make sure that old scripts works with newer
> perf tools and vice versa as much as possible. If not possible, add
> notes to the relevant tools/perf/Documentation/ file.
>
> - Arnaldo
On 5/17/12 9:45 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 16, 2012 at 08:59:14PM +0800, Feng Tang escreveu:
>> Both perl and python script start processing events other than trace
>> points, and it's useful to pass the resolved symbol and the dso info
>> to the event handler in script for better analysis and statistics.
>>
>> Struct thread is already a member of struct addr_location, using
>> addr_location will keep the thread info, while providing additional
>> symbol and dso info if exist, so that the script itself doesn't need
>> to bother to do the symbol resolving and dso searching work.
>
> This seems ok.
>
> David, any objections or suggestions?
No objections.
Acked-by / Tested-by: David Ahern <[email protected]>