linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events
@ 2012-05-16 12:59 Feng Tang
  2012-05-16 12:59 ` [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()" Feng Tang
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Feng Tang @ 2012-05-16 12:59 UTC (permalink / raw)
  To: acme, mingo, a.p.zijlstra, robert.richter, ak, linux-kernel; +Cc: Feng Tang

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 <feng.tang@intel.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()"
  2012-05-16 12:59 [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Feng Tang
@ 2012-05-16 12:59 ` Feng Tang
  2012-05-17 15:45   ` Arnaldo Carvalho de Melo
  2012-05-16 12:59 ` [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python Feng Tang
  2012-05-17 15:43 ` [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 14+ messages in thread
From: Feng Tang @ 2012-05-16 12:59 UTC (permalink / raw)
  To: acme, mingo, a.p.zijlstra, robert.richter, ak, linux-kernel; +Cc: Feng Tang

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 <feng.tang@intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python
  2012-05-16 12:59 [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Feng Tang
  2012-05-16 12:59 ` [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()" Feng Tang
@ 2012-05-16 12:59 ` Feng Tang
  2012-05-17 15:47   ` Arnaldo Carvalho de Melo
  2012-05-17 15:43 ` [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 14+ messages in thread
From: Feng Tang @ 2012-05-16 12:59 UTC (permalink / raw)
  To: acme, mingo, a.p.zijlstra, robert.richter, ak, linux-kernel; +Cc: Feng Tang

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events
  2012-05-16 12:59 [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Feng Tang
  2012-05-16 12:59 ` [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()" Feng Tang
  2012-05-16 12:59 ` [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python Feng Tang
@ 2012-05-17 15:43 ` Arnaldo Carvalho de Melo
  2012-05-18  2:48   ` Feng Tang
  2 siblings, 1 reply; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-17 15:43 UTC (permalink / raw)
  To: Feng Tang; +Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

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 <feng.tang@intel.com>
> ---
>  .../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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()"
  2012-05-16 12:59 ` [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()" Feng Tang
@ 2012-05-17 15:45   ` Arnaldo Carvalho de Melo
  2012-05-17 16:08     ` David Ahern
  2012-05-30 16:10     ` David Ahern
  0 siblings, 2 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-17 15:45 UTC (permalink / raw)
  To: Feng Tang
  Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel, David Ahern

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 <feng.tang@intel.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python
  2012-05-16 12:59 ` [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python Feng Tang
@ 2012-05-17 15:47   ` Arnaldo Carvalho de Melo
  2012-05-18  2:55     ` Feng Tang
  0 siblings, 1 reply; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-17 15:47 UTC (permalink / raw)
  To: Feng Tang; +Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

Em Wed, May 16, 2012 at 08:59:15PM +0800, Feng Tang escreveu:
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>  .../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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()"
  2012-05-17 15:45   ` Arnaldo Carvalho de Melo
@ 2012-05-17 16:08     ` David Ahern
  2012-05-17 16:22       ` Arnaldo Carvalho de Melo
  2012-05-30 16:10     ` David Ahern
  1 sibling, 1 reply; 14+ messages in thread
From: David Ahern @ 2012-05-17 16:08 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Feng Tang, mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()"
  2012-05-17 16:08     ` David Ahern
@ 2012-05-17 16:22       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-17 16:22 UTC (permalink / raw)
  To: David Ahern
  Cc: Feng Tang, mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events
  2012-05-17 15:43 ` [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Arnaldo Carvalho de Melo
@ 2012-05-18  2:48   ` Feng Tang
  2012-05-18 15:34     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 14+ messages in thread
From: Feng Tang @ 2012-05-18  2:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

Hi Arnaldo,

Thanks for your review!

On Thu, 17 May 2012 12:43:15 -0300
Arnaldo Carvalho de Melo <acme@redhat.com> 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 <feng.tang@intel.com>
> > ---
> >  .../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 <feng.tang@intel.com>
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 <feng.tang@intel.com>
---
 .../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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python
  2012-05-17 15:47   ` Arnaldo Carvalho de Melo
@ 2012-05-18  2:55     ` Feng Tang
  2012-05-18 15:38       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 14+ messages in thread
From: Feng Tang @ 2012-05-18  2:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

On Thu, 17 May 2012 12:47:26 -0300
Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Em Wed, May 16, 2012 at 08:59:15PM +0800, Feng Tang escreveu:
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > ---
> >  .../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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events
  2012-05-18  2:48   ` Feng Tang
@ 2012-05-18 15:34     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-18 15:34 UTC (permalink / raw)
  To: Feng Tang; +Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

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 <feng.tang@intel.com>
> 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 <feng.tang@intel.com>
> ---
>  .../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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python
  2012-05-18  2:55     ` Feng Tang
@ 2012-05-18 15:38       ` Arnaldo Carvalho de Melo
  2012-05-19 14:13         ` Feng Tang
  0 siblings, 1 reply; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-18 15:38 UTC (permalink / raw)
  To: Feng Tang; +Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

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 <acme@redhat.com> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python
  2012-05-18 15:38       ` Arnaldo Carvalho de Melo
@ 2012-05-19 14:13         ` Feng Tang
  0 siblings, 0 replies; 14+ messages in thread
From: Feng Tang @ 2012-05-19 14:13 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

Hi Arnaldo,


On Fri, 18 May 2012 12:38:54 -0300
Arnaldo Carvalho de Melo <acme@redhat.com> 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 <acme@redhat.com> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()"
  2012-05-17 15:45   ` Arnaldo Carvalho de Melo
  2012-05-17 16:08     ` David Ahern
@ 2012-05-30 16:10     ` David Ahern
  1 sibling, 0 replies; 14+ messages in thread
From: David Ahern @ 2012-05-30 16:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Feng Tang, mingo, a.p.zijlstra, robert.richter, ak, linux-kernel

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 <dsahern@gmail.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-05-30 16:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-16 12:59 [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Feng Tang
2012-05-16 12:59 ` [PATCH 2/3] perf script: Replace "struct thread" with "struct addr_location" as a parameter for "process_event()" Feng Tang
2012-05-17 15:45   ` Arnaldo Carvalho de Melo
2012-05-17 16:08     ` David Ahern
2012-05-17 16:22       ` Arnaldo Carvalho de Melo
2012-05-30 16:10     ` David Ahern
2012-05-16 12:59 ` [PATCH 3/3] perf script/python: Pass thread/dso name and symbol info to event handler in python Feng Tang
2012-05-17 15:47   ` Arnaldo Carvalho de Melo
2012-05-18  2:55     ` Feng Tang
2012-05-18 15:38       ` Arnaldo Carvalho de Melo
2012-05-19 14:13         ` Feng Tang
2012-05-17 15:43 ` [PATCH 1/3] perf script: Add general python handler to process non-tracepoint events Arnaldo Carvalho de Melo
2012-05-18  2:48   ` Feng Tang
2012-05-18 15:34     ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).