[RFC 0/4] perf tools: Remove event types data

[RFC 0/4] perf tools: Remove event types data

[Jiri Olsa]

hi,
following up on the 'perf timechart' FIXME note and changing
its tracepoint match not to use event types data.

In my tests the old and new timechart outputs look the same,
so.. any other tester would be appreciated ;-)

Also as this was the only user of the event types data
the rest of the patchset is removing it out of the perf.

The event types data are referenced from the perf data
file header. The reference (offset & size) stays in the
header with 0s.

Also the event types data is already duplicated via the
event_desc FEATURE, so there's no information loss.

thanks for comments,
jirka


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Thomas Renninger <trenn@suse.de>
---
 tools/perf/builtin-inject.c    |   7 ------
 tools/perf/builtin-record.c    |  13 -----------
 tools/perf/builtin-report.c    |   1 -
 tools/perf/builtin-script.c    |   1 -
 tools/perf/builtin-timechart.c |  94 ++++++++++++++++++++++++++++++++++++++++++-------------------------------------
 tools/perf/util/event.h        |   2 +-
 tools/perf/util/header.c       | 118 +---------------------------------------------------------------------------------------------------
 tools/perf/util/header.h       |  16 +-------------
 tools/perf/util/session.c      |  11 ----------
 tools/perf/util/tool.h         |   3 ---
 10 files changed, 54 insertions(+), 212 deletions(-)

[PATCH 1/4] perf tools: Remove event types framework only user

[Jiri Olsa]

The only user of the event types data is 'perf timechart'
command and uses this info to identify proper tracepoints
based on its name.

Switching this code to use traceevent library API to obtain
IDs for needed tracepoints. This should also make the samples
processing faster as we no longer compare strings but numbers.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Thomas Renninger <trenn@suse.de>
---
 tools/perf/builtin-timechart.c | 94 +++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 43 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 4536a92..852f11ed 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -12,6 +12,8 @@
  * of the License.
  */
 
+#include <traceevent/event-parse.h>
+
 #include "builtin.h"
 
 #include "util/util.h"
@@ -47,6 +49,16 @@ static u64		first_time, last_time;
 
 static bool		power_only;
 
+static u32 tp_power_cpu_idle;
+static u32 tp_power_cpu_frequency;
+static u32 tp_sched_sched_wakeup;
+static u32 tp_sched_sched_switch;
+
+#ifdef SUPPORT_OLD_POWER_EVENTS
+static u32 tp_power_power_start;
+static u32 tp_power_power_end;
+static u32 tp_power_power_frequency;
+#endif
 
 struct per_pid;
 struct per_pidcomm;
@@ -328,25 +340,6 @@ struct wakeup_entry {
 	int   success;
 };
 
-/*
- * trace_flag_type is an enumeration that holds different
- * states when a trace occurs. These are:
- *  IRQS_OFF            - interrupts were disabled
- *  IRQS_NOSUPPORT      - arch does not support irqs_disabled_flags
- *  NEED_RESCED         - reschedule is requested
- *  HARDIRQ             - inside an interrupt handler
- *  SOFTIRQ             - inside a softirq handler
- */
-enum trace_flag_type {
-	TRACE_FLAG_IRQS_OFF		= 0x01,
-	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
-	TRACE_FLAG_NEED_RESCHED		= 0x04,
-	TRACE_FLAG_HARDIRQ		= 0x08,
-	TRACE_FLAG_SOFTIRQ		= 0x10,
-};
-
-
-
 struct sched_switch {
 	struct trace_entry te;
 	char prev_comm[TASK_COMM_LEN];
@@ -497,59 +490,42 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 
 	te = (void *)sample->raw_data;
 	if ((evsel->attr.sample_type & PERF_SAMPLE_RAW) && sample->raw_size > 0) {
-		char *event_str;
 #ifdef SUPPORT_OLD_POWER_EVENTS
 		struct power_entry_old *peo;
 		peo = (void *)te;
 #endif
-		/*
-		 * FIXME: use evsel, its already mapped from id to perf_evsel,
-		 * remove perf_header__find_event infrastructure bits.
-		 * Mapping all these "power:cpu_idle" strings to the tracepoint
-		 * ID and then just comparing against evsel->attr.config.
-		 *
-		 * e.g.:
-		 *
-		 * if (evsel->attr.config == power_cpu_idle_id)
-		 */
-		event_str = perf_header__find_event(te->type);
-
-		if (!event_str)
-			return 0;
 
 		if (sample->cpu > numcpus)
 			numcpus = sample->cpu;
 
-		if (strcmp(event_str, "power:cpu_idle") == 0) {
+		if (evsel->attr.config == tp_power_cpu_idle) {
 			struct power_processor_entry *ppe = (void *)te;
 			if (ppe->state == (u32)PWR_EVENT_EXIT)
 				c_state_end(ppe->cpu_id, sample->time);
 			else
 				c_state_start(ppe->cpu_id, sample->time,
 					      ppe->state);
-		}
-		else if (strcmp(event_str, "power:cpu_frequency") == 0) {
+		} else if (evsel->attr.config == tp_power_cpu_frequency) {
 			struct power_processor_entry *ppe = (void *)te;
 			p_state_change(ppe->cpu_id, sample->time, ppe->state);
 		}
 
-		else if (strcmp(event_str, "sched:sched_wakeup") == 0)
+		else if (evsel->attr.config == tp_sched_sched_wakeup)
 			sched_wakeup(sample->cpu, sample->time, sample->pid, te);
 
-		else if (strcmp(event_str, "sched:sched_switch") == 0)
+		else if (evsel->attr.config == tp_sched_sched_switch)
 			sched_switch(sample->cpu, sample->time, te);
 
 #ifdef SUPPORT_OLD_POWER_EVENTS
 		if (use_old_power_events) {
-			if (strcmp(event_str, "power:power_start") == 0)
+			if (evsel->attr.config == tp_power_power_start)
 				c_state_start(peo->cpu_id, sample->time,
 					      peo->value);
 
-			else if (strcmp(event_str, "power:power_end") == 0)
+			else if (evsel->attr.config == tp_power_power_end)
 				c_state_end(sample->cpu, sample->time);
 
-			else if (strcmp(event_str,
-					"power:power_frequency") == 0)
+			else if (evsel->attr.config == tp_power_power_frequency)
 				p_state_change(peo->cpu_id, sample->time,
 					       peo->value);
 		}
@@ -965,6 +941,35 @@ static void write_svg_file(const char *filename)
 	svg_close();
 }
 
+static int get_id(const char *sys, const char *name, u32 *id)
+{
+	struct event_format *format;
+
+	format = event_format__new(sys, name);
+	if (!format)
+		return -1;
+
+	*id = format->id;
+	pevent_free_format(format);
+	return 0;
+}
+
+static int resolve_tracepoints(void)
+{
+	if (get_id("power", "cpu_idle", &tp_power_cpu_idle) ||
+	    get_id("power", "cpu_frequency", &tp_power_cpu_frequency) ||
+	    get_id("sched", "sched_wakeup", &tp_sched_sched_wakeup) ||
+#ifdef SUPPORT_OLD_POWER_EVENTS
+	    get_id("power", "power_start", &tp_power_power_start) ||
+	    get_id("power", "power_end", &tp_power_power_end) ||
+	    get_id("power", "power_frequency", &tp_power_power_frequency) ||
+#endif
+	    get_id("sched", "sched_switch", &tp_sched_sched_switch))
+		return -1;
+
+	return 0;
+}
+
 static int __cmd_timechart(const char *output_name)
 {
 	struct perf_tool perf_timechart = {
@@ -984,6 +989,9 @@ static int __cmd_timechart(const char *output_name)
 	if (!perf_session__has_traces(session, "timechart record"))
 		goto out_delete;
 
+	if (resolve_tracepoints())
+		goto out_delete;
+
 	ret = perf_session__process_events(session, &perf_timechart);
 	if (ret)
 		goto out_delete;
-- 
1.7.11.7

[PATCH 2/4] perf tools: Remove event types from perf data file

[Jiri Olsa]

Removing event types data storing/reading to/from perf
data file as it's no longer needed. The only user of
this data 'perf timechart' is switched to use traceevent
library.

The event_types offset and size stays in the perf data
file header but are ignored from now on.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Thomas Renninger <trenn@suse.de>
---
 tools/perf/util/header.c | 28 +---------------------------
 tools/perf/util/header.h |  3 +--
 2 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d12d79c..8862667 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2334,16 +2334,6 @@ int perf_session__write_header(struct perf_session *session,
 		}
 	}
 
-	header->event_offset = lseek(fd, 0, SEEK_CUR);
-	header->event_size = trace_event_count * sizeof(struct perf_trace_event_type);
-	if (trace_events) {
-		err = do_write(fd, trace_events, header->event_size);
-		if (err < 0) {
-			pr_debug("failed to write perf header events\n");
-			return err;
-		}
-	}
-
 	header->data_offset = lseek(fd, 0, SEEK_CUR);
 
 	if (at_exit) {
@@ -2364,10 +2354,7 @@ int perf_session__write_header(struct perf_session *session,
 			.offset = header->data_offset,
 			.size	= header->data_size,
 		},
-		.event_types = {
-			.offset = header->event_offset,
-			.size	= header->event_size,
-		},
+		/* event_types is ignored, store zeros */
 	};
 
 	memcpy(&f_header.adds_features, &header->adds_features, sizeof(header->adds_features));
@@ -2614,8 +2601,6 @@ int perf_file_header__read(struct perf_file_header *header,
 	memcpy(&ph->adds_features, &header->adds_features,
 	       sizeof(ph->adds_features));
 
-	ph->event_offset = header->event_types.offset;
-	ph->event_size   = header->event_types.size;
 	ph->data_offset  = header->data.offset;
 	ph->data_size	 = header->data.size;
 	return 0;
@@ -2839,17 +2824,6 @@ int perf_session__read_header(struct perf_session *session, int fd)
 
 	symbol_conf.nr_events = nr_attrs;
 
-	if (f_header.event_types.size) {
-		lseek(fd, f_header.event_types.offset, SEEK_SET);
-		trace_events = malloc(f_header.event_types.size);
-		if (trace_events == NULL)
-			return -ENOMEM;
-		if (perf_header__getbuffer64(header, fd, trace_events,
-					     f_header.event_types.size))
-			goto out_errno;
-		trace_event_count =  f_header.event_types.size / sizeof(struct perf_trace_event_type);
-	}
-
 	perf_header__process_sections(header, fd, &session->pevent,
 				      perf_file_section__process);
 
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 2d1ca7d..298982f 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -45,6 +45,7 @@ struct perf_file_header {
 	u64				attr_size;
 	struct perf_file_section	attrs;
 	struct perf_file_section	data;
+	/* event_types is ignored */
 	struct perf_file_section	event_types;
 	DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS);
 };
@@ -88,8 +89,6 @@ struct perf_header {
 	s64			attr_offset;
 	u64			data_offset;
 	u64			data_size;
-	u64			event_offset;
-	u64			event_size;
 	DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS);
 	struct perf_session_env env;
 };
-- 
1.7.11.7

[PATCH 3/4] perf tools: Remove event types pushing from record command

[Jiri Olsa]

Removing event types data pushing from record command. It's
no longer needed, because this data is ignored.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Thomas Renninger <trenn@suse.de>
---
 tools/perf/builtin-record.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ecca62e..1f5243c 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -904,7 +904,6 @@ const struct option record_options[] = {
 int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int err = -ENOMEM;
-	struct perf_evsel *pos;
 	struct perf_evlist *evsel_list;
 	struct perf_record *rec = &record;
 	char errbuf[BUFSIZ];
@@ -968,11 +967,6 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (perf_evlist__create_maps(evsel_list, &rec->opts.target) < 0)
 		usage_with_options(record_usage, record_options);
 
-	list_for_each_entry(pos, &evsel_list->entries, node) {
-		if (perf_header__push_event(pos->attr.config, perf_evsel__name(pos)))
-			goto out_free_fd;
-	}
-
 	if (rec->opts.user_interval != ULLONG_MAX)
 		rec->opts.default_interval = rec->opts.user_interval;
 	if (rec->opts.user_freq != UINT_MAX)
-- 
1.7.11.7

[PATCH 4/4] perf tools: Remove event types framework completely

[Jiri Olsa]

Removing event types framework completely. The only remainder
(apart from few comments) is following enum:

  enum perf_user_event_type {
    ...
    PERF_RECORD_HEADER_EVENT_TYPE           = 65, /* depreceated */
    ...
  }

It's kept as depreceated, resulting in error when processed
in perf_session__process_user_event function.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Thomas Renninger <trenn@suse.de>
---
 tools/perf/builtin-inject.c |  7 ----
 tools/perf/builtin-record.c |  7 ----
 tools/perf/builtin-report.c |  1 -
 tools/perf/builtin-script.c |  1 -
 tools/perf/util/event.h     |  2 +-
 tools/perf/util/header.c    | 90 ---------------------------------------------
 tools/perf/util/header.h    | 13 -------
 tools/perf/util/session.c   | 11 ------
 tools/perf/util/tool.h      |  3 --
 9 files changed, 1 insertion(+), 134 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index ad1296c..1d8de2e 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -67,12 +67,6 @@ static int perf_event__repipe_op2_synth(struct perf_tool *tool,
 	return perf_event__repipe_synth(tool, event);
 }
 
-static int perf_event__repipe_event_type_synth(struct perf_tool *tool,
-					       union perf_event *event)
-{
-	return perf_event__repipe_synth(tool, event);
-}
-
 static int perf_event__repipe_attr(struct perf_tool *tool,
 				   union perf_event *event,
 				   struct perf_evlist **pevlist)
@@ -402,7 +396,6 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 			.throttle	= perf_event__repipe,
 			.unthrottle	= perf_event__repipe,
 			.attr		= perf_event__repipe_attr,
-			.event_type	= perf_event__repipe_event_type_synth,
 			.tracing_data	= perf_event__repipe_op2_synth,
 			.finished_round	= perf_event__repipe_op2_synth,
 			.build_id	= perf_event__repipe_op2_synth,
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 1f5243c..a41ac415 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -474,13 +474,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 			goto out_delete_session;
 		}
 
-		err = perf_event__synthesize_event_types(tool, process_synthesized_event,
-							 machine);
-		if (err < 0) {
-			pr_err("Couldn't synthesize event_types.\n");
-			goto out_delete_session;
-		}
-
 		if (have_tracepoints(&evsel_list->entries)) {
 			/*
 			 * FIXME err <= 0 here actually means that
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d1bd252..73759ac 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -741,7 +741,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 			.lost		 = perf_event__process_lost,
 			.read		 = process_read_event,
 			.attr		 = perf_event__process_attr,
-			.event_type	 = perf_event__process_event_type,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
 			.ordered_samples = true,
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 3de8979..ecb6979 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -524,7 +524,6 @@ static struct perf_tool perf_script = {
 	.exit		 = perf_event__process_exit,
 	.fork		 = perf_event__process_fork,
 	.attr		 = perf_event__process_attr,
-	.event_type	 = perf_event__process_event_type,
 	.tracing_data	 = perf_event__process_tracing_data,
 	.build_id	 = perf_event__process_build_id,
 	.ordered_samples = true,
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 1813895..1ebb8fb 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -116,7 +116,7 @@ struct build_id_event {
 enum perf_user_event_type { /* above any possible kernel type */
 	PERF_RECORD_USER_TYPE_START		= 64,
 	PERF_RECORD_HEADER_ATTR			= 64,
-	PERF_RECORD_HEADER_EVENT_TYPE		= 65,
+	PERF_RECORD_HEADER_EVENT_TYPE		= 65, /* depreceated */
 	PERF_RECORD_HEADER_TRACING_DATA		= 66,
 	PERF_RECORD_HEADER_BUILD_ID		= 67,
 	PERF_RECORD_FINISHED_ROUND		= 68,
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 8862667..b28a65e 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -25,41 +25,9 @@
 
 static bool no_buildid_cache = false;
 
-static int trace_event_count;
-static struct perf_trace_event_type *trace_events;
-
 static u32 header_argc;
 static const char **header_argv;
 
-int perf_header__push_event(u64 id, const char *name)
-{
-	struct perf_trace_event_type *nevents;
-
-	if (strlen(name) > MAX_EVENT_NAME)
-		pr_warning("Event %s will be truncated\n", name);
-
-	nevents = realloc(trace_events, (trace_event_count + 1) * sizeof(*trace_events));
-	if (nevents == NULL)
-		return -ENOMEM;
-	trace_events = nevents;
-
-	memset(&trace_events[trace_event_count], 0, sizeof(struct perf_trace_event_type));
-	trace_events[trace_event_count].event_id = id;
-	strncpy(trace_events[trace_event_count].name, name, MAX_EVENT_NAME - 1);
-	trace_event_count++;
-	return 0;
-}
-
-char *perf_header__find_event(u64 id)
-{
-	int i;
-	for (i = 0 ; i < trace_event_count; i++) {
-		if (trace_events[i].event_id == id)
-			return trace_events[i].name;
-	}
-	return NULL;
-}
-
 /*
  * magic2 = "PERFILE2"
  * must be a numerical value to let the endianness
@@ -2936,64 +2904,6 @@ int perf_event__process_attr(struct perf_tool *tool __maybe_unused,
 	return 0;
 }
 
-int perf_event__synthesize_event_type(struct perf_tool *tool,
-				      u64 event_id, char *name,
-				      perf_event__handler_t process,
-				      struct machine *machine)
-{
-	union perf_event ev;
-	size_t size = 0;
-	int err = 0;
-
-	memset(&ev, 0, sizeof(ev));
-
-	ev.event_type.event_type.event_id = event_id;
-	memset(ev.event_type.event_type.name, 0, MAX_EVENT_NAME);
-	strncpy(ev.event_type.event_type.name, name, MAX_EVENT_NAME - 1);
-
-	ev.event_type.header.type = PERF_RECORD_HEADER_EVENT_TYPE;
-	size = strlen(ev.event_type.event_type.name);
-	size = PERF_ALIGN(size, sizeof(u64));
-	ev.event_type.header.size = sizeof(ev.event_type) -
-		(sizeof(ev.event_type.event_type.name) - size);
-
-	err = process(tool, &ev, NULL, machine);
-
-	return err;
-}
-
-int perf_event__synthesize_event_types(struct perf_tool *tool,
-				       perf_event__handler_t process,
-				       struct machine *machine)
-{
-	struct perf_trace_event_type *type;
-	int i, err = 0;
-
-	for (i = 0; i < trace_event_count; i++) {
-		type = &trace_events[i];
-
-		err = perf_event__synthesize_event_type(tool, type->event_id,
-							type->name, process,
-							machine);
-		if (err) {
-			pr_debug("failed to create perf header event type\n");
-			return err;
-		}
-	}
-
-	return err;
-}
-
-int perf_event__process_event_type(struct perf_tool *tool __maybe_unused,
-				   union perf_event *event)
-{
-	if (perf_header__push_event(event->event_type.event_type.event_id,
-				    event->event_type.event_type.name) < 0)
-		return -ENOMEM;
-
-	return 0;
-}
-
 int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 					struct perf_evlist *evlist,
 					perf_event__handler_t process)
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 298982f..669fda5 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -102,9 +102,6 @@ int perf_session__write_header(struct perf_session *session,
 			       int fd, bool at_exit);
 int perf_header__write_pipe(int fd);
 
-int perf_header__push_event(u64 id, const char *name);
-char *perf_header__find_event(u64 id);
-
 void perf_header__set_feat(struct perf_header *header, int feat);
 void perf_header__clear_feat(struct perf_header *header, int feat);
 bool perf_header__has_feat(const struct perf_header *header, int feat);
@@ -132,16 +129,6 @@ int perf_event__synthesize_attrs(struct perf_tool *tool,
 int perf_event__process_attr(struct perf_tool *tool, union perf_event *event,
 			     struct perf_evlist **pevlist);
 
-int perf_event__synthesize_event_type(struct perf_tool *tool,
-				      u64 event_id, char *name,
-				      perf_event__handler_t process,
-				      struct machine *machine);
-int perf_event__synthesize_event_types(struct perf_tool *tool,
-				       perf_event__handler_t process,
-				       struct machine *machine);
-int perf_event__process_event_type(struct perf_tool *tool,
-				   union perf_event *event);
-
 int perf_event__synthesize_tracing_data(struct perf_tool *tool,
 					int fd, struct perf_evlist *evlist,
 					perf_event__handler_t process);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1eb58ee..d0d9f94 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -241,13 +241,6 @@ static int process_finished_round_stub(struct perf_tool *tool __maybe_unused,
 	return 0;
 }
 
-static int process_event_type_stub(struct perf_tool *tool __maybe_unused,
-				   union perf_event *event __maybe_unused)
-{
-	dump_printf(": unhandled!\n");
-	return 0;
-}
-
 static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event,
 				  struct perf_session *session);
@@ -274,8 +267,6 @@ static void perf_tool__fill_defaults(struct perf_tool *tool)
 		tool->unthrottle = process_event_stub;
 	if (tool->attr == NULL)
 		tool->attr = process_event_synth_attr_stub;
-	if (tool->event_type == NULL)
-		tool->event_type = process_event_type_stub;
 	if (tool->tracing_data == NULL)
 		tool->tracing_data = process_event_synth_tracing_data_stub;
 	if (tool->build_id == NULL)
@@ -928,8 +919,6 @@ static int perf_session__process_user_event(struct perf_session *session, union
 		if (err == 0)
 			perf_session__set_id_hdr_size(session);
 		return err;
-	case PERF_RECORD_HEADER_EVENT_TYPE:
-		return tool->event_type(tool, event);
 	case PERF_RECORD_HEADER_TRACING_DATA:
 		/* setup for reading amidst mmap */
 		lseek(session->fd, file_offset, SEEK_SET);
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 88f8cbd..62b16b6 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -22,8 +22,6 @@ typedef int (*event_attr_op)(struct perf_tool *tool,
 			     union perf_event *event,
 			     struct perf_evlist **pevlist);
 
-typedef int (*event_simple_op)(struct perf_tool *tool, union perf_event *event);
-
 typedef int (*event_op2)(struct perf_tool *tool, union perf_event *event,
 			 struct perf_session *session);
 
@@ -39,7 +37,6 @@ struct perf_tool {
 			unthrottle;
 	event_attr_op	attr;
 	event_op2	tracing_data;
-	event_simple_op	event_type;
 	event_op2	finished_round,
 			build_id;
 	bool		ordered_samples;
-- 
1.7.11.7

Re: [RFC 0/4] perf tools: Remove event types data

[Jiri Olsa]

On Tue, Jul 09, 2013 at 06:48:55PM +0200, Jiri Olsa wrote:
> hi,
> following up on the 'perf timechart' FIXME note and changing
> its tracepoint match not to use event types data.
> 
> In my tests the old and new timechart outputs look the same,
> so.. any other tester would be appreciated ;-)
> 
> Also as this was the only user of the event types data
> the rest of the patchset is removing it out of the perf.
> 
> The event types data are referenced from the perf data
> file header. The reference (offset & size) stays in the
> header with 0s.
> 
> Also the event types data is already duplicated via the
> event_desc FEATURE, so there's no information loss.
> 
> thanks for comments,
> jirka

forgot to mention, it's available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/et1

jirka

Re: [RFC 0/4] perf tools: Remove event types data

[Thomas Renninger]

On Tuesday, July 09, 2013 06:48:55 PM Jiri Olsa wrote:
> hi,
> following up on the 'perf timechart' FIXME note and changing
> its tracepoint match not to use event types data.

Looks sane and pretty straight forward.
Still I am not that deep involved in perf to give a Reviewed-by: quickly...

#define SUPPORT_OLD_POWER_EVENTS
could also vanish, maybe with the next round.

         Thomas

Re: [RFC 0/4] perf tools: Remove event types data

[Jiri Olsa]

On Wed, Jul 10, 2013 at 08:11:48AM +0200, Thomas Renninger wrote:
> On Tuesday, July 09, 2013 06:48:55 PM Jiri Olsa wrote:
> > hi,
> > following up on the 'perf timechart' FIXME note and changing
> > its tracepoint match not to use event types data.
> 
> Looks sane and pretty straight forward.
> Still I am not that deep involved in perf to give a Reviewed-by: quickly...

thanks, I saw your name in changelog.. wanted to include
someone who probably uses timechart command ;-)

> 
> #define SUPPORT_OLD_POWER_EVENTS
> could also vanish, maybe with the next round.

ok

jirka

Re: [PATCH 1/4] perf tools: Remove event types framework only user

[Namhyung Kim]

Hi Jiri,

On Tue,  9 Jul 2013 18:48:56 +0200, Jiri Olsa wrote:
> The only user of the event types data is 'perf timechart'
> command and uses this info to identify proper tracepoints
> based on its name.
>
> Switching this code to use traceevent library API to obtain
> IDs for needed tracepoints. This should also make the samples
> processing faster as we no longer compare strings but numbers.

A better way I think is using tracepoint handlers.  You don't need to
compare anything this way since an evsel can know its handler.

Please see perf_session__set_tracepoints_handlers() and builtin-
{kmem,lock,sched}.c.


[SNIP]
>  
> -/*
> - * trace_flag_type is an enumeration that holds different
> - * states when a trace occurs. These are:
> - *  IRQS_OFF            - interrupts were disabled
> - *  IRQS_NOSUPPORT      - arch does not support irqs_disabled_flags
> - *  NEED_RESCED         - reschedule is requested
> - *  HARDIRQ             - inside an interrupt handler
> - *  SOFTIRQ             - inside a softirq handler
> - */
> -enum trace_flag_type {
> -	TRACE_FLAG_IRQS_OFF		= 0x01,
> -	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
> -	TRACE_FLAG_NEED_RESCHED		= 0x04,
> -	TRACE_FLAG_HARDIRQ		= 0x08,
> -	TRACE_FLAG_SOFTIRQ		= 0x10,
> -};

Why did you remove this part?  Isn't it used by elsewhere?  Anyway, it
seems unrelated to this change.


[SNIP]
> +static int get_id(const char *sys, const char *name, u32 *id)
> +{
> +	struct event_format *format;
> +
> +	format = event_format__new(sys, name);

No.  You need to access to tracepoints in the perf session data, not
ones in the current system.  The perf_session__set_tracepoints_handlers()
deals with it correctly.

Thanks,
Namhyung


> +	if (!format)
> +		return -1;
> +
> +	*id = format->id;
> +	pevent_free_format(format);
> +	return 0;
> +}

Re: [RFC 0/4] perf tools: Remove event types data

[Namhyung Kim]

On Tue,  9 Jul 2013 18:48:55 +0200, Jiri Olsa wrote:
> hi,
> following up on the 'perf timechart' FIXME note and changing
> its tracepoint match not to use event types data.
>
> In my tests the old and new timechart outputs look the same,
> so.. any other tester would be appreciated ;-)
>
> Also as this was the only user of the event types data
> the rest of the patchset is removing it out of the perf.
>
> The event types data are referenced from the perf data
> file header. The reference (offset & size) stays in the
> header with 0s.
>
> Also the event types data is already duplicated via the
> event_desc FEATURE, so there's no information loss.

I generally agree on removing the event_types but have a couple of
issues on patch 1.  Please see my comments on it.

Thanks,
Namhyung

Re: [PATCH 1/4] perf tools: Remove event types framework only user

[Jiri Olsa]

On Thu, Jul 11, 2013 at 03:16:45PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Tue,  9 Jul 2013 18:48:56 +0200, Jiri Olsa wrote:
> > The only user of the event types data is 'perf timechart'
> > command and uses this info to identify proper tracepoints
> > based on its name.
> >
> > Switching this code to use traceevent library API to obtain
> > IDs for needed tracepoints. This should also make the samples
> > processing faster as we no longer compare strings but numbers.
> 
> A better way I think is using tracepoint handlers.  You don't need to
> compare anything this way since an evsel can know its handler.
> 
> Please see perf_session__set_tracepoints_handlers() and builtin-
> {kmem,lock,sched}.c.

nice, that will be better

> 
> 
> [SNIP]
> >  
> > -/*
> > - * trace_flag_type is an enumeration that holds different
> > - * states when a trace occurs. These are:
> > - *  IRQS_OFF            - interrupts were disabled
> > - *  IRQS_NOSUPPORT      - arch does not support irqs_disabled_flags
> > - *  NEED_RESCED         - reschedule is requested
> > - *  HARDIRQ             - inside an interrupt handler
> > - *  SOFTIRQ             - inside a softirq handler
> > - */
> > -enum trace_flag_type {
> > -	TRACE_FLAG_IRQS_OFF		= 0x01,
> > -	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
> > -	TRACE_FLAG_NEED_RESCHED		= 0x04,
> > -	TRACE_FLAG_HARDIRQ		= 0x08,
> > -	TRACE_FLAG_SOFTIRQ		= 0x10,
> > -};
> 
> Why did you remove this part?  Isn't it used by elsewhere?  Anyway, it
> seems unrelated to this change.

it's now defined in <traceevent/event-parse.h>
leftover from some earlier change I guess

> 
> 
> [SNIP]
> > +static int get_id(const char *sys, const char *name, u32 *id)
> > +{
> > +	struct event_format *format;
> > +
> > +	format = event_format__new(sys, name);
> 
> No.  You need to access to tracepoints in the perf session data, not
> ones in the current system.  The perf_session__set_tracepoints_handlers()
> deals with it correctly.

right you are.. need to access data

thanks,
jirka

Re: [RFC 0/4] perf tools: Remove event types data

[Arnaldo Carvalho de Melo]

Em Wed, Jul 10, 2013 at 09:49:40AM +0200, Jiri Olsa escreveu:
> On Wed, Jul 10, 2013 at 08:11:48AM +0200, Thomas Renninger wrote:
> > On Tuesday, July 09, 2013 06:48:55 PM Jiri Olsa wrote:
> > > hi,
> > > following up on the 'perf timechart' FIXME note and changing
> > > its tracepoint match not to use event types data.
> > 
> > Looks sane and pretty straight forward.
> > Still I am not that deep involved in perf to give a Reviewed-by: quickly...

In such cases an "Acked-by:" seems adequate enough, right? I'm merging
this now, so will not add it, but please let us know if that would be
ok, I may go thru some rebase and then would add that line.
 
> thanks, I saw your name in changelog.. wanted to include
> someone who probably uses timechart command ;-)
> 
> > 
> > #define SUPPORT_OLD_POWER_EVENTS
> > could also vanish, maybe with the next round.
> 
> ok
> 
> jirka
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/