linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] perf: Teach perf tool to profile sleep times
@ 2012-08-06 10:01 Andrew Vagin
  2012-08-06 10:01 ` [PATCH 1/3] perf: teach "perf inject" to work with files Andrew Vagin
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Andrew Vagin @ 2012-08-06 10:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo

This functionality helps to analize where a task sleeps or waits locks.
This feature can help to investigate a scalability problems.

The main idea is that we can combine sched_switch and sched_stat_sleep events.
sched_switch contains a callchain, when a task starts sleeping.
sched_stat_sleep contains a time period for which a task slept.

This series teaches "perf inject" to combine this events.

All kernel related patches were committed committed in 3.6-rc1.

Here is an example of a report:
$ cat ~/foo.c
....
          for (i = 0; i <  10; i++) {
                  ts1.tv_sec = 0;
                  ts1.tv_nsec = 10000000;
                  nanosleep(&ts1, NULL);

                  tv1.tv_sec = 0;
                  tv1.tv_usec = 40000;
                  select(0, NULL, NULL, NULL,&tv1);
          }
...

$ ./perf record -e sched:sched_stat_sleep -e sched:sched_switch \
		-e sched:sched_process_exit -gP -o ~/perf.data.raw ~/foo
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB /root/perf.data.raw (~661 samples) ]
$ ./perf inject -v -s -i ~/perf.data.raw -o ~/perf.data
$ ./perf report -i ~/perf.data
# Samples: 40  of event 'sched:sched_switch'
# Event count (approx.): 1005527702
#
# Overhead  Command      Shared Object          Symbol
# ........  .......  .................  ..............
#
   100.00%      foo  [kernel.kallsyms]  [k] __schedule
                |
                --- __schedule
                    schedule
                   |          
                   |--79.81%-- schedule_hrtimeout_range_clock
                   |          schedule_hrtimeout_range
                   |          poll_schedule_timeout
                   |          do_select
                   |          core_sys_select
                   |          sys_select
                   |          system_call_fastpath
                   |          __select
                   |          __libc_start_main
                   |          
                    --20.19%-- do_nanosleep
                              hrtimer_nanosleep
                              sys_nanosleep
                              system_call_fastpath
                              __GI___libc_nanosleep
                              __libc_start_main

Andrew Vagin (3):
  perf: teach "perf inject" to work with files
  perf: teach perf inject to merge sched_stat_* and sched_switch events
  perf: mark a dso if it's used

 tools/perf/builtin-inject.c |  139 ++++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/build-id.c  |    2 +-
 tools/perf/util/build-id.h  |    5 ++
 3 files changed, 137 insertions(+), 9 deletions(-)


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

* [PATCH 1/3] perf: teach "perf inject" to work with files
  2012-08-06 10:01 [PATCH 0/3] perf: Teach perf tool to profile sleep times Andrew Vagin
@ 2012-08-06 10:01 ` Andrew Vagin
  2012-08-06 18:12   ` Arnaldo Carvalho de Melo
  2012-08-06 10:01 ` [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events Andrew Vagin
  2012-08-06 10:01 ` [PATCH 3/3] perf: mark a dso if it's used Andrew Vagin
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Vagin @ 2012-08-06 10:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo

Before this patch "perf inject" can only handle data from pipe.

I want to use "perf inject" for reworking events. Look at my following patch.

Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
 tools/perf/builtin-inject.c |   33 +++++++++++++++++++++++++++++++--
 1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 3beab48..d04b7a4 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -14,7 +14,12 @@
 
 #include "util/parse-options.h"
 
-static char		const *input_name = "-";
+static char		const *input_name	= "-";
+static const char	*output_name		= "-";
+static int		pipe_output;
+static int		output;
+static u64		bytes_written;
+
 static bool		inject_build_ids;
 
 static int perf_event__repipe_synth(struct perf_tool *tool __used,
@@ -27,12 +32,14 @@ static int perf_event__repipe_synth(struct perf_tool *tool __used,
 	size = event->header.size;
 
 	while (size) {
-		int ret = write(STDOUT_FILENO, buf, size);
+		int ret = write(output, buf, size);
 		if (ret < 0)
 			return -errno;
 
 		size -= ret;
 		buf += ret;
+
+		bytes_written += ret;
 	}
 
 	return 0;
@@ -244,8 +251,14 @@ static int __cmd_inject(void)
 	if (session == NULL)
 		return -ENOMEM;
 
+	if (!pipe_output)
+		lseek(output, session->header.data_offset, SEEK_SET);
 	ret = perf_session__process_events(session, &perf_inject);
 
+	if (!pipe_output) {
+		session->header.data_size = bytes_written;
+		perf_session__write_header(session, session->evlist, output, true);
+	}
 	perf_session__delete(session);
 
 	return ret;
@@ -259,6 +272,10 @@ static const char * const report_usage[] = {
 static const struct option options[] = {
 	OPT_BOOLEAN('b', "build-ids", &inject_build_ids,
 		    "Inject build-ids into the output stream"),
+	OPT_STRING('i', "input", &input_name, "file",
+		    "input file name"),
+	OPT_STRING('o', "output", &output_name, "file",
+		    "output file name"),
 	OPT_INCR('v', "verbose", &verbose,
 		 "be more verbose (show build ids, etc)"),
 	OPT_END()
@@ -274,6 +291,18 @@ int cmd_inject(int argc, const char **argv, const char *prefix __used)
 	if (argc)
 		usage_with_options(report_usage, options);
 
+	if (!strcmp(output_name, "-")) {
+		pipe_output = 1;
+		output = STDOUT_FILENO;
+	} else {
+		output = open(output_name, O_CREAT | O_WRONLY | O_TRUNC,
+							S_IRUSR | S_IWUSR);
+		if (output < 0) {
+			perror("failed to create output file");
+			exit(-1);
+		}
+	}
+
 	if (symbol__init() < 0)
 		return -1;
 
-- 
1.7.1


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

* [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events
  2012-08-06 10:01 [PATCH 0/3] perf: Teach perf tool to profile sleep times Andrew Vagin
  2012-08-06 10:01 ` [PATCH 1/3] perf: teach "perf inject" to work with files Andrew Vagin
@ 2012-08-06 10:01 ` Andrew Vagin
  2012-08-06 18:19   ` Arnaldo Carvalho de Melo
  2012-08-06 10:01 ` [PATCH 3/3] perf: mark a dso if it's used Andrew Vagin
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Vagin @ 2012-08-06 10:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo

You may want to know where and how long a task is sleeping. A callchain
may be found in sched_switch and a time slice in stat_iowait, so I add
handler in perf inject for merging this events.

My code saves sched_switch event for each process and when it meets
stat_iowait, it reports the sched_switch event, because this event
contains a correct callchain. By another words it replaces all
stat_iowait events on proper sched_switch events.

Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
 tools/perf/builtin-inject.c |   96 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index d04b7a4..247f41c 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -13,6 +13,8 @@
 #include "util/debug.h"
 
 #include "util/parse-options.h"
+#include "util/trace-event.h"
+
 
 static char		const *input_name	= "-";
 static const char	*output_name		= "-";
@@ -21,6 +23,9 @@ static int		output;
 static u64		bytes_written;
 
 static bool		inject_build_ids;
+static bool		inject_sched_stat;
+
+struct perf_session	*session;
 
 static int perf_event__repipe_synth(struct perf_tool *tool __used,
 				    union perf_event *event,
@@ -47,7 +52,7 @@ static int perf_event__repipe_synth(struct perf_tool *tool __used,
 
 static int perf_event__repipe_op2_synth(struct perf_tool *tool,
 					union perf_event *event,
-					struct perf_session *session __used)
+					struct perf_session *s __used)
 {
 	return perf_event__repipe_synth(tool, event, NULL);
 }
@@ -59,7 +64,7 @@ static int perf_event__repipe_event_type_synth(struct perf_tool *tool,
 }
 
 static int perf_event__repipe_tracing_data_synth(union perf_event *event,
-						 struct perf_session *session __used)
+						 struct perf_session *s __used)
 {
 	return perf_event__repipe_synth(NULL, event, NULL);
 }
@@ -119,12 +124,12 @@ static int perf_event__repipe_task(struct perf_tool *tool,
 }
 
 static int perf_event__repipe_tracing_data(union perf_event *event,
-					   struct perf_session *session)
+					   struct perf_session *s)
 {
 	int err;
 
 	perf_event__repipe_synth(NULL, event, NULL);
-	err = perf_event__process_tracing_data(event, session);
+	err = perf_event__process_tracing_data(event, s);
 
 	return err;
 }
@@ -210,6 +215,83 @@ repipe:
 	return 0;
 }
 
+struct event_entry {
+	struct list_head list;
+	u32		 pid;
+	union perf_event event[0];
+};
+
+static LIST_HEAD(samples);
+
+static int perf_event__sched_stat(struct perf_tool *tool,
+				      union perf_event *event,
+				      struct perf_sample *sample,
+				      struct perf_evsel *evsel __used,
+				      struct machine *machine)
+{
+	int type;
+	struct event_format *e;
+	const char *evname = NULL;
+	uint32_t size;
+	struct event_entry *ent;
+	union perf_event *event_sw = NULL;
+	struct perf_sample sample_sw;
+	int sched_process_exit;
+
+	size = event->header.size;
+
+	type = trace_parse_common_type(session->pevent, sample->raw_data);
+	e = pevent_find_event(session->pevent, type);
+	if (e)
+		evname = e->name;
+
+	sched_process_exit = !strcmp(evname, "sched_process_exit");
+
+	if (!strcmp(evname, "sched_switch") ||  sched_process_exit) {
+		list_for_each_entry(ent, &samples, list)
+			if (sample->pid == ent->pid)
+				break;
+
+		if (&ent->list != &samples) {
+			list_del(&ent->list);
+			free(ent);
+		}
+
+		if (sched_process_exit)
+			return 0;
+
+		ent = malloc(size + sizeof(struct event_entry));
+		ent->pid = sample->pid;
+		memcpy(&ent->event, event, size);
+		list_add(&ent->list, &samples);
+		return 0;
+
+	} else if (!strncmp(evname, "sched_stat_", 11)) {
+		u32 pid;
+
+		pid = raw_field_value(e, "pid", sample->raw_data);
+
+		list_for_each_entry(ent, &samples, list) {
+			if (pid == ent->pid)
+				break;
+		}
+
+		if (&ent->list == &samples)
+			return 0;
+
+		event_sw = &ent->event[0];
+		perf_session__parse_sample(session, event_sw, &sample_sw);
+		sample_sw.period = sample->period;
+		sample_sw.time = sample->time;
+		perf_session__synthesize_sample(session, event_sw, &sample_sw);
+		perf_event__repipe(tool, event_sw, &sample_sw, machine);
+		return 0;
+	}
+
+	perf_event__repipe(tool, event, sample, machine);
+
+	return 0;
+}
 struct perf_tool perf_inject = {
 	.sample		= perf_event__repipe_sample,
 	.mmap		= perf_event__repipe,
@@ -235,7 +317,6 @@ static void sig_handler(int sig __attribute__((__unused__)))
 
 static int __cmd_inject(void)
 {
-	struct perf_session *session;
 	int ret = -EINVAL;
 
 	signal(SIGINT, sig_handler);
@@ -245,6 +326,9 @@ static int __cmd_inject(void)
 		perf_inject.mmap	 = perf_event__repipe_mmap;
 		perf_inject.fork	 = perf_event__repipe_task;
 		perf_inject.tracing_data = perf_event__repipe_tracing_data;
+	} else if (inject_sched_stat) {
+		perf_inject.sample	= perf_event__sched_stat;
+		perf_inject.ordered_samples = true;
 	}
 
 	session = perf_session__new(input_name, O_RDONLY, false, true, &perf_inject);
@@ -272,6 +356,8 @@ static const char * const report_usage[] = {
 static const struct option options[] = {
 	OPT_BOOLEAN('b', "build-ids", &inject_build_ids,
 		    "Inject build-ids into the output stream"),
+	OPT_BOOLEAN('s', "sched-stat", &inject_sched_stat,
+		    "Set source call-chains for sched:shed-stat-*"),
 	OPT_STRING('i', "input", &input_name, "file",
 		    "input file name"),
 	OPT_STRING('o', "output", &output_name, "file",
-- 
1.7.1


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

* [PATCH 3/3] perf: mark a dso if it's used
  2012-08-06 10:01 [PATCH 0/3] perf: Teach perf tool to profile sleep times Andrew Vagin
  2012-08-06 10:01 ` [PATCH 1/3] perf: teach "perf inject" to work with files Andrew Vagin
  2012-08-06 10:01 ` [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events Andrew Vagin
@ 2012-08-06 10:01 ` Andrew Vagin
  2012-08-06 18:14   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Vagin @ 2012-08-06 10:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo

Otherwise they will be not written in an output file.

Signed-off-by: Andrew Vagin <avagin@openvz.org>
---
 tools/perf/builtin-inject.c |   11 +++++++++--
 tools/perf/util/build-id.c  |    2 +-
 tools/perf/util/build-id.h  |    5 +++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 247f41c..cb2fd77 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -14,6 +14,7 @@
 
 #include "util/parse-options.h"
 #include "util/trace-event.h"
+#include "util/build-id.h"
 
 
 static char		const *input_name	= "-";
@@ -281,13 +282,17 @@ static int perf_event__sched_stat(struct perf_tool *tool,
 
 		event_sw = &ent->event[0];
 		perf_session__parse_sample(session, event_sw, &sample_sw);
+
 		sample_sw.period = sample->period;
 		sample_sw.time = sample->time;
 		perf_session__synthesize_sample(session, event_sw, &sample_sw);
+
+		build_id__mark_dso_hit(tool, event_sw, &sample_sw, evsel, machine);
 		perf_event__repipe(tool, event_sw, &sample_sw, machine);
 		return 0;
 	}
 
+	build_id__mark_dso_hit(tool, event, sample, evsel, machine);
 	perf_event__repipe(tool, event, sample, machine);
 
 	return 0;
@@ -321,12 +326,14 @@ static int __cmd_inject(void)
 
 	signal(SIGINT, sig_handler);
 
-	if (inject_build_ids) {
+	if (inject_build_ids || inject_sched_stat) {
 		perf_inject.sample	 = perf_event__inject_buildid;
 		perf_inject.mmap	 = perf_event__repipe_mmap;
 		perf_inject.fork	 = perf_event__repipe_task;
 		perf_inject.tracing_data = perf_event__repipe_tracing_data;
-	} else if (inject_sched_stat) {
+	}
+
+	if (inject_sched_stat) {
 		perf_inject.sample	= perf_event__sched_stat;
 		perf_inject.ordered_samples = true;
 	}
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index fd9a594..9ce0e11 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -16,7 +16,7 @@
 #include "session.h"
 #include "tool.h"
 
-static int build_id__mark_dso_hit(struct perf_tool *tool __used,
+int build_id__mark_dso_hit(struct perf_tool *tool __used,
 				  union perf_event *event,
 				  struct perf_sample *sample __used,
 				  struct perf_evsel *evsel __used,
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index a993ba8..032a968 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -7,4 +7,9 @@ extern struct perf_tool build_id__mark_dso_hit_ops;
 
 char *dso__build_id_filename(struct dso *self, char *bf, size_t size);
 
+int build_id__mark_dso_hit(struct perf_tool *tool __used,
+				  union perf_event *event,
+				  struct perf_sample *sample __used,
+				  struct perf_evsel *evsel __used,
+				  struct machine *machine);
 #endif
-- 
1.7.1


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

* Re: [PATCH 1/3] perf: teach "perf inject" to work with files
  2012-08-06 10:01 ` [PATCH 1/3] perf: teach "perf inject" to work with files Andrew Vagin
@ 2012-08-06 18:12   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-06 18:12 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar

Em Mon, Aug 06, 2012 at 02:01:57PM +0400, Andrew Vagin escreveu:
> Before this patch "perf inject" can only handle data from pipe.
> 
> I want to use "perf inject" for reworking events. Look at my following patch.
> 
> Signed-off-by: Andrew Vagin <avagin@openvz.org>

Patches that add options to commands should include updates to the docs
(tools/perf/Documentation/).

Also something I saw was the const * char versus const char * stuff for
the foo_file variables, please make them consistent.

- Arnaldo

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

* Re: [PATCH 3/3] perf: mark a dso if it's used
  2012-08-06 10:01 ` [PATCH 3/3] perf: mark a dso if it's used Andrew Vagin
@ 2012-08-06 18:14   ` Arnaldo Carvalho de Melo
  2012-08-06 19:50     ` Andrey Wagin
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-06 18:14 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar

Em Mon, Aug 06, 2012 at 02:01:59PM +0400, Andrew Vagin escreveu:
> -	if (inject_build_ids) {
> +	if (inject_build_ids || inject_sched_stat) {
>  		perf_inject.sample	 = perf_event__inject_buildid;
>  		perf_inject.mmap	 = perf_event__repipe_mmap;
>  		perf_inject.fork	 = perf_event__repipe_task;
>  		perf_inject.tracing_data = perf_event__repipe_tracing_data;
> -	} else if (inject_sched_stat) {
> +	}
> +
> +	if (inject_sched_stat) {
>  		perf_inject.sample	= perf_event__sched_stat;
>  		perf_inject.ordered_samples = true;
>  	}

Huh? so if inject_sched_stat is true we will first set
perf_inject.sample to something, then to another?

- Arnaldo

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

* Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events
  2012-08-06 10:01 ` [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events Andrew Vagin
@ 2012-08-06 18:19   ` Arnaldo Carvalho de Melo
  2012-08-06 19:43     ` Andrey Wagin
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-06 18:19 UTC (permalink / raw)
  To: Andrew Vagin; +Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar

Em Mon, Aug 06, 2012 at 02:01:58PM +0400, Andrew Vagin escreveu:
> You may want to know where and how long a task is sleeping. A callchain
> may be found in sched_switch and a time slice in stat_iowait, so I add
> handler in perf inject for merging this events.
> 
> My code saves sched_switch event for each process and when it meets
> stat_iowait, it reports the sched_switch event, because this event
> contains a correct callchain. By another words it replaces all
> stat_iowait events on proper sched_switch events.
> 
> Signed-off-by: Andrew Vagin <avagin@openvz.org>
> ---
>  tools/perf/builtin-inject.c |   96 ++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 91 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index d04b7a4..247f41c 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -13,6 +13,8 @@
>  #include "util/debug.h"
>  
>  #include "util/parse-options.h"
> +#include "util/trace-event.h"
> +
>  
>  static char		const *input_name	= "-";
>  static const char	*output_name		= "-";
> @@ -21,6 +23,9 @@ static int		output;
>  static u64		bytes_written;
>  
>  static bool		inject_build_ids;
> +static bool		inject_sched_stat;
> +
> +struct perf_session	*session;

Why do we need to insert even more globals?
  
>  static int perf_event__repipe_synth(struct perf_tool *tool __used,
>  				    union perf_event *event,
> @@ -47,7 +52,7 @@ static int perf_event__repipe_synth(struct perf_tool *tool __used,
>  
>  static int perf_event__repipe_op2_synth(struct perf_tool *tool,
>  					union perf_event *event,
> -					struct perf_session *session __used)
> +					struct perf_session *s __used)

What is the point of the above hunk?

>  {
>  	return perf_event__repipe_synth(tool, event, NULL);
>  }
> @@ -59,7 +64,7 @@ static int perf_event__repipe_event_type_synth(struct perf_tool *tool,
>  }
>  
>  static int perf_event__repipe_tracing_data_synth(union perf_event *event,
> -						 struct perf_session *session __used)
> +						 struct perf_session *s __used)

Ditto

>  {
>  	return perf_event__repipe_synth(NULL, event, NULL);
>  }
> @@ -119,12 +124,12 @@ static int perf_event__repipe_task(struct perf_tool *tool,
>  }
>  
>  static int perf_event__repipe_tracing_data(union perf_event *event,
> -					   struct perf_session *session)
> +					   struct perf_session *s)
>  {
>  	int err;
>  
>  	perf_event__repipe_synth(NULL, event, NULL);
> -	err = perf_event__process_tracing_data(event, session);
> +	err = perf_event__process_tracing_data(event, s);

Ditto

>  
>  	return err;
>  }
> @@ -210,6 +215,83 @@ repipe:
>  	return 0;
>  }
>  
> +struct event_entry {
> +	struct list_head list;

Is this really the head of a list? Or is this a node that will allow
event_entry instances to be added to a head of a list? If the former,
please rename this to "node".

> +	u32		 pid;
> +	union perf_event event[0];
> +};
> +
> +static LIST_HEAD(samples);
> +
> +static int perf_event__sched_stat(struct perf_tool *tool,
> +				      union perf_event *event,
> +				      struct perf_sample *sample,
> +				      struct perf_evsel *evsel __used,
> +				      struct machine *machine)
> +{
> +	int type;
> +	struct event_format *e;
> +	const char *evname = NULL;
> +	uint32_t size;
> +	struct event_entry *ent;
> +	union perf_event *event_sw = NULL;
> +	struct perf_sample sample_sw;
> +	int sched_process_exit;
> +
> +	size = event->header.size;
> +
> +	type = trace_parse_common_type(session->pevent, sample->raw_data);
> +	e = pevent_find_event(session->pevent, type);
> +	if (e)
> +		evname = e->name;
> +
> +	sched_process_exit = !strcmp(evname, "sched_process_exit");
> +
> +	if (!strcmp(evname, "sched_switch") ||  sched_process_exit) {
                                              extra space
> +		list_for_each_entry(ent, &samples, list)
> +			if (sample->pid == ent->pid)
> +				break;
> +
> +		if (&ent->list != &samples) {
> +			list_del(&ent->list);
> +			free(ent);
> +		}
> +
> +		if (sched_process_exit)
> +			return 0;
> +
> +		ent = malloc(size + sizeof(struct event_entry));

Can malloc fail?

> +		ent->pid = sample->pid;
> +		memcpy(&ent->event, event, size);
> +		list_add(&ent->list, &samples);
> +		return 0;
> +
> +	} else if (!strncmp(evname, "sched_stat_", 11)) {
> +		u32 pid;
> +
> +		pid = raw_field_value(e, "pid", sample->raw_data);
> +
> +		list_for_each_entry(ent, &samples, list) {
> +			if (pid == ent->pid)
> +				break;
> +		}
> +
> +		if (&ent->list == &samples)
> +			return 0;
> +
> +		event_sw = &ent->event[0];
> +		perf_session__parse_sample(session, event_sw, &sample_sw);
> +		sample_sw.period = sample->period;
> +		sample_sw.time = sample->time;
> +		perf_session__synthesize_sample(session, event_sw, &sample_sw);

Please use perf_evsel__parse_sample, recently introduced.

> +		perf_event__repipe(tool, event_sw, &sample_sw, machine);
> +		return 0;
> +	}
> +
> +	perf_event__repipe(tool, event, sample, machine);
> +
> +	return 0;
> +}
>  struct perf_tool perf_inject = {
>  	.sample		= perf_event__repipe_sample,
>  	.mmap		= perf_event__repipe,
> @@ -235,7 +317,6 @@ static void sig_handler(int sig __attribute__((__unused__)))
>  
>  static int __cmd_inject(void)
>  {
> -	struct perf_session *session;
>  	int ret = -EINVAL;
>  
>  	signal(SIGINT, sig_handler);
> @@ -245,6 +326,9 @@ static int __cmd_inject(void)
>  		perf_inject.mmap	 = perf_event__repipe_mmap;
>  		perf_inject.fork	 = perf_event__repipe_task;
>  		perf_inject.tracing_data = perf_event__repipe_tracing_data;
> +	} else if (inject_sched_stat) {
> +		perf_inject.sample	= perf_event__sched_stat;
> +		perf_inject.ordered_samples = true;
>  	}
>  
>  	session = perf_session__new(input_name, O_RDONLY, false, true, &perf_inject);
> @@ -272,6 +356,8 @@ static const char * const report_usage[] = {
>  static const struct option options[] = {
>  	OPT_BOOLEAN('b', "build-ids", &inject_build_ids,
>  		    "Inject build-ids into the output stream"),
> +	OPT_BOOLEAN('s', "sched-stat", &inject_sched_stat,
> +		    "Set source call-chains for sched:shed-stat-*"),

You're adding an option, needs to be documented on
perf/tools/Documentation/

>  	OPT_STRING('i', "input", &input_name, "file",
>  		    "input file name"),
>  	OPT_STRING('o', "output", &output_name, "file",
> -- 
> 1.7.1

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

* Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events
  2012-08-06 18:19   ` Arnaldo Carvalho de Melo
@ 2012-08-06 19:43     ` Andrey Wagin
  2012-08-06 22:00       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Andrey Wagin @ 2012-08-06 19:43 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar

Hello Arnaldo,

Thanks for comments, I will correct them. I need a bit more details
about two of them.

2012/8/6 Arnaldo Carvalho de Melo <acme@ghostprotocols.net>:
>> @@ -21,6 +23,9 @@ static int          output;
>>  static u64           bytes_written;
>>
>>  static bool          inject_build_ids;
>> +static bool          inject_sched_stat;
>> +
>> +struct perf_session  *session;

perf_event__sched_stat (perf_inject.sample) uses "session" for getting
an event name. I don't know how to get it by another way

>
> Why do we need to insert even more globals?
>
>>  static int perf_event__repipe_synth(struct perf_tool *tool __used,
>>                                   union perf_event *event,
>> @@ -47,7 +52,7 @@ static int perf_event__repipe_synth(struct perf_tool *tool __used,
>>
>>  static int perf_event__repipe_op2_synth(struct perf_tool *tool,
>>                                       union perf_event *event,
>> -                                     struct perf_session *session __used)
>> +                                     struct perf_session *s __used)
>
> What is the point of the above hunk?

"session" is global, for this reason I renamed all arguments.

p.s. Arnaldo, sorry for the personal message with the same content.
It's my mistake.

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

* Re: [PATCH 3/3] perf: mark a dso if it's used
  2012-08-06 18:14   ` Arnaldo Carvalho de Melo
@ 2012-08-06 19:50     ` Andrey Wagin
  0 siblings, 0 replies; 12+ messages in thread
From: Andrey Wagin @ 2012-08-06 19:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar

2012/8/6 Arnaldo Carvalho de Melo <acme@ghostprotocols.net>:
> Em Mon, Aug 06, 2012 at 02:01:59PM +0400, Andrew Vagin escreveu:
>> -     if (inject_build_ids) {
>> +     if (inject_build_ids || inject_sched_stat) {
>>               perf_inject.sample       = perf_event__inject_buildid;
>>               perf_inject.mmap         = perf_event__repipe_mmap;
>>               perf_inject.fork         = perf_event__repipe_task;
>>               perf_inject.tracing_data = perf_event__repipe_tracing_data;
>> -     } else if (inject_sched_stat) {
>> +     }
>> +
>> +     if (inject_sched_stat) {
>>               perf_inject.sample      = perf_event__sched_stat;
>>               perf_inject.ordered_samples = true;
>>       }
>
> Huh? so if inject_sched_stat is true we will first set
> perf_inject.sample to something, then to another?

Yes, we will. I though that it will be better then this:

if (inject_build_ids || inject_sched_stat) {
               perf_inject.mmap         = perf_event__repipe_mmap;
               perf_inject.fork         = perf_event__repipe_task;
               perf_inject.tracing_data = perf_event__repipe_tracing_data;
}

if (inject_build_ids) {
               perf_inject.sample       = perf_event__inject_buildid;
} else if (inject_sched_stat) {
               perf_inject.sample      = perf_event__sched_stat;
               perf_inject.ordered_samples = true;
}

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

* Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events
  2012-08-06 19:43     ` Andrey Wagin
@ 2012-08-06 22:00       ` Arnaldo Carvalho de Melo
  2012-08-06 23:31         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-06 22:00 UTC (permalink / raw)
  To: Andrey Wagin; +Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

Em Mon, Aug 06, 2012 at 11:43:04PM +0400, Andrey Wagin escreveu:
> 2012/8/6 Arnaldo Carvalho de Melo <acme@ghostprotocols.net>:
> >> +struct perf_session  *session;
> 
> perf_event__sched_stat (perf_inject.sample) uses "session" for getting
> an event name. I don't know how to get it by another way

Can you try with the attached patch? We already lookup the event_format
entries when we read the perf.data header so that we can cache
evsel->name, we might as well cache the event_format in
evsel->tp_format, so that tools don't have to relookup this for each
sample.

It would look like:

	static int perf_event__sched_stat(struct perf_tool *tool,
					  union perf_event *event,
					  struct perf_sample *sample,
					  struct perf_evsel *evsel,
					  struct machine *machine)
	{
		int type;
		struct event_format *e = evsel->tp_format;
		const char *evname = e->name;

- Arnaldo

[-- Attachment #2: a.patch --]
[-- Type: text/plain, Size: 660 bytes --]

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index b559929..a56c457 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -56,6 +56,7 @@ struct perf_evsel {
 	int			ids;
 	struct hists		hists;
 	char			*name;
+	struct event_format	*tp_format;
 	union {
 		void		*priv;
 		off_t		id_offset;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 24c489b..5b328a4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2126,6 +2126,7 @@ static int perf_evsel__set_tracepoint_name(struct perf_evsel *evsel,
 	if (event->name == NULL)
 		return -1;
 
+	evsel->tp_format = event;
 	return 0;
 }
 

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

* Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events
  2012-08-06 22:00       ` Arnaldo Carvalho de Melo
@ 2012-08-06 23:31         ` Arnaldo Carvalho de Melo
  2012-08-13  9:17           ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-08-06 23:31 UTC (permalink / raw)
  To: Andrey Wagin
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	David Ahern

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

Em Mon, Aug 06, 2012 at 07:00:00PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 06, 2012 at 11:43:04PM +0400, Andrey Wagin escreveu:
> > 2012/8/6 Arnaldo Carvalho de Melo <acme@ghostprotocols.net>:
> > >> +struct perf_session  *session;

> > perf_event__sched_stat (perf_inject.sample) uses "session" for getting
> > an event name. I don't know how to get it by another way
> 
> Can you try with the attached patch? We already lookup the event_format
> entries when we read the perf.data header so that we can cache
> evsel->name, we might as well cache the event_format in
> evsel->tp_format, so that tools don't have to relookup this for each
> sample.

Attached goes a more complete patch that removes the pevent_find_event
calls from several tools, David, could you give it some testing?

- Arnaldo

[-- Attachment #2: perf_evsel_tp_format.patch --]
[-- Type: text/plain, Size: 16372 bytes --]

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index ce35015..ffb93f4 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "perf.h"
 
+#include "util/evsel.h"
 #include "util/util.h"
 #include "util/cache.h"
 #include "util/symbol.h"
@@ -57,11 +58,6 @@ static unsigned long nr_allocs, nr_cross_allocs;
 
 #define PATH_SYS_NODE	"/sys/devices/system/node"
 
-struct perf_kmem {
-	struct perf_tool    tool;
-	struct perf_session *session;
-};
-
 static void init_cpunode_map(void)
 {
 	FILE *fp;
@@ -283,16 +279,10 @@ static void process_free_event(void *data,
 	s_alloc->alloc_cpu = -1;
 }
 
-static void process_raw_event(struct perf_tool *tool,
-			      union perf_event *raw_event __used, void *data,
+static void process_raw_event(struct perf_evsel *evsel, void *data,
 			      int cpu, u64 timestamp, struct thread *thread)
 {
-	struct perf_kmem *kmem = container_of(tool, struct perf_kmem, tool);
-	struct event_format *event;
-	int type;
-
-	type = trace_parse_common_type(kmem->session->pevent, data);
-	event = pevent_find_event(kmem->session->pevent, type);
+	struct event_format *event = evsel->tp_format;
 
 	if (!strcmp(event->name, "kmalloc") ||
 	    !strcmp(event->name, "kmem_cache_alloc")) {
@@ -313,10 +303,10 @@ static void process_raw_event(struct perf_tool *tool,
 	}
 }
 
-static int process_sample_event(struct perf_tool *tool,
+static int process_sample_event(struct perf_tool *tool __used,
 				union perf_event *event,
 				struct perf_sample *sample,
-				struct perf_evsel *evsel __used,
+				struct perf_evsel *evsel,
 				struct machine *machine)
 {
 	struct thread *thread = machine__findnew_thread(machine, event->ip.pid);
@@ -329,18 +319,16 @@ static int process_sample_event(struct perf_tool *tool,
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(tool, event, sample->raw_data, sample->cpu,
+	process_raw_event(evsel, sample->raw_data, sample->cpu,
 			  sample->time, thread);
 
 	return 0;
 }
 
-static struct perf_kmem perf_kmem = {
-	.tool = {
-		.sample			= process_sample_event,
-		.comm			= perf_event__process_comm,
-		.ordered_samples	= true,
-	},
+static struct perf_tool perf_kmem = {
+	.sample		 = process_sample_event,
+	.comm		 = perf_event__process_comm,
+	.ordered_samples = true,
 };
 
 static double fragmentation(unsigned long n_req, unsigned long n_alloc)
@@ -497,13 +485,10 @@ static int __cmd_kmem(void)
 	int err = -EINVAL;
 	struct perf_session *session;
 
-	session = perf_session__new(input_name, O_RDONLY, 0, false,
-				    &perf_kmem.tool);
+	session = perf_session__new(input_name, O_RDONLY, 0, false, &perf_kmem);
 	if (session == NULL)
 		return -ENOMEM;
 
-	perf_kmem.session = session;
-
 	if (perf_session__create_kernel_maps(session) < 0)
 		goto out_delete;
 
@@ -511,7 +496,7 @@ static int __cmd_kmem(void)
 		goto out_delete;
 
 	setup_pager();
-	err = perf_session__process_events(session, &perf_kmem.tool);
+	err = perf_session__process_events(session, &perf_kmem);
 	if (err != 0)
 		goto out_delete;
 	sort_result();
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index b3c4285..142b303 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "perf.h"
 
+#include "util/evsel.h"
 #include "util/util.h"
 #include "util/cache.h"
 #include "util/symbol.h"
@@ -718,14 +719,10 @@ process_lock_release_event(void *data,
 		trace_handler->release_event(&release_event, event, cpu, timestamp, thread);
 }
 
-static void
-process_raw_event(void *data, int cpu, u64 timestamp, struct thread *thread)
+static void process_raw_event(struct perf_evsel *evsel, void *data, int cpu,
+			      u64 timestamp, struct thread *thread)
 {
-	struct event_format *event;
-	int type;
-
-	type = trace_parse_common_type(session->pevent, data);
-	event = pevent_find_event(session->pevent, type);
+	struct event_format *event = evsel->tp_format;
 
 	if (!strcmp(event->name, "lock_acquire"))
 		process_lock_acquire_event(data, event, cpu, timestamp, thread);
@@ -849,7 +846,7 @@ static void dump_info(void)
 static int process_sample_event(struct perf_tool *tool __used,
 				union perf_event *event,
 				struct perf_sample *sample,
-				struct perf_evsel *evsel __used,
+				struct perf_evsel *evsel,
 				struct machine *machine)
 {
 	struct thread *thread = machine__findnew_thread(machine, sample->tid);
@@ -860,7 +857,7 @@ static int process_sample_event(struct perf_tool *tool __used,
 		return -1;
 	}
 
-	process_raw_event(sample->raw_data, sample->cpu, sample->time, thread);
+	process_raw_event(evsel, sample->raw_data, sample->cpu, sample->time, thread);
 
 	return 0;
 }
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 7a9ad2b..30ef82a 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -43,11 +43,6 @@ static u64			sleep_measurement_overhead;
 
 static unsigned long		nr_tasks;
 
-struct perf_sched {
-	struct perf_tool    tool;
-	struct perf_session *session;
-};
-
 struct sched_atom;
 
 struct task_desc {
@@ -1596,14 +1591,12 @@ typedef void (*tracepoint_handler)(struct perf_tool *tool, struct event_format *
 				   struct machine *machine,
 				   struct thread *thread);
 
-static int perf_sched__process_tracepoint_sample(struct perf_tool *tool,
+static int perf_sched__process_tracepoint_sample(struct perf_tool *tool __used,
 						 union perf_event *event __used,
 						 struct perf_sample *sample,
 						 struct perf_evsel *evsel,
 						 struct machine *machine)
 {
-	struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
-	struct pevent *pevent = sched->session->pevent;
 	struct thread *thread = machine__findnew_thread(machine, sample->pid);
 
 	if (thread == NULL) {
@@ -1617,25 +1610,18 @@ static int perf_sched__process_tracepoint_sample(struct perf_tool *tool,
 
 	if (evsel->handler.func != NULL) {
 		tracepoint_handler f = evsel->handler.func;
-
-		if (evsel->handler.data == NULL)
-			evsel->handler.data = pevent_find_event(pevent,
-							  evsel->attr.config);
-
-		f(tool, evsel->handler.data, sample, machine, thread);
+		f(tool, evsel->tp_format, sample, machine, thread);
 	}
 
 	return 0;
 }
 
-static struct perf_sched perf_sched = {
-	.tool = {
-		.sample		 = perf_sched__process_tracepoint_sample,
-		.comm		 = perf_event__process_comm,
-		.lost		 = perf_event__process_lost,
-		.fork		 = perf_event__process_task,
-		.ordered_samples = true,
-	},
+static struct perf_tool perf_sched = {
+	.sample		 = perf_sched__process_tracepoint_sample,
+	.comm		 = perf_event__process_comm,
+	.lost		 = perf_event__process_lost,
+	.fork		 = perf_event__process_task,
+	.ordered_samples = true,
 };
 
 static void read_events(bool destroy, struct perf_session **psession)
@@ -1652,18 +1638,15 @@ static void read_events(bool destroy, struct perf_session **psession)
 	};
 	struct perf_session *session;
 
-	session = perf_session__new(input_name, O_RDONLY, 0, false,
-				    &perf_sched.tool);
+	session = perf_session__new(input_name, O_RDONLY, 0, false, &perf_sched);
 	if (session == NULL)
 		die("No Memory");
 
-	perf_sched.session = session;
-
 	err = perf_session__set_tracepoints_handlers(session, handlers);
 	assert(err == 0);
 
 	if (perf_session__has_traces(session, "record -R")) {
-		err = perf_session__process_events(session, &perf_sched.tool);
+		err = perf_session__process_events(session, &perf_sched);
 		if (err)
 			die("Failed to process events, error %d", err);
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 1e60ab7..8dba470 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -262,14 +262,11 @@ static int perf_session__check_output_opt(struct perf_session *session)
 	return 0;
 }
 
-static void print_sample_start(struct pevent *pevent,
-			       struct perf_sample *sample,
+static void print_sample_start(struct perf_sample *sample,
 			       struct thread *thread,
 			       struct perf_evsel *evsel)
 {
-	int type;
 	struct perf_event_attr *attr = &evsel->attr;
-	struct event_format *event;
 	const char *evname = NULL;
 	unsigned long secs;
 	unsigned long usecs;
@@ -307,20 +304,7 @@ static void print_sample_start(struct pevent *pevent,
 	}
 
 	if (PRINT_FIELD(EVNAME)) {
-		if (attr->type == PERF_TYPE_TRACEPOINT) {
-			/*
-			 * XXX Do we really need this here?
-			 * perf_evlist__set_tracepoint_names should have done
-			 * this already
-			 */
-			type = trace_parse_common_type(pevent,
-						       sample->raw_data);
-			event = pevent_find_event(pevent, type);
-			if (event)
-				evname = event->name;
-		} else
-			evname = perf_evsel__name(evsel);
-
+		evname = perf_evsel__name(evsel);
 		printf("%s: ", evname ? evname : "[unknown]");
 	}
 }
@@ -416,7 +400,7 @@ static void print_sample_bts(union perf_event *event,
 }
 
 static void process_event(union perf_event *event __unused,
-			  struct pevent *pevent,
+			  struct pevent *pevent __unused,
 			  struct perf_sample *sample,
 			  struct perf_evsel *evsel,
 			  struct machine *machine,
@@ -427,7 +411,7 @@ static void process_event(union perf_event *event __unused,
 	if (output[attr->type].fields == 0)
 		return;
 
-	print_sample_start(pevent, sample, thread, evsel);
+	print_sample_start(sample, thread, evsel);
 
 	if (is_bts_event(attr)) {
 		print_sample_bts(event, sample, evsel, machine, thread);
@@ -435,9 +419,8 @@ static void process_event(union perf_event *event __unused,
 	}
 
 	if (PRINT_FIELD(TRACE))
-		print_trace_event(pevent, sample->cpu, sample->raw_data,
-				  sample->raw_size);
-
+		event_format__print(evsel->tp_format, sample->cpu,
+				    sample->raw_data, sample->raw_size);
 	if (PRINT_FIELD(ADDR))
 		print_sample_addr(event, sample, machine, thread, attr);
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index b559929..a56c457 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -56,6 +56,7 @@ struct perf_evsel {
 	int			ids;
 	struct hists		hists;
 	char			*name;
+	struct event_format	*tp_format;
 	union {
 		void		*priv;
 		off_t		id_offset;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 74ea3c2..7f13ed4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2123,6 +2123,7 @@ static int perf_evsel__set_tracepoint_name(struct perf_evsel *evsel,
 	if (event->name == NULL)
 		return -1;
 
+	evsel->tp_format = event;
 	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 02dfa19..c266281 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -237,16 +237,16 @@ static void define_event_symbols(struct event_format *event,
 		define_event_symbols(event, ev_name, args->next);
 }
 
-static inline
-struct event_format *find_cache_event(struct pevent *pevent, int type)
+static inline struct event_format *find_cache_event(struct perf_evsel *evsel)
 {
 	static char ev_name[256];
 	struct event_format *event;
+	int type = evsel->attr.config;
 
 	if (events[type])
 		return events[type];
 
-	events[type] = event = pevent_find_event(pevent, type);
+	events[type] = event = evsel->tp_format;
 	if (!event)
 		return NULL;
 
@@ -269,7 +269,6 @@ static void perl_process_tracepoint(union perf_event *perf_event __unused,
 	unsigned long long val;
 	unsigned long s, ns;
 	struct event_format *event;
-	int type;
 	int pid;
 	int cpu = sample->cpu;
 	void *data = sample->raw_data;
@@ -281,11 +280,9 @@ static void perl_process_tracepoint(union perf_event *perf_event __unused,
 	if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
 		return;
 
-	type = trace_parse_common_type(pevent, data);
-
-	event = find_cache_event(pevent, type);
+	event = find_cache_event(evsel);
 	if (!event)
-		die("ug! no event found for type %d", type);
+		die("ug! no event found for type %d", evsel->attr.config);
 
 	pid = trace_parse_common_pid(pevent, data);
 
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index ce4d1b0..8006978 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -27,6 +27,7 @@
 #include <errno.h>
 
 #include "../../perf.h"
+#include "../evsel.h"
 #include "../util.h"
 #include "../event.h"
 #include "../thread.h"
@@ -194,16 +195,21 @@ static void define_event_symbols(struct event_format *event,
 		define_event_symbols(event, ev_name, args->next);
 }
 
-static inline
-struct event_format *find_cache_event(struct pevent *pevent, int type)
+static inline struct event_format *find_cache_event(struct perf_evsel *evsel)
 {
 	static char ev_name[256];
 	struct event_format *event;
+	int type = evsel->attr.config;
 
+	/*
+ 	 * XXX: Do we really need to cache this since now we have evsel->tp_format
+ 	 * cached already? Need to re-read this "cache" routine that as well calls
+ 	 * define_event_symbols() :-\
+ 	 */
 	if (events[type])
 		return events[type];
 
-	events[type] = event = pevent_find_event(pevent, type);
+	events[type] = event = evsel->tp_format;
 	if (!event)
 		return NULL;
 
@@ -217,7 +223,7 @@ struct event_format *find_cache_event(struct pevent *pevent, int type)
 static void python_process_event(union perf_event *perf_event __unused,
 				 struct pevent *pevent,
 				 struct perf_sample *sample,
-				 struct perf_evsel *evsel __unused,
+				 struct perf_evsel *evsel,
 				 struct machine *machine __unused,
 				 struct thread *thread)
 {
@@ -228,7 +234,6 @@ static void python_process_event(union perf_event *perf_event __unused,
 	unsigned long s, ns;
 	struct event_format *event;
 	unsigned n = 0;
-	int type;
 	int pid;
 	int cpu = sample->cpu;
 	void *data = sample->raw_data;
@@ -239,11 +244,9 @@ static void python_process_event(union perf_event *perf_event __unused,
 	if (!t)
 		Py_FatalError("couldn't create Python tuple");
 
-	type = trace_parse_common_type(pevent, data);
-
-	event = find_cache_event(pevent, type);
+	event = find_cache_event(evsel);
 	if (!event)
-		die("ug! no event found for type %d", type);
+		die("ug! no event found for type %d", (int)evsel->attr.config);
 
 	pid = trace_parse_common_pid(pevent, data);
 
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 0715c84..1208834 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -167,20 +167,11 @@ unsigned long long read_size(struct pevent *pevent, void *ptr, int size)
 	return pevent_read_number(pevent, ptr, size);
 }
 
-void print_trace_event(struct pevent *pevent, int cpu, void *data, int size)
+void event_format__print(struct event_format *event,
+			 int cpu, void *data, int size)
 {
-	struct event_format *event;
 	struct pevent_record record;
 	struct trace_seq s;
-	int type;
-
-	type = trace_parse_common_type(pevent, data);
-
-	event = pevent_find_event(pevent, type);
-	if (!event) {
-		warning("ug! no event found for type %d", type);
-		return;
-	}
 
 	memset(&record, 0, sizeof(record));
 	record.cpu = cpu;
@@ -192,6 +183,19 @@ void print_trace_event(struct pevent *pevent, int cpu, void *data, int size)
 	trace_seq_do_printf(&s);
 }
 
+void print_trace_event(struct pevent *pevent, int cpu, void *data, int size)
+{
+	int type = trace_parse_common_type(pevent, data);
+	struct event_format *event = pevent_find_event(pevent, type);
+
+	if (!event) {
+		warning("ug! no event found for type %d", type);
+		return;
+	}
+
+	event_format__print(event, cpu, data, size);
+}
+
 void print_event(struct pevent *pevent, int cpu, void *data, int size,
 		 unsigned long long nsecs, char *comm)
 {
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 8fef1d6..069d105 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -32,6 +32,8 @@ int bigendian(void);
 
 struct pevent *read_trace_init(int file_bigendian, int host_bigendian);
 void print_trace_event(struct pevent *pevent, int cpu, void *data, int size);
+void event_format__print(struct event_format *event,
+			 int cpu, void *data, int size);
 
 void print_event(struct pevent *pevent, int cpu, void *data, int size,
 		 unsigned long long nsecs, char *comm);

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

* Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events
  2012-08-06 23:31         ` Arnaldo Carvalho de Melo
@ 2012-08-13  9:17           ` Ingo Molnar
  0 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2012-08-13  9:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Andrey Wagin, linux-kernel, Peter Zijlstra, Paul Mackerras,
	David Ahern


* Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:

> static int process_sample_event(struct perf_tool *tool __used,
>  				union perf_event *event,
>  				struct perf_sample *sample,
>				struct perf_evsel *evsel,
>  				struct machine *machine)

Just saw this 5-parameter function signature fly by: as a 
separate clean-up it would be really neat to stick most of these 
into an intuitively named helper structure or so. 

struct event_context ectx?

That would make extension of the context easier as well in the 
future. Or so.

Thanks,

	Ingo

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

end of thread, other threads:[~2012-08-13  9:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-06 10:01 [PATCH 0/3] perf: Teach perf tool to profile sleep times Andrew Vagin
2012-08-06 10:01 ` [PATCH 1/3] perf: teach "perf inject" to work with files Andrew Vagin
2012-08-06 18:12   ` Arnaldo Carvalho de Melo
2012-08-06 10:01 ` [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events Andrew Vagin
2012-08-06 18:19   ` Arnaldo Carvalho de Melo
2012-08-06 19:43     ` Andrey Wagin
2012-08-06 22:00       ` Arnaldo Carvalho de Melo
2012-08-06 23:31         ` Arnaldo Carvalho de Melo
2012-08-13  9:17           ` Ingo Molnar
2012-08-06 10:01 ` [PATCH 3/3] perf: mark a dso if it's used Andrew Vagin
2012-08-06 18:14   ` Arnaldo Carvalho de Melo
2012-08-06 19:50     ` Andrey Wagin

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).