[PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[Stephane Eranian]

This patch series adds meta-data support to perf record/report in pipe
mode:
	perf record -o - foo | perf inject -b | perf report -i -

We had meta-data (information about host configuration, perf tool version,...)
in regular (file) mode but it was lacking in pipe mode. This patch series fixes
this.

In pipe mode, there is no meta-data header structures at the beginning of the
streamed data given we cannot seek in a pipe. Instead, we need to create pseudo
record types for each of the possible features, e.g., hostname, cpuid, event_desc,
and so on. Those pseudo records are guaranteed to be before any actual sample
records, therefore perf report/annotate are guaranteed to get the information
they need before processing the first sample.

The series also fixes perf inject to actually inject build-ids and buildid-list
to work better with pipe mode, i.e., print the build-ids.

With this series:

 $ perf record -o - noploop 2 | perf inject -b | perf report -i -
      # ========
      # captured on: Fri Jan 20 18:13:55 2012
      # ========
      #
      # hostname : quad
      # os release : 3.2.0-rc7-tip
      # perf version : 3.2.0
      # arch : x86_64
      # nrcpus online : 4
      # nrcpus avail : 4
      # cpudesc : Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz
      # cpuid : GenuineIntel,6,15,11
      # total memory : 8092884 kB
      ...
      # HEADER_CPU_TOPOLOGY info available, use -I to display
      noploop for 2 seconds
      [ perf record: Woken up 1 times to write data ]
      [ perf record: Captured and wrote 0.084 MB - (~3677 samples) ]
        99.80%  noploop  noploop            [.] noploop
         0.19%  noploop  [kernel.kallsyms]  [k] radix_tree_gang_lookup

V2 is just a rebase to 3.4.0-rc7.

Signed-off-by: Stephane Eranian <eranian@google.com>

Stephane Eranian (6):
  perf inject: fix broken perf inject -b
  perf tools: fix piped mode read code
  perf tools: rename HEADER_TRACE_INFO to HEADER_TRACING_DATA
  perf record: add meta-data support for pipe-mode
  perf: make perf buildid-list work better with pipe mode

-- 
1.7.4.1

[PATCH v2 1/5] perf inject: fix broken perf inject -b

[Stephane Eranian]

perf inject -b was broken. It would not inject any build_id
into the stream. Furthermore, it would strip samples from the
stream.

The reason was a missing initialization of the event attribute
structure. The perf_tool.tool.attr() callback was pointing to
a simple repipe. But there was no initialization of the internal
data structures to keep track of events and event ids. That later
caused event id lookups to fail, and sample would get removed.

The patch simply adds back the call to perf_event__process_attr()
to initialize the evlist structure and now build_ids are again
injected.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-inject.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 09c1061..3beab48 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
 static int perf_event__repipe_attr(union perf_event *event,
 				   struct perf_evlist **pevlist __used)
 {
+	int ret;
+	ret = perf_event__process_attr(event, pevlist);
+	if (ret)
+		return ret;
+
 	return perf_event__repipe_synth(NULL, event, NULL);
 }
 
-- 
1.7.4.1

[PATCH v2 2/5] perf tools: fix piped mode read code

[Stephane Eranian]

In __perf_session__process_pipe_events(), there was a risk
we would read more than what a union perf_event struct can
hold. this could happen in case, perf is reading a file which
contains new record types it does not know about and which are
larger than anything it knows about.

In general, perf is supposed to skip records it does not
understand, but in pipe mode, those have to be read and ignored.
The fixed size header contains the size of the record, but that
size may be larger than union perf_event, yet it was used as
the backing to the read in:

  union perf_event event;
  void *p;

  size = event->header.size;

  p = &event;
  p += sizeof(struct perf_event_header);
  if (size - sizeof(struct perf_event_header)) {
    err = readn(self->fd, p, size - sizeof(struct perf_event_header));

We fix this by allocating a buffer based on the size reported in
the header. We reuse the buffer as much as we can. We realloc in
case it becomes too small. In the  common case, the performance
impact is negligible.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/session.c |   34 ++++++++++++++++++++++++++--------
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 4dcc8f3..af6a5f0 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1064,8 +1064,9 @@ volatile int session_done;
 static int __perf_session__process_pipe_events(struct perf_session *self,
 					       struct perf_tool *tool)
 {
-	union perf_event event;
-	uint32_t size;
+	union perf_event *event;
+	uint32_t size, cur_size = 0;
+	void *buf = NULL;
 	int skip = 0;
 	u64 head;
 	int err;
@@ -1074,8 +1075,14 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
 	perf_tool__fill_defaults(tool);
 
 	head = 0;
+	cur_size = sizeof(union perf_event);
+
+	buf = malloc(cur_size);
+	if (!buf)
+		return -errno;
 more:
-	err = readn(self->fd, &event, sizeof(struct perf_event_header));
+	event = buf;
+	err = readn(self->fd, event, sizeof(struct perf_event_header));
 	if (err <= 0) {
 		if (err == 0)
 			goto done;
@@ -1085,13 +1092,23 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
 	}
 
 	if (self->header.needs_swap)
-		perf_event_header__bswap(&event.header);
+		perf_event_header__bswap(&event->header);
 
-	size = event.header.size;
+	size = event->header.size;
 	if (size == 0)
 		size = 8;
 
-	p = &event;
+	if (size > cur_size) {
+		void *new = realloc(buf, size);
+		if (!new) {
+			pr_err("failed to allocate memory to read event\n");
+			goto out_err;
+		}
+		buf = new;
+		cur_size = size;
+		event = buf;
+	}
+	p = event;
 	p += sizeof(struct perf_event_header);
 
 	if (size - sizeof(struct perf_event_header)) {
@@ -1107,9 +1124,9 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
 		}
 	}
 
-	if ((skip = perf_session__process_event(self, &event, tool, head)) < 0) {
+	if ((skip = perf_session__process_event(self, event, tool, head)) < 0) {
 		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
-		       head, event.header.size, event.header.type);
+		       head, event->header.size, event->header.type);
 		err = -EINVAL;
 		goto out_err;
 	}
@@ -1124,6 +1141,7 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
 done:
 	err = 0;
 out_err:
+	free(buf);
 	perf_session__warn_about_errors(self, tool);
 	perf_session_free_sample_buffers(self);
 	return err;
-- 
1.7.4.1

[PATCH v2 3/5] perf tools: rename HEADER_TRACE_INFO to HEADER_TRACING_DATA

[Stephane Eranian]

To match the PERF_RECORD_HEADER_TRACING_DATA record type.
This is the same info as the one used for pipe mode whereas
the other one is for regular file output. This will help in
the later patch to add meta-data infos in pipe mode.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-record.c |    4 ++--
 tools/perf/util/header.c    |   10 +++++-----
 tools/perf/util/header.h    |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d19058a..21cf20b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -396,7 +396,7 @@ static void perf_record__mmap_read_all(struct perf_record *rec)
 			perf_record__mmap_read(rec, &rec->evlist->mmap[i]);
 	}
 
-	if (perf_header__has_feat(&rec->session->header, HEADER_TRACE_INFO))
+	if (perf_header__has_feat(&rec->session->header, HEADER_TRACING_DATA))
 		write_output(rec, &finished_round_event, sizeof(finished_round_event));
 }
 
@@ -478,7 +478,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		perf_header__clear_feat(&session->header, HEADER_BUILD_ID);
 
 	if (!have_tracepoints(&evsel_list->entries))
-		perf_header__clear_feat(&session->header, HEADER_TRACE_INFO);
+		perf_header__clear_feat(&session->header, HEADER_TRACING_DATA);
 
 	if (!rec->opts.branch_stack)
 		perf_header__clear_feat(&session->header, HEADER_BRANCH_STACK);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 5385980..2dd5edf 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -437,7 +437,7 @@ static bool perf_session__read_build_ids(struct perf_session *session, bool with
 	return ret;
 }
 
-static int write_trace_info(int fd, struct perf_header *h __used,
+static int write_tracing_data(int fd, struct perf_header *h __used,
 			    struct perf_evlist *evlist)
 {
 	return read_tracing_data(fd, &evlist->entries);
@@ -1472,7 +1472,7 @@ static int perf_header__read_build_ids(struct perf_header *header,
 	return err;
 }
 
-static int process_trace_info(struct perf_file_section *section __unused,
+static int process_tracing_data(struct perf_file_section *section __unused,
 			      struct perf_header *ph __unused,
 			      int feat __unused, int fd)
 {
@@ -1508,11 +1508,11 @@ struct feature_ops {
 		.full_only = true }
 
 /* feature_ops not implemented: */
-#define print_trace_info		NULL
-#define print_build_id			NULL
+#define print_tracing_data	NULL
+#define print_build_id		NULL
 
 static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
-	FEAT_OPP(HEADER_TRACE_INFO,	trace_info),
+	FEAT_OPP(HEADER_TRACING_DATA,	tracing_data),
 	FEAT_OPP(HEADER_BUILD_ID,	build_id),
 	FEAT_OPA(HEADER_HOSTNAME,	hostname),
 	FEAT_OPA(HEADER_OSRELEASE,	osrelease),
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 21a6be0..2d42b3e 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -12,7 +12,7 @@
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
 	HEADER_FIRST_FEATURE	= 1,
-	HEADER_TRACE_INFO	= 1,
+	HEADER_TRACING_DATA	= 1,
 	HEADER_BUILD_ID,
 
 	HEADER_HOSTNAME,
-- 
1.7.4.1

[PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Stephane Eranian]

This patch adds the meta-data header support for perf record
when used in pipe mode: perf record -o -

Up until now, meta-data was only available when perf record
was used in "regular" mode, i.e., generating a perf.data file.
For users depending on pipe mode, no host, event header information
was gathered. This patch addresses this limitation.

The difficulty in pipe mode is that information needs to be written
sequentially to the pipe. Meta data headers are usually generated
(and also expected) at the beginning of the file (or piped output).
To solve this problem, we introduce new synthetic record types,
one for each meta-data type. The approach is similar to what
is already used for BUILD_ID and TRACING_DATA.

We have modified util/header.c such that the same routines are used
to generate and read the meta-data information regardless of pipe-mode
vs. regular mode. To make this work, we added a new struct called
feat_fd which encapsulates all the information necessary to read or
write meta-data information to a file/pipe or from a file/pipe.

It should be noted that there is a limitation with the current
perf in terms of endianess in pipe mode. Perf assumes the records
are generated using the same endianess during collection and
analysis.  That is always the case with the example shown below.
However, one could also do:
$ perf record -o - noploop 2 | perf inject -b >perf.data
$ cat perf.data | perf report -i -

With this patch, it is possible to get:
 $ perf record -o - noploop 2 | perf inject -b | perf report -i -
      # ========
      # captured on: Fri Jan 20 18:13:55 2012
      # ========
      #
      # hostname : quad
      # os release : 3.2.0-rc7-tip
      # perf version : 3.2.0
      # arch : x86_64
      # nrcpus online : 4
      # nrcpus avail : 4
      # cpudesc : Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz
      # cpuid : GenuineIntel,6,15,11
      # total memory : 8092884 kB
      ...
      # HEADER_CPU_TOPOLOGY info available, use -I to display
      noploop for 2 seconds
      [ perf record: Woken up 1 times to write data ]
      [ perf record: Captured and wrote 0.084 MB - (~3677 samples) ]
        99.80%  noploop  noploop            [.] noploop
         0.19%  noploop  [kernel.kallsyms]  [k] radix_tree_gang_lookup

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-annotate.c |    5 +
 tools/perf/builtin-inject.c   |    1 +
 tools/perf/builtin-record.c   |    7 +
 tools/perf/builtin-report.c   |    7 +-
 tools/perf/util/event.c       |   12 +
 tools/perf/util/event.h       |   18 ++
 tools/perf/util/header.c      |  532 +++++++++++++++++++++++++++++------------
 tools/perf/util/header.h      |    9 +
 tools/perf/util/session.c     |   17 ++-
 tools/perf/util/tool.h        |    4 +-
 10 files changed, 449 insertions(+), 163 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 806e0a2..fe42298 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -247,6 +247,11 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __used)
 			.mmap	= perf_event__process_mmap,
 			.comm	= perf_event__process_comm,
 			.fork	= perf_event__process_task,
+			.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,
+			.feature	 = perf_event__process_feature,
 			.ordered_samples = true,
 			.ordering_requires_timestamps = true,
 		},
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 3beab48..65b0529 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -217,6 +217,7 @@ struct perf_tool perf_inject = {
 	.event_type	= perf_event__repipe_event_type_synth,
 	.tracing_data	= perf_event__repipe_tracing_data_synth,
 	.build_id	= perf_event__repipe_op2_synth,
+	.feature	= perf_event__repipe_op2_synth,
 };
 
 extern volatile int session_done;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 21cf20b..e2c5310 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -531,6 +531,13 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	}
 
 	if (opts->pipe_output) {
+		err = perf_event__synthesize_features(tool, session, evsel_list,
+						   process_synthesized_event);
+		if (err < 0) {
+			pr_err("Couldn't synthesize features.\n");
+			return err;
+		}
+
 		err = perf_event__synthesize_attrs(tool, session,
 						   process_synthesized_event);
 		if (err < 0) {
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d58e414..be5d544 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -43,7 +43,6 @@ struct perf_report {
 	bool			force, use_tui, use_gtk, use_stdio;
 	bool			hide_unresolved;
 	bool			dont_use_callchains;
-	bool			show_full_info;
 	bool			show_threads;
 	bool			inverted_callchain;
 	struct perf_read_values	show_threads_values;
@@ -358,7 +357,8 @@ static int __cmd_report(struct perf_report *rep)
 	}
 
 	if (use_browser <= 0)
-		perf_session__fprintf_info(session, stdout, rep->show_full_info);
+		perf_session__fprintf_info(session, stdout,
+					   rep->tool.show_all_features);
 
 	if (rep->show_threads)
 		perf_read_values_init(&rep->show_threads_values);
@@ -562,6 +562,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 			.event_type	 = perf_event__process_event_type,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
+			.feature	 = perf_event__process_feature,
 			.ordered_samples = true,
 			.ordering_requires_timestamps = true,
 		},
@@ -625,7 +626,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 		    "Look for files with symbols relative to this directory"),
 	OPT_STRING('C', "cpu", &report.cpu_list, "cpu",
 		   "list of cpus to profile"),
-	OPT_BOOLEAN('I', "show-info", &report.show_full_info,
+	OPT_BOOLEAN('I', "show-info", &report.tool.show_all_features,
 		    "Display extended information about perf.data file"),
 	OPT_BOOLEAN(0, "source", &symbol_conf.annotate_src,
 		    "Interleave source code with assembly code (default)"),
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 2a6f33c..da3551f 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -23,6 +23,18 @@ static const char *perf_event__names[] = {
 	[PERF_RECORD_HEADER_TRACING_DATA]	= "TRACING_DATA",
 	[PERF_RECORD_HEADER_BUILD_ID]		= "BUILD_ID",
 	[PERF_RECORD_FINISHED_ROUND]		= "FINISHED_ROUND",
+	[PERF_RECORD_HEADER_HOSTNAME]		= "HOSTNAME",
+	[PERF_RECORD_HEADER_OSRELEASE]		= "OSRELEASE",
+	[PERF_RECORD_HEADER_VERSION]		= "VERSION",
+	[PERF_RECORD_HEADER_ARCH]		= "ARCH",
+	[PERF_RECORD_HEADER_NRCPUS]		= "NRCPUS",
+	[PERF_RECORD_HEADER_CPUDESC]		= "CPUDESC",
+	[PERF_RECORD_HEADER_CPUID]		= "CPUID",
+	[PERF_RECORD_HEADER_TOTAL_MEM]		= "TOTAL_MEM",
+	[PERF_RECORD_HEADER_CMDLINE]		= "CMDLINE",
+	[PERF_RECORD_HEADER_EVENT_DESC]		= "EVENT_DESC",
+	[PERF_RECORD_HEADER_CPU_TOPOLOGY]	= "CPU_TOPOLOGY",
+	[PERF_RECORD_HEADER_NUMA_TOPOLOGY]	= "NUMA_TOPOLOGY",
 };
 
 const char *perf_event__name(unsigned int id)
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 1b19728..e44aaec 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -100,6 +100,18 @@ enum perf_user_event_type { /* above any possible kernel type */
 	PERF_RECORD_HEADER_TRACING_DATA		= 66,
 	PERF_RECORD_HEADER_BUILD_ID		= 67,
 	PERF_RECORD_FINISHED_ROUND		= 68,
+	PERF_RECORD_HEADER_HOSTNAME		= 69,
+	PERF_RECORD_HEADER_OSRELEASE		= 70,
+	PERF_RECORD_HEADER_VERSION		= 71,
+	PERF_RECORD_HEADER_ARCH			= 72,
+	PERF_RECORD_HEADER_NRCPUS		= 73,
+	PERF_RECORD_HEADER_CPUDESC		= 74,
+	PERF_RECORD_HEADER_CPUID		= 75,
+	PERF_RECORD_HEADER_TOTAL_MEM		= 76,
+	PERF_RECORD_HEADER_CMDLINE		= 77,
+	PERF_RECORD_HEADER_EVENT_DESC		= 78,
+	PERF_RECORD_HEADER_CPU_TOPOLOGY		= 79,
+	PERF_RECORD_HEADER_NUMA_TOPOLOGY	= 80,
 	PERF_RECORD_HEADER_MAX
 };
 
@@ -126,6 +138,11 @@ struct tracing_data_event {
 	u32 size;
 };
 
+struct feature_event {
+	struct perf_event_header header;
+	char data[]; /* size bytes of raw data specific to the feature */
+};
+
 union perf_event {
 	struct perf_event_header	header;
 	struct ip_event			ip;
@@ -139,6 +156,7 @@ union perf_event {
 	struct event_type_event		event_type;
 	struct tracing_data_event	tracing_data;
 	struct build_id_event		build_id;
+	struct feature_event		feat;
 };
 
 void perf_event__print_totals(void);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2dd5edf..d5cf9b4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -20,6 +20,16 @@
 #include "symbol.h"
 #include "debug.h"
 #include "cpumap.h"
+#include "tool.h"
+
+struct feat_fd {
+	int fd;
+	int needs_swap;
+	void *buf;
+	struct perf_header *hdr;
+	size_t size;
+	size_t pos;
+};
 
 static bool no_buildid_cache = false;
 
@@ -93,14 +103,46 @@ bool perf_header__has_feat(const struct perf_header *header, int feat)
 	return test_bit(feat, header->adds_features);
 }
 
-static int do_write(int fd, const void *buf, size_t size)
+static ssize_t do_read(struct feat_fd *fd, void *addr, size_t size)
+{
+	if (!fd->buf)
+		return read(fd->fd, addr, size);
+
+	size = min(size, (fd->size - fd->pos));
+
+	memcpy(addr, fd->buf+fd->pos, size);
+
+	fd->pos += size;
+
+	return size;
+}
+
+static int do_write(struct feat_fd *fd, const void *buf, size_t size)
 {
+	void *addr;
+	int ret;
+
 	while (size) {
-		int ret = write(fd, buf, size);
 
-		if (ret < 0)
-			return -errno;
+		if (fd->buf == NULL) {
+			ret = write(fd->fd, buf, size);
+			if (ret < 0)
+				return -errno;
+		} else {
+retry:
+			if (size > (fd->size - fd->pos)) {
+				addr = realloc(fd->buf, fd->size << 1);
+				if (!addr)
+					return -ENOSPC;
+				fd->buf = addr;
+				fd->size <<= 1;
+				goto retry;
+			}
 
+			memcpy(fd->buf+fd->pos, buf, size);
+			fd->pos += size;
+			ret = size;
+		}
 		size -= ret;
 		buf += ret;
 	}
@@ -110,7 +152,7 @@ static int do_write(int fd, const void *buf, size_t size)
 
 #define NAME_ALIGN 64
 
-static int write_padded(int fd, const void *bf, size_t count,
+static int write_padded(struct feat_fd *fd, const void *bf, size_t count,
 			size_t count_aligned)
 {
 	static const char zero_buf[NAME_ALIGN];
@@ -122,7 +164,7 @@ static int write_padded(int fd, const void *bf, size_t count,
 	return err;
 }
 
-static int do_write_string(int fd, const char *str)
+static int do_write_string(struct feat_fd *fd, const char *str)
 {
 	u32 len, olen;
 	int ret;
@@ -138,24 +180,35 @@ static int do_write_string(int fd, const char *str)
 	return write_padded(fd, str, olen, len);
 }
 
-static char *do_read_string(int fd, struct perf_header *ph)
+static char *do_read_string(struct feat_fd *fd)
 {
 	ssize_t sz, ret;
 	u32 len;
 	char *buf;
 
-	sz = read(fd, &len, sizeof(len));
-	if (sz < (ssize_t)sizeof(len))
-		return NULL;
+	if (fd->buf) {
+		len = *(int *)(fd->buf + fd->pos);
+		fd->pos += sizeof(u32);
+	} else {
+		sz = read(fd->fd, &len, sizeof(len));
+		if (sz < (ssize_t)sizeof(len))
+			return NULL;
+	}
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		len = bswap_32(len);
 
 	buf = malloc(len);
 	if (!buf)
 		return NULL;
 
-	ret = read(fd, buf, len);
+	if (fd->buf) {
+		memcpy(buf, fd->buf+fd->pos, len);
+		fd->pos += len;
+		return buf;
+	}
+
+	ret = read(fd->fd, buf, len);
 	if (ret == (ssize_t)len) {
 		/*
 		 * strings are padded by zeroes
@@ -164,7 +217,6 @@ static char *do_read_string(int fd, struct perf_header *ph)
 		 */
 		return buf;
 	}
-
 	free(buf);
 	return NULL;
 }
@@ -198,7 +250,7 @@ perf_header__set_cmdline(int argc, const char **argv)
 		else
 
 static int __dsos__write_buildid_table(struct list_head *head, pid_t pid,
-				u16 misc, int fd)
+				u16 misc, struct feat_fd *fd)
 {
 	struct dso *pos;
 
@@ -228,7 +280,8 @@ static int __dsos__write_buildid_table(struct list_head *head, pid_t pid,
 	return 0;
 }
 
-static int machine__write_buildid_table(struct machine *machine, int fd)
+static int machine__write_buildid_table(struct machine *machine,
+					struct feat_fd *fd)
 {
 	int err;
 	u16 kmisc = PERF_RECORD_MISC_KERNEL,
@@ -247,7 +300,8 @@ static int machine__write_buildid_table(struct machine *machine, int fd)
 	return err;
 }
 
-static int dsos__write_buildid_table(struct perf_header *header, int fd)
+static int dsos__write_buildid_table(struct perf_header *header,
+				     struct feat_fd *fd)
 {
 	struct perf_session *session = container_of(header,
 			struct perf_session, header);
@@ -437,25 +491,24 @@ static bool perf_session__read_build_ids(struct perf_session *session, bool with
 	return ret;
 }
 
-static int write_tracing_data(int fd, struct perf_header *h __used,
-			    struct perf_evlist *evlist)
+static int write_tracing_data(struct feat_fd *fd, struct perf_evlist *evlist)
 {
-	return read_tracing_data(fd, &evlist->entries);
+	return read_tracing_data(fd->fd, &evlist->entries);
 }
 
 
-static int write_build_id(int fd, struct perf_header *h,
+static int write_build_id(struct feat_fd *fd,
 			  struct perf_evlist *evlist __used)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(h, struct perf_session, header);
+	session = container_of(fd->hdr, struct perf_session, header);
 
 	if (!perf_session__read_build_ids(session, true))
 		return -1;
 
-	err = dsos__write_buildid_table(h, fd);
+	err = dsos__write_buildid_table(fd->hdr, fd);
 	if (err < 0) {
 		pr_debug("failed to write buildid table\n");
 		return err;
@@ -466,8 +519,7 @@ static int write_build_id(int fd, struct perf_header *h,
 	return 0;
 }
 
-static int write_hostname(int fd, struct perf_header *h __used,
-			  struct perf_evlist *evlist __used)
+static int write_hostname(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 	struct utsname uts;
 	int ret;
@@ -479,7 +531,7 @@ static int write_hostname(int fd, struct perf_header *h __used,
 	return do_write_string(fd, uts.nodename);
 }
 
-static int write_osrelease(int fd, struct perf_header *h __used,
+static int write_osrelease(struct feat_fd *fd,
 			   struct perf_evlist *evlist __used)
 {
 	struct utsname uts;
@@ -492,8 +544,7 @@ static int write_osrelease(int fd, struct perf_header *h __used,
 	return do_write_string(fd, uts.release);
 }
 
-static int write_arch(int fd, struct perf_header *h __used,
-		      struct perf_evlist *evlist __used)
+static int write_arch(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 	struct utsname uts;
 	int ret;
@@ -505,14 +556,12 @@ static int write_arch(int fd, struct perf_header *h __used,
 	return do_write_string(fd, uts.machine);
 }
 
-static int write_version(int fd, struct perf_header *h __used,
-			 struct perf_evlist *evlist __used)
+static int write_version(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 	return do_write_string(fd, perf_version_string);
 }
 
-static int write_cpudesc(int fd, struct perf_header *h __used,
-		       struct perf_evlist *evlist __used)
+static int write_cpudesc(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 #ifndef CPUINFO_PROC
 #define CPUINFO_PROC NULL
@@ -570,8 +619,7 @@ static int write_cpudesc(int fd, struct perf_header *h __used,
 	return ret;
 }
 
-static int write_nrcpus(int fd, struct perf_header *h __used,
-			struct perf_evlist *evlist __used)
+static int write_nrcpus(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 	long nr;
 	u32 nrc, nra;
@@ -596,8 +644,7 @@ static int write_nrcpus(int fd, struct perf_header *h __used,
 	return do_write(fd, &nra, sizeof(nra));
 }
 
-static int write_event_desc(int fd, struct perf_header *h __used,
-			    struct perf_evlist *evlist)
+static int write_event_desc(struct feat_fd *fd, struct perf_evlist *evlist)
 {
 	struct perf_evsel *attr;
 	u32 nre = 0, nri, sz;
@@ -654,8 +701,7 @@ static int write_event_desc(int fd, struct perf_header *h __used,
 	return 0;
 }
 
-static int write_cmdline(int fd, struct perf_header *h __used,
-			 struct perf_evlist *evlist __used)
+static int write_cmdline(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 	char buf[MAXPATHLEN];
 	char proc[32];
@@ -823,8 +869,8 @@ static struct cpu_topo *build_cpu_topology(void)
 	return tp;
 }
 
-static int write_cpu_topology(int fd, struct perf_header *h __used,
-			  struct perf_evlist *evlist __used)
+static int write_cpu_topology(struct feat_fd *fd,
+			      struct perf_evlist *evlist __used)
 {
 	struct cpu_topo *tp;
 	u32 i;
@@ -857,10 +903,8 @@ static int write_cpu_topology(int fd, struct perf_header *h __used,
 	return ret;
 }
 
-
-
-static int write_total_mem(int fd, struct perf_header *h __used,
-			  struct perf_evlist *evlist __used)
+static int write_total_mem(struct feat_fd *fd,
+			   struct perf_evlist *evlist __used)
 {
 	char *buf = NULL;
 	FILE *fp;
@@ -887,7 +931,7 @@ static int write_total_mem(int fd, struct perf_header *h __used,
 	return ret;
 }
 
-static int write_topo_node(int fd, int node)
+static int write_topo_node(struct feat_fd *fd, int node)
 {
 	char str[MAXPATHLEN];
 	char field[32];
@@ -945,8 +989,8 @@ static int write_topo_node(int fd, int node)
 	return ret;
 }
 
-static int write_numa_topology(int fd, struct perf_header *h __used,
-			  struct perf_evlist *evlist __used)
+static int write_numa_topology(struct feat_fd *fd,
+			       struct perf_evlist *evlist __used)
 {
 	char *buf = NULL;
 	size_t len = 0;
@@ -1003,8 +1047,7 @@ int __attribute__((weak)) get_cpuid(char *buffer __used, size_t sz __used)
 	return -1;
 }
 
-static int write_cpuid(int fd, struct perf_header *h __used,
-		       struct perf_evlist *evlist __used)
+static int write_cpuid(struct feat_fd *fd, struct perf_evlist *evlist __used)
 {
 	char buffer[64];
 	int ret;
@@ -1018,128 +1061,128 @@ static int write_cpuid(int fd, struct perf_header *h __used,
 	return do_write_string(fd, buffer);
 }
 
-static int write_branch_stack(int fd __used, struct perf_header *h __used,
-		       struct perf_evlist *evlist __used)
+static int write_branch_stack(struct feat_fd *fd __used,
+			      struct perf_evlist *evlist __used)
 {
 	return 0;
 }
 
-static void print_hostname(struct perf_header *ph, int fd, FILE *fp)
+static void print_hostname(struct feat_fd *fd, FILE *fp)
 {
-	char *str = do_read_string(fd, ph);
+	char *str = do_read_string(fd);
 	fprintf(fp, "# hostname : %s\n", str);
 	free(str);
 }
 
-static void print_osrelease(struct perf_header *ph, int fd, FILE *fp)
+static void print_osrelease(struct feat_fd *fd, FILE *fp)
 {
-	char *str = do_read_string(fd, ph);
+	char *str = do_read_string(fd);
 	fprintf(fp, "# os release : %s\n", str);
 	free(str);
 }
 
-static void print_arch(struct perf_header *ph, int fd, FILE *fp)
+static void print_arch(struct feat_fd *fd, FILE *fp)
 {
-	char *str = do_read_string(fd, ph);
+	char *str = do_read_string(fd);
 	fprintf(fp, "# arch : %s\n", str);
 	free(str);
 }
 
-static void print_cpudesc(struct perf_header *ph, int fd, FILE *fp)
+static void print_cpudesc(struct feat_fd *fd, FILE *fp)
 {
-	char *str = do_read_string(fd, ph);
+	char *str = do_read_string(fd);
 	fprintf(fp, "# cpudesc : %s\n", str);
 	free(str);
 }
 
-static void print_nrcpus(struct perf_header *ph, int fd, FILE *fp)
+static void print_nrcpus(struct feat_fd *fd, FILE *fp)
 {
 	ssize_t ret;
 	u32 nr;
 
-	ret = read(fd, &nr, sizeof(nr));
+	ret = do_read(fd, &nr, sizeof(nr));
 	if (ret != (ssize_t)sizeof(nr))
 		nr = -1; /* interpreted as error */
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		nr = bswap_32(nr);
 
 	fprintf(fp, "# nrcpus online : %u\n", nr);
 
-	ret = read(fd, &nr, sizeof(nr));
+	ret = do_read(fd, &nr, sizeof(nr));
 	if (ret != (ssize_t)sizeof(nr))
 		nr = -1; /* interpreted as error */
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		nr = bswap_32(nr);
 
 	fprintf(fp, "# nrcpus avail : %u\n", nr);
 }
 
-static void print_version(struct perf_header *ph, int fd, FILE *fp)
+static void print_version(struct feat_fd *fd, FILE *fp)
 {
-	char *str = do_read_string(fd, ph);
+	char *str = do_read_string(fd);
 	fprintf(fp, "# perf version : %s\n", str);
 	free(str);
 }
 
-static void print_cmdline(struct perf_header *ph, int fd, FILE *fp)
+static void print_cmdline(struct feat_fd *fd, FILE *fp)
 {
 	ssize_t ret;
 	char *str;
 	u32 nr, i;
 
-	ret = read(fd, &nr, sizeof(nr));
+	ret = do_read(fd, &nr, sizeof(nr));
 	if (ret != (ssize_t)sizeof(nr))
 		return;
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		nr = bswap_32(nr);
 
 	fprintf(fp, "# cmdline : ");
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		fprintf(fp, "%s ", str);
 		free(str);
 	}
 	fputc('\n', fp);
 }
 
-static void print_cpu_topology(struct perf_header *ph, int fd, FILE *fp)
+static void print_cpu_topology(struct feat_fd *fd, FILE *fp)
 {
 	ssize_t ret;
 	u32 nr, i;
 	char *str;
 
-	ret = read(fd, &nr, sizeof(nr));
+	ret = do_read(fd, &nr, sizeof(nr));
 	if (ret != (ssize_t)sizeof(nr))
 		return;
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		nr = bswap_32(nr);
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		fprintf(fp, "# sibling cores   : %s\n", str);
 		free(str);
 	}
 
-	ret = read(fd, &nr, sizeof(nr));
+	ret = do_read(fd, &nr, sizeof(nr));
 	if (ret != (ssize_t)sizeof(nr))
 		return;
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		nr = bswap_32(nr);
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		fprintf(fp, "# sibling threads : %s\n", str);
 		free(str);
 	}
 }
 
-static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
+static void print_event_desc(struct feat_fd *fd, FILE *fp)
 {
 	struct perf_event_attr attr;
 	uint64_t id;
@@ -1150,18 +1193,18 @@ static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
 	size_t msz;
 
 	/* number of events */
-	ret = read(fd, &nre, sizeof(nre));
+	ret = do_read(fd, &nre, sizeof(nre));
 	if (ret != (ssize_t)sizeof(nre))
 		goto error;
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		nre = bswap_32(nre);
 
-	ret = read(fd, &sz, sizeof(sz));
+	ret = do_read(fd, &sz, sizeof(sz));
 	if (ret != (ssize_t)sizeof(sz))
 		goto error;
 
-	if (ph->needs_swap)
+	if (fd->needs_swap)
 		sz = bswap_32(sz);
 
 	memset(&attr, 0, sizeof(attr));
@@ -1176,28 +1219,27 @@ static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
 		msz = sz;
 
 	for (i = 0 ; i < nre; i++) {
-
 		/*
 		 * must read entire on-file attr struct to
 		 * sync up with layout.
 		 */
-		ret = read(fd, buf, sz);
+		ret = do_read(fd, buf, sz);
 		if (ret != (ssize_t)sz)
 			goto error;
 
-		if (ph->needs_swap)
+		if (fd->needs_swap)
 			perf_event__attr_swap(buf);
 
 		memcpy(&attr, buf, msz);
 
-		ret = read(fd, &nr, sizeof(nr));
+		ret = do_read(fd, &nr, sizeof(nr));
 		if (ret != (ssize_t)sizeof(nr))
 			goto error;
 
-		if (ph->needs_swap)
+		if (fd->needs_swap)
 			nr = bswap_32(nr);
 
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		fprintf(fp, "# event : name = %s, ", str);
 		free(str);
 
@@ -1216,11 +1258,11 @@ static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
 			fprintf(fp, ", id = {");
 
 		for (j = 0 ; j < nr; j++) {
-			ret = read(fd, &id, sizeof(id));
+			ret = do_read(fd, &id, sizeof(id));
 			if (ret != (ssize_t)sizeof(id))
 				goto error;
 
-			if (ph->needs_swap)
+			if (fd->needs_swap)
 				id = bswap_64(id);
 
 			if (j)
@@ -1238,16 +1280,16 @@ static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
 	fprintf(fp, "# event desc: not available or unable to read\n");
 }
 
-static void print_total_mem(struct perf_header *h __used, int fd, FILE *fp)
+static void print_total_mem(struct feat_fd *fd, FILE *fp)
 {
 	uint64_t mem;
 	ssize_t ret;
 
-	ret = read(fd, &mem, sizeof(mem));
+	ret = do_read(fd, &mem, sizeof(mem));
 	if (ret != sizeof(mem))
 		goto error;
 
-	if (h->needs_swap)
+	if (fd->needs_swap)
 		mem = bswap_64(mem);
 
 	fprintf(fp, "# total memory : %"PRIu64" kB\n", mem);
@@ -1256,7 +1298,7 @@ static void print_total_mem(struct perf_header *h __used, int fd, FILE *fp)
 	fprintf(fp, "# total memory : unknown\n");
 }
 
-static void print_numa_topology(struct perf_header *h __used, int fd, FILE *fp)
+static void print_numa_topology(struct feat_fd *fd, FILE *fp)
 {
 	ssize_t ret;
 	u32 nr, c, i;
@@ -1264,32 +1306,32 @@ static void print_numa_topology(struct perf_header *h __used, int fd, FILE *fp)
 	uint64_t mem_total, mem_free;
 
 	/* nr nodes */
-	ret = read(fd, &nr, sizeof(nr));
+	ret = do_read(fd, &nr, sizeof(nr));
 	if (ret != (ssize_t)sizeof(nr))
 		goto error;
 
-	if (h->needs_swap)
+	if (fd->needs_swap)
 		nr = bswap_32(nr);
 
 	for (i = 0; i < nr; i++) {
 
 		/* node number */
-		ret = read(fd, &c, sizeof(c));
+		ret = do_read(fd, &c, sizeof(c));
 		if (ret != (ssize_t)sizeof(c))
 			goto error;
 
-		if (h->needs_swap)
+		if (fd->needs_swap)
 			c = bswap_32(c);
 
-		ret = read(fd, &mem_total, sizeof(u64));
+		ret = do_read(fd, &mem_total, sizeof(u64));
 		if (ret != sizeof(u64))
 			goto error;
 
-		ret = read(fd, &mem_free, sizeof(u64));
+		ret = do_read(fd, &mem_free, sizeof(u64));
 		if (ret != sizeof(u64))
 			goto error;
 
-		if (h->needs_swap) {
+		if (fd->needs_swap) {
 			mem_total = bswap_64(mem_total);
 			mem_free = bswap_64(mem_free);
 		}
@@ -1300,7 +1342,7 @@ static void print_numa_topology(struct perf_header *h __used, int fd, FILE *fp)
 			mem_total,
 			mem_free);
 
-		str = do_read_string(fd, h);
+		str = do_read_string(fd);
 		fprintf(fp, "# node%u cpu list : %s\n", c, str);
 		free(str);
 	}
@@ -1309,15 +1351,14 @@ static void print_numa_topology(struct perf_header *h __used, int fd, FILE *fp)
 	fprintf(fp, "# numa topology : not available\n");
 }
 
-static void print_cpuid(struct perf_header *ph, int fd, FILE *fp)
+static void print_cpuid(struct feat_fd *fd, FILE *fp)
 {
-	char *str = do_read_string(fd, ph);
+	char *str = do_read_string(fd);
 	fprintf(fp, "# cpuid : %s\n", str);
 	free(str);
 }
 
-static void print_branch_stack(struct perf_header *ph __used, int fd __used,
-			       FILE *fp)
+static void print_branch_stack(struct feat_fd *fd __used, FILE *fp)
 {
 	fprintf(fp, "# contains samples with branch stack\n");
 }
@@ -1490,43 +1531,88 @@ static int process_build_id(struct perf_file_section *section,
 }
 
 struct feature_ops {
-	int (*write)(int fd, struct perf_header *h, struct perf_evlist *evlist);
-	void (*print)(struct perf_header *h, int fd, FILE *fp);
+	int (*write)(struct feat_fd *fd, struct perf_evlist *evlist);
+	void (*print)(struct feat_fd *fd, FILE *fp);
 	int (*process)(struct perf_file_section *section,
 		       struct perf_header *h, int feat, int fd);
 	const char *name;
 	bool full_only;
+	int record_type;
 };
 
-#define FEAT_OPA(n, func) \
-	[n] = { .name = #n, .write = write_##func, .print = print_##func }
-#define FEAT_OPP(n, func) \
-	[n] = { .name = #n, .write = write_##func, .print = print_##func, \
-		.process = process_##func }
-#define FEAT_OPF(n, func) \
-	[n] = { .name = #n, .write = write_##func, .print = print_##func, \
-		.full_only = true }
+#define FEAT_OPA(n, func)				\
+	[HEADER_##n] = { .name = #n,			\
+		.write = write_##func,			\
+		.print = print_##func,			\
+		.full_only = false,			\
+		.record_type = PERF_RECORD_HEADER_##n	\
+	}
+
+#define FEAT_OPB(n, func)                               \
+	[HEADER_##n] = { .name = #n,			\
+		.write = write_##func,			\
+		.print = print_##func,			\
+		.full_only = false,			\
+	}
+
+#define FEAT_OPP(n, func)				\
+	[HEADER_##n] = { .name = #n,			\
+		.write = write_##func,			\
+		.print = print_##func,			\
+		.process = process_##func,		\
+		.record_type = PERF_RECORD_HEADER_##n	\
+	}
+
+#define FEAT_OPF(n, func)				\
+	[HEADER_##n] = { .name = #n,			\
+		.write = write_##func,			\
+		.print = print_##func,			\
+		.full_only = true,			\
+		.record_type = PERF_RECORD_HEADER_##n	\
+	}
 
 /* feature_ops not implemented: */
 #define print_tracing_data	NULL
 #define print_build_id		NULL
 
 static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
-	FEAT_OPP(HEADER_TRACING_DATA,	tracing_data),
-	FEAT_OPP(HEADER_BUILD_ID,	build_id),
-	FEAT_OPA(HEADER_HOSTNAME,	hostname),
-	FEAT_OPA(HEADER_OSRELEASE,	osrelease),
-	FEAT_OPA(HEADER_VERSION,	version),
-	FEAT_OPA(HEADER_ARCH,		arch),
-	FEAT_OPA(HEADER_NRCPUS,		nrcpus),
-	FEAT_OPA(HEADER_CPUDESC,	cpudesc),
-	FEAT_OPA(HEADER_CPUID,		cpuid),
-	FEAT_OPA(HEADER_TOTAL_MEM,	total_mem),
-	FEAT_OPA(HEADER_EVENT_DESC,	event_desc),
-	FEAT_OPA(HEADER_CMDLINE,	cmdline),
-	FEAT_OPF(HEADER_CPU_TOPOLOGY,	cpu_topology),
-	FEAT_OPF(HEADER_NUMA_TOPOLOGY,	numa_topology),
-	FEAT_OPA(HEADER_BRANCH_STACK,	branch_stack),
+	FEAT_OPP(TRACING_DATA,	tracing_data),
+	FEAT_OPP(BUILD_ID,	build_id),
+	FEAT_OPA(HOSTNAME,	hostname),
+	FEAT_OPA(OSRELEASE,	osrelease),
+	FEAT_OPA(VERSION,	version),
+	FEAT_OPA(ARCH,		arch),
+	FEAT_OPA(NRCPUS,	nrcpus),
+	FEAT_OPA(CPUDESC,	cpudesc),
+	FEAT_OPA(CPUID,		cpuid),
+	FEAT_OPA(TOTAL_MEM,	total_mem),
+	FEAT_OPA(EVENT_DESC,	event_desc),
+	FEAT_OPA(CMDLINE,	cmdline),
+	FEAT_OPF(CPU_TOPOLOGY,	cpu_topology),
+	FEAT_OPF(NUMA_TOPOLOGY,	numa_topology),
+	FEAT_OPB(BRANCH_STACK,	branch_stack),
+};
+
+/*
+ * we use a mapping table to go from record type to feature header
+ * because we have no guarantee that new record types may not be added
+ * after the feature header.
+ */
+#define REC2FEAT(a)	[PERF_RECORD_HEADER_##a] = HEADER_##a
+
+static const int rec2feat[PERF_RECORD_HEADER_MAX] = {
+	REC2FEAT(HOSTNAME),
+	REC2FEAT(OSRELEASE),
+	REC2FEAT(VERSION),
+	REC2FEAT(ARCH),
+	REC2FEAT(NRCPUS),
+	REC2FEAT(CPUDESC),
+	REC2FEAT(CPUID),
+	REC2FEAT(TOTAL_MEM),
+	REC2FEAT(EVENT_DESC),
+	REC2FEAT(CMDLINE),
+	REC2FEAT(CPU_TOPOLOGY),
+	REC2FEAT(NUMA_TOPOLOGY),
 };
 
 struct header_print_data {
@@ -1539,6 +1625,7 @@ static int perf_file_section__fprintf_info(struct perf_file_section *section,
 					   int feat, int fd, void *data)
 {
 	struct header_print_data *hd = data;
+	struct feat_fd fdd;
 
 	if (lseek(fd, section->offset, SEEK_SET) == (off_t)-1) {
 		pr_debug("Failed to lseek to %" PRIu64 " offset for feature "
@@ -1552,8 +1639,19 @@ static int perf_file_section__fprintf_info(struct perf_file_section *section,
 	if (!feat_ops[feat].print)
 		return 0;
 
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.fd = fd;
+	fdd.hdr = ph;
+	fdd.needs_swap = ph->needs_swap;
+
+	/*
+	 * propagate endianess setting to fdd
+	 * in pipe-mode there is no perf_header
+	 */
+	fdd.needs_swap = ph->needs_swap;
+
 	if (!feat_ops[feat].full_only || hd->full)
-		feat_ops[feat].print(ph, fd, hd->fp);
+		feat_ops[feat].print(&fdd, hd->fp);
 	else
 		fprintf(hd->fp, "# %s info available, use -I to display\n",
 			feat_ops[feat].name);
@@ -1574,44 +1672,51 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
 	return 0;
 }
 
-static int do_write_feat(int fd, struct perf_header *h, int type,
+static int do_write_feat(struct feat_fd *fdd, int type,
 			 struct perf_file_section **p,
 			 struct perf_evlist *evlist)
 {
 	int err;
-	int ret = 0;
 
-	if (perf_header__has_feat(h, type)) {
+	if (perf_header__has_feat(fdd->hdr, type)) {
 		if (!feat_ops[type].write)
 			return -1;
 
-		(*p)->offset = lseek(fd, 0, SEEK_CUR);
+		(*p)->offset = lseek(fdd->fd, 0, SEEK_CUR);
 
-		err = feat_ops[type].write(fd, h, evlist);
+		err = feat_ops[type].write(fdd, evlist);
 		if (err < 0) {
 			pr_debug("failed to write feature %d\n", type);
-
-			/* undo anything written */
-			lseek(fd, (*p)->offset, SEEK_SET);
-
+			lseek(fdd->fd, (*p)->offset, SEEK_SET);
 			return -1;
+
 		}
-		(*p)->size = lseek(fd, 0, SEEK_CUR) - (*p)->offset;
+		(*p)->size = lseek(fdd->fd, 0, SEEK_CUR) - (*p)->offset;
 		(*p)++;
 	}
-	return ret;
+	return 0;
 }
 
 static int perf_header__adds_write(struct perf_header *header,
 				   struct perf_evlist *evlist, int fd)
 {
 	int nr_sections;
+	struct feat_fd fdd;
 	struct perf_file_section *feat_sec, *p;
 	int sec_size;
 	u64 sec_start;
 	int feat;
 	int err;
 
+	/*
+	 * may write more than needed due to dropped feature, but
+	 * this is okay, reader will skip the mising entries
+	 */
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.fd  = fd;
+	fdd.hdr = header;
+	fdd.needs_swap = header->needs_swap;
+
 	nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
 	if (!nr_sections)
 		return 0;
@@ -1626,38 +1731,40 @@ static int perf_header__adds_write(struct perf_header *header,
 	lseek(fd, sec_start + sec_size, SEEK_SET);
 
 	for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
-		if (do_write_feat(fd, header, feat, &p, evlist))
+		if (do_write_feat(&fdd, feat, &p, evlist))
 			perf_header__clear_feat(header, feat);
 	}
 
 	lseek(fd, sec_start, SEEK_SET);
-	/*
-	 * may write more than needed due to dropped feature, but
-	 * this is okay, reader will skip the mising entries
-	 */
-	err = do_write(fd, feat_sec, sec_size);
+
+	err = do_write(&fdd, feat_sec, sec_size);
 	if (err < 0)
 		pr_debug("failed to write feature section\n");
+
 	free(feat_sec);
+
 	return err;
 }
 
 int perf_header__write_pipe(int fd)
 {
 	struct perf_pipe_file_header f_header;
+	struct feat_fd fdd;
 	int err;
 
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.fd = fd;
+
 	f_header = (struct perf_pipe_file_header){
 		.magic	   = PERF_MAGIC,
 		.size	   = sizeof(f_header),
 	};
 
-	err = do_write(fd, &f_header, sizeof(f_header));
+	err = do_write(&fdd, &f_header, sizeof(f_header));
 	if (err < 0) {
 		pr_debug("failed to write perf pipe header\n");
 		return err;
 	}
-
 	return 0;
 }
 
@@ -1665,12 +1772,16 @@ int perf_session__write_header(struct perf_session *session,
 			       struct perf_evlist *evlist,
 			       int fd, bool at_exit)
 {
+	struct feat_fd fdd;
 	struct perf_file_header f_header;
 	struct perf_file_attr   f_attr;
 	struct perf_header *header = &session->header;
 	struct perf_evsel *attr, *pair = NULL;
 	int err;
 
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.fd = fd;
+
 	lseek(fd, sizeof(f_header), SEEK_SET);
 
 	if (session->evlist != evlist)
@@ -1678,14 +1789,14 @@ int perf_session__write_header(struct perf_session *session,
 
 	list_for_each_entry(attr, &evlist->entries, node) {
 		attr->id_offset = lseek(fd, 0, SEEK_CUR);
-		err = do_write(fd, attr->id, attr->ids * sizeof(u64));
+		err = do_write(&fdd, attr->id, attr->ids * sizeof(u64));
 		if (err < 0) {
 out_err_write:
 			pr_debug("failed to write perf header\n");
 			return err;
 		}
 		if (session->evlist != evlist) {
-			err = do_write(fd, pair->id, pair->ids * sizeof(u64));
+			err = do_write(&fdd, pair->id, pair->ids * sizeof(u64));
 			if (err < 0)
 				goto out_err_write;
 			attr->ids += pair->ids;
@@ -1703,7 +1814,7 @@ int perf_session__write_header(struct perf_session *session,
 				.size   = attr->ids * sizeof(u64),
 			}
 		};
-		err = do_write(fd, &f_attr, sizeof(f_attr));
+		err = do_write(&fdd, &f_attr, sizeof(f_attr));
 		if (err < 0) {
 			pr_debug("failed to write perf header attribute\n");
 			return err;
@@ -1713,7 +1824,7 @@ int perf_session__write_header(struct perf_session *session,
 	header->event_offset = lseek(fd, 0, SEEK_CUR);
 	header->event_size = event_count * sizeof(struct perf_trace_event_type);
 	if (events) {
-		err = do_write(fd, events, header->event_size);
+		err = do_write(&fdd, events, header->event_size);
 		if (err < 0) {
 			pr_debug("failed to write perf header events\n");
 			return err;
@@ -1749,7 +1860,7 @@ int perf_session__write_header(struct perf_session *session,
 	memcpy(&f_header.adds_features, &header->adds_features, sizeof(header->adds_features));
 
 	lseek(fd, 0, SEEK_SET);
-	err = do_write(fd, &f_header, sizeof(f_header));
+	err = do_write(&fdd, &f_header, sizeof(f_header));
 	if (err < 0) {
 		pr_debug("failed to write perf header\n");
 		return err;
@@ -2009,8 +2120,13 @@ static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
 				       struct perf_header *ph, int fd,
 				       bool repipe)
 {
+	struct feat_fd fdd;
 	int ret;
 
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.fd  = STDOUT_FILENO;
+	fdd.hdr = ph;
+
 	ret = readn(fd, header, sizeof(*header));
 	if (ret <= 0)
 		return -1;
@@ -2023,7 +2139,7 @@ static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
 	if (ph->needs_swap)
 		header->size = bswap_64(header->size);
 
-	if (repipe && do_write(STDOUT_FILENO, header, sizeof(*header)) < 0)
+	if (repipe && do_write(&fdd, header, sizeof(*header)) < 0)
 		return -1;
 
 	return 0;
@@ -2318,6 +2434,58 @@ int perf_event__synthesize_event_types(struct perf_tool *tool,
 	return err;
 }
 
+int perf_event__synthesize_features(struct perf_tool *tool,
+				    struct perf_session *session,
+				    struct perf_evlist *evlist,
+				    perf_event__handler_t process)
+{
+	struct perf_header *header = &session->header;
+	struct feat_fd fdd;
+	struct feature_event *fe;
+	size_t sz, sz_hdr;
+	int feat, ret;
+
+	sz_hdr = sizeof(fe->header);
+	sz = sizeof(union perf_event);
+	/* get a nice alignment */
+	sz = ALIGN(sz, getpagesize());
+
+	memset(&fdd, 0, sizeof(fdd));
+
+	fdd.buf = malloc(sz);
+	if (!fdd.buf)
+		return -1;
+
+	fdd.size = sz - sz_hdr;
+
+	for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) {
+		/*
+		 * those are already written as:
+		 * - PERF_RECORD_HEADER_TRACING_DATA
+		 * - PERF_RECORD_HEADER_BUILD_ID
+		 */
+		if (feat == HEADER_TRACING_DATA || feat == HEADER_BUILD_ID)
+			continue;
+
+		fdd.pos = sizeof(*fe);
+
+		ret = feat_ops[feat].write(&fdd, evlist);
+		if (ret || fdd.pos == sizeof(*fe))
+			continue;
+
+		/* fdd.buf may change due to realloc in do_write() */
+		fe = fdd.buf;
+		memset(fe, 0, sizeof(*fe));
+
+		fe->header.type = feat_ops[feat].record_type;
+		fe->header.size = fdd.pos;
+
+		process(tool, fdd.buf, NULL, NULL);
+	}
+	free(fdd.buf);
+	return 0;
+}
+
 int perf_event__process_event_type(struct perf_tool *tool __unused,
 				   union perf_event *event)
 {
@@ -2333,6 +2501,7 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 					perf_event__handler_t process)
 {
 	union perf_event ev;
+	struct feat_fd fdd;
 	struct tracing_data *tdata;
 	ssize_t size = 0, aligned_size = 0, padding;
 	int err __used = 0;
@@ -2369,7 +2538,10 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 	 */
 	tracing_data_put(tdata);
 
-	write_padded(fd, NULL, 0, padding);
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.fd = fd;
+
+	write_padded(&fdd, NULL, 0, padding);
 
 	return aligned_size;
 }
@@ -2441,6 +2613,52 @@ int perf_event__process_build_id(struct perf_tool *tool __used,
 	return 0;
 }
 
+int perf_event__process_feature(struct perf_tool *tool,
+				union perf_event *event,
+				 struct perf_session *session __used)
+{
+	struct feat_fd fdd;
+	int type = event->header.type;
+	int feat;
+
+	if (type < 0 || type >= PERF_RECORD_HEADER_MAX) {
+		pr_warning("invalid record type %d\n", type);
+		return 0;
+	}
+	/*
+	 * unhandled feature
+	 */
+	feat = rec2feat[type];
+	if (feat == 0)
+		return 0;
+
+	/*
+	 * no priunt routine
+	 */
+	if (!feat_ops[feat].print)
+		return 0;
+
+	memset(&fdd, 0, sizeof(fdd));
+	fdd.buf  = (void *)event;
+	fdd.buf += sizeof(event->header);
+	fdd.size = event->header.size - sizeof(event->header);
+	/*
+	 * have to assume endianess match for now
+	 * as we do not have the info coming from the header.
+	 *
+	 * Once header is updated, we'll use the info here
+	 */
+	fdd.needs_swap = 0;
+
+	if (!feat_ops[feat].full_only || tool->show_all_features)
+		feat_ops[feat].print(&fdd, stdout);
+	else
+		fprintf(stdout, "# %s info available, use -I to display\n",
+			feat_ops[feat].name);
+
+	return 0;
+}
+
 void disable_buildid_cache(void)
 {
 	no_buildid_cache = true;
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 2d42b3e..af4db9e 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -98,6 +98,15 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 			  const char *name, bool is_kallsyms);
 int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir);
 
+int perf_event__synthesize_features(struct perf_tool *tool,
+				    struct perf_session *session,
+				    struct perf_evlist *evlist,
+				    perf_event__handler_t process);
+
+int perf_event__process_feature(struct perf_tool *tool,
+				union perf_event *event,
+				 struct perf_session *session);
+
 int perf_event__synthesize_attr(struct perf_tool *tool,
 				struct perf_event_attr *attr, u16 ids, u64 *id,
 				perf_event__handler_t process);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index af6a5f0..8e5d74d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -390,6 +390,14 @@ static int process_event_type_stub(struct perf_tool *tool __used,
 	return 0;
 }
 
+static int process_feature_stub(struct perf_tool *tool __used,
+				union perf_event *event __used,
+				struct perf_session *session __used)
+{
+	dump_printf(": unhandled!\n");
+	return 0;
+}
+
 static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event,
 				  struct perf_session *session);
@@ -428,6 +436,8 @@ static void perf_tool__fill_defaults(struct perf_tool *tool)
 		else
 			tool->finished_round = process_finished_round_stub;
 	}
+	if (tool->feature == NULL)
+		tool->feature = process_feature_stub;
 }
 
 void mem_bswap_64(void *src, int byte_size)
@@ -530,8 +540,8 @@ static perf_event__swap_op perf_event__swap_ops[] = {
 	[PERF_RECORD_SAMPLE]		  = perf_event__all64_swap,
 	[PERF_RECORD_HEADER_ATTR]	  = perf_event__hdr_attr_swap,
 	[PERF_RECORD_HEADER_EVENT_TYPE]	  = perf_event__event_type_swap,
-	[PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap,
 	[PERF_RECORD_HEADER_BUILD_ID]	  = NULL,
+	[PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap,
 	[PERF_RECORD_HEADER_MAX]	  = NULL,
 };
 
@@ -947,6 +957,8 @@ static int perf_session__process_user_event(struct perf_session *session, union
 		return tool->build_id(tool, event, session);
 	case PERF_RECORD_FINISHED_ROUND:
 		return tool->finished_round(tool, event, session);
+	case PERF_RECORD_HEADER_HOSTNAME ... PERF_RECORD_HEADER_NUMA_TOPOLOGY:
+		return tool->feature(tool, event, session);
 	default:
 		return -EINVAL;
 	}
@@ -1509,6 +1521,7 @@ void perf_session__fprintf_info(struct perf_session *session, FILE *fp,
 
 	fprintf(fp, "# ========\n");
 	fprintf(fp, "# captured on: %s", ctime(&st.st_ctime));
-	perf_header__fprintf_info(session, fp, full);
 	fprintf(fp, "# ========\n#\n");
+	if (!session->fd_pipe)
+		perf_header__fprintf_info(session, fp, full);
 }
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index b0e1aad..af16af1f 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -42,9 +42,11 @@ struct perf_tool {
 	event_synth_op	tracing_data;
 	event_simple_op	event_type;
 	event_op2	finished_round,
-			build_id;
+			build_id,
+			feature;
 	bool		ordered_samples;
 	bool		ordering_requires_timestamps;
+	bool		show_all_features;
 };
 
 #endif /* __PERF_TOOL_H */
-- 
1.7.4.1

[PATCH v2 5/5] perf: make perf buildid-list work better with pipe mode

[Stephane Eranian]

In order for perf buildid-list to work with pipe-mode files,
it needs to process buildids and event attr structs.

$ perf record -o - noploop 2 | ./perf inject -b | perf buildid-list -i - -H
noploop for 2 seconds
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.084 MB - (~3678 samples) ]
0000000000000000000000000000000000000000 [kernel.kallsyms]
3a0d0629efe74a8da3eeba372cdbd74ad9b8f5d5 /usr/local/bin/noploop

The reason [kernel.kallsyms] shows a 0 build-id comes from the
way buildids are injected in the stream. The buildid for the kernel
is provided by a BUILD_ID record. The [kernel.kallsyms] is provided
by a MMAP record. There is no clean and obvious way to link the two,
unfortunately. In regular mode, the kernel buildid is generated from
reading the ELF image or kallsyms and perf knows to associate
[kernel.kallsyms] to it. Later on, when perf processes the
[kernel.kallsyms] MMAP record, it will already have a dso for it.

So for now, make sure perf buildid-list shows the buildids for
everything but the kernel image.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-buildid-list.c |    6 +++++-
 tools/perf/util/build-id.c        |    2 ++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 5248046..6b2bcfb 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -84,7 +84,11 @@ static int perf_session__list_build_ids(void)
 	if (filename__fprintf_build_id(session->filename, stdout))
 		goto out;
 
-	if (with_hits)
+	/*
+	 * in pipe-mode, the only way to get the buildids is to parse
+	 * the record stream. Buildids are stored as RECORD_HEADER_BUILD_ID
+	 */
+	if (with_hits || session->fd_pipe)
 		perf_session__process_events(session, &build_id__mark_dso_hit_ops);
 
 	perf_session__fprintf_dsos_buildid(session, stdout, with_hits);
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index dff9c7a..fd9a594 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -65,6 +65,8 @@ struct perf_tool build_id__mark_dso_hit_ops = {
 	.mmap	= perf_event__process_mmap,
 	.fork	= perf_event__process_task,
 	.exit	= perf_event__exit_del_thread,
+	.attr		 = perf_event__process_attr,
+	.build_id	 = perf_event__process_build_id,
 };
 
 char *dso__build_id_filename(struct dso *self, char *bf, size_t size)
-- 
1.7.4.1

Re: [PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[Namhyung Kim]

Hi, Stephane

On Tue, 15 May 2012 13:28:10 +0200, Stephane Eranian wrote:
> This patch series adds meta-data support to perf record/report in pipe
> mode:
> 	perf record -o - foo | perf inject -b | perf report -i -
>
> We had meta-data (information about host configuration, perf tool version,...)
> in regular (file) mode but it was lacking in pipe mode. This patch series fixes
> this.
>

In addition to this change, I think it'd be great if we keep this
meta-data somewhere for later use - I guess perf report and
annotate on TUI, at least, will need it too. Just a suggestion :).

Thanks,
Namhyung

Re: [PATCH v2 1/5] perf inject: fix broken perf inject -b

[David Ahern]

On 5/15/12 5:28 AM, Stephane Eranian wrote:
> perf inject -b was broken. It would not inject any build_id
> into the stream. Furthermore, it would strip samples from the
> stream.
>
> The reason was a missing initialization of the event attribute
> structure. The perf_tool.tool.attr() callback was pointing to
> a simple repipe. But there was no initialization of the internal
> data structures to keep track of events and event ids. That later
> caused event id lookups to fail, and sample would get removed.
>
> The patch simply adds back the call to perf_event__process_attr()
> to initialize the evlist structure and now build_ids are again
> injected.
>
> Signed-off-by: Stephane Eranian<eranian@google.com>
> ---
>   tools/perf/builtin-inject.c |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 09c1061..3beab48 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
>   static int perf_event__repipe_attr(union perf_event *event,
>   				   struct perf_evlist **pevlist __used)

The _used can be dropped now that pevlist is used.

>   {
> +	int ret;
> +	ret = perf_event__process_attr(event, pevlist);
> +	if (ret)
> +		return ret;
> +
>   	return perf_event__repipe_synth(NULL, event, NULL);
>   }
>

The change seems right, but I could not recreate the dropped events. In 
both cases running:

perf record -o - -m 256 /tmp/a.sh | perf inject -b | perf report -i - -D

where /tmp/a.sh is:

#!/bin/sh
cd /opt/sw/ahern/kernels/kernel.git
make O=/tmp/kbuild -j 32 >/dev/null 2>&1

just to give it some "real work."

With your patch:
            TOTAL events:     284211
             MMAP events:      69236
             COMM events:      10863
             EXIT events:      22858
         THROTTLE events:         11
       UNTHROTTLE events:         11
             FORK events:      11428
           SAMPLE events:     169779
             ATTR events:          1
       EVENT_TYPE events:          1
         BUILD_ID events:         23
cycles stats:
            TOTAL events:     284186
             MMAP events:      69236
             COMM events:      10863
             EXIT events:      22858
         THROTTLE events:         11
       UNTHROTTLE events:         11
             FORK events:      11428
           SAMPLE events:     169779


Current acme/core:
            TOTAL events:     288990
             MMAP events:      69236
             COMM events:      10863
             EXIT events:      22858
         THROTTLE events:         20
       UNTHROTTLE events:         20
             FORK events:      11428
           SAMPLE events:     174538
             ATTR events:          1
       EVENT_TYPE events:          1
         BUILD_ID events:         25
cycles stats:
            TOTAL events:     288963
             MMAP events:      69236
             COMM events:      10863
             EXIT events:      22858
         THROTTLE events:         20
       UNTHROTTLE events:         20
             FORK events:      11428
           SAMPLE events:     174538


David

[1] Both tests have Namhyung's patch:
https://lkml.org/lkml/2012/5/15/280

Re: [PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[David Ahern]

On 5/15/12 7:34 PM, Namhyung Kim wrote:
> Hi, Stephane
>
> On Tue, 15 May 2012 13:28:10 +0200, Stephane Eranian wrote:
>> This patch series adds meta-data support to perf record/report in pipe
>> mode:
>> 	perf record -o - foo | perf inject -b | perf report -i -
>>
>> We had meta-data (information about host configuration, perf tool version,...)
>> in regular (file) mode but it was lacking in pipe mode. This patch series fixes
>> this.
>>
>
> In addition to this change, I think it'd be great if we keep this
> meta-data somewhere for later use - I guess perf report and
> annotate on TUI, at least, will need it too. Just a suggestion :).
>
> Thanks,
> Namhyung
>

It is there -- see perf-report header. It gets dropped on pipe mode b/c 
the meta-data is in the header.

David

Re: [PATCH v2 2/5] perf tools: fix piped mode read code

[David Ahern]

On 5/15/12 5:28 AM, Stephane Eranian wrote:
> In __perf_session__process_pipe_events(), there was a risk
> we would read more than what a union perf_event struct can
> hold. this could happen in case, perf is reading a file which
> contains new record types it does not know about and which are
> larger than anything it knows about.
>
> In general, perf is supposed to skip records it does not
> understand, but in pipe mode, those have to be read and ignored.
> The fixed size header contains the size of the record, but that
> size may be larger than union perf_event, yet it was used as
> the backing to the read in:
>
>    union perf_event event;
>    void *p;
>
>    size = event->header.size;
>
>    p =&event;
>    p += sizeof(struct perf_event_header);
>    if (size - sizeof(struct perf_event_header)) {
>      err = readn(self->fd, p, size - sizeof(struct perf_event_header));
>
> We fix this by allocating a buffer based on the size reported in
> the header. We reuse the buffer as much as we can. We realloc in
> case it becomes too small. In the  common case, the performance
> impact is negligible.
>
> Signed-off-by: Stephane Eranian<eranian@google.com>
> ---
>   tools/perf/util/session.c |   34 ++++++++++++++++++++++++++--------
>   1 files changed, 26 insertions(+), 8 deletions(-)
>

I don't have a file/perf with different sized events, but the change 
makes sense.

Reviewed-and-tested-by: David Ahern <dsahern@gmail.com>

Re: [PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[Namhyung Kim]

Hi,

On Tue, 15 May 2012 20:05:06 -0600, David Ahern wrote:
> On 5/15/12 7:34 PM, Namhyung Kim wrote:
>> Hi, Stephane
>>
>> On Tue, 15 May 2012 13:28:10 +0200, Stephane Eranian wrote:
>>> This patch series adds meta-data support to perf record/report in pipe
>>> mode:
>>> 	perf record -o - foo | perf inject -b | perf report -i -
>>>
>>> We had meta-data (information about host configuration, perf tool version,...)
>>> in regular (file) mode but it was lacking in pipe mode. This patch series fixes
>>> this.
>>>
>>
>> In addition to this change, I think it'd be great if we keep this
>> meta-data somewhere for later use - I guess perf report and
>> annotate on TUI, at least, will need it too. Just a suggestion :).
>>
>> Thanks,
>> Namhyung
>>
>
> It is there -- see perf-report header. It gets dropped on pipe mode
> b/c the meta-data is in the header.
>
> David

Do you mean this?

struct perf_report {
	struct perf_tool	tool;
	struct perf_session	*session;
	char const		*input_name;
	bool			force, use_tui, use_gtk, use_stdio;
	bool			hide_unresolved;
	bool			dont_use_callchains;
	bool			show_full_info;
	bool			show_threads;
	bool			inverted_callchain;
	struct perf_read_values	show_threads_values;
	const char		*pretty_printing_style;
	symbol_filter_t		annotate_init;
	const char		*cpu_list;
	const char		*symbol_filter_str;
	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
};

Thanks,
Namhyung

Re: [PATCH v2 3/5] perf tools: rename HEADER_TRACE_INFO to HEADER_TRACING_DATA

[David Ahern]

On 5/15/12 5:28 AM, Stephane Eranian wrote:
> To match the PERF_RECORD_HEADER_TRACING_DATA record type.
> This is the same info as the one used for pipe mode whereas
> the other one is for regular file output. This will help in
> the later patch to add meta-data infos in pipe mode.
>
> Signed-off-by: Stephane Eranian<eranian@google.com>

Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[David Ahern]

See tools/perf/util/header.{ch}
Runtime example:

perf script -i /tmp/perf.data -I

# ========
# captured on: Tue May 15 20:22:15 2012
# ========
#
# hostname : nxos-vdc-dev3
# os release : 3.4.0-rc7+
# perf version : 3.4.rc4.137.g978da3.dirty
# arch : x86_64
# nrcpus online : 16
# nrcpus avail : 16
# cpudesc : Intel(R) Xeon(R) CPU E5540 @ 2.53GHz
# cpuid : GenuineIntel,6,26,5
# total memory : 24680324 kB
# cmdline : /tmp/pbuild/perf record -fo /tmp/perf.data -- /tmp/noploop

...

'perf report --stdio' also shows it. It gets lost in the TUI.

David

Re: [PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[Namhyung Kim]

Hi,

On Tue, 15 May 2012 20:38:05 -0600, David Ahern wrote:
> See tools/perf/util/header.{ch}
> Runtime example:
>
> perf script -i /tmp/perf.data -I
>
> # ========
> # captured on: Tue May 15 20:22:15 2012
> # ========
> #
> # hostname : nxos-vdc-dev3
> # os release : 3.4.0-rc7+
> # perf version : 3.4.rc4.137.g978da3.dirty
> # arch : x86_64
> # nrcpus online : 16
> # nrcpus avail : 16
> # cpudesc : Intel(R) Xeon(R) CPU E5540 @ 2.53GHz
> # cpuid : GenuineIntel,6,26,5
> # total memory : 24680324 kB
> # cmdline : /tmp/pbuild/perf record -fo /tmp/perf.data -- /tmp/noploop
>
> ...
>
> 'perf report --stdio' also shows it. It gets lost in the TUI.
>

Yeah, I know. What I mean is keeping the info in memory after parsing a 
perf.data file so that it can be used for displaying it on a dialog in
perf report TUI (on demand) or checking arch before annotation or ...

Thanks,
Namhyung

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[David Ahern]

On 5/15/12 5:28 AM, Stephane Eranian wrote:
> This patch adds the meta-data header support for perf record
> when used in pipe mode: perf record -o -
>
> Up until now, meta-data was only available when perf record
> was used in "regular" mode, i.e., generating a perf.data file.
> For users depending on pipe mode, no host, event header information
> was gathered. This patch addresses this limitation.
>
> The difficulty in pipe mode is that information needs to be written
> sequentially to the pipe. Meta data headers are usually generated
> (and also expected) at the beginning of the file (or piped output).
> To solve this problem, we introduce new synthetic record types,
> one for each meta-data type. The approach is similar to what
> is already used for BUILD_ID and TRACING_DATA.
>
> We have modified util/header.c such that the same routines are used
> to generate and read the meta-data information regardless of pipe-mode
> vs. regular mode. To make this work, we added a new struct called
> feat_fd which encapsulates all the information necessary to read or
> write meta-data information to a file/pipe or from a file/pipe.
>
> It should be noted that there is a limitation with the current
> perf in terms of endianess in pipe mode. Perf assumes the records
> are generated using the same endianess during collection and
> analysis.  That is always the case with the example shown below.
> However, one could also do:
> $ perf record -o - noploop 2 | perf inject -b>perf.data
> $ cat perf.data | perf report -i -
>
> With this patch, it is possible to get:
>   $ perf record -o - noploop 2 | perf inject -b | perf report -i -
>        # ========
>        # captured on: Fri Jan 20 18:13:55 2012
>        # ========
>        #
>        # hostname : quad
>        # os release : 3.2.0-rc7-tip
>        # perf version : 3.2.0
>        # arch : x86_64
>        # nrcpus online : 4
>        # nrcpus avail : 4
>        # cpudesc : Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz
>        # cpuid : GenuineIntel,6,15,11
>        # total memory : 8092884 kB
>        ...
>        # HEADER_CPU_TOPOLOGY info available, use -I to display
>        noploop for 2 seconds
>        [ perf record: Woken up 1 times to write data ]
>        [ perf record: Captured and wrote 0.084 MB - (~3677 samples) ]
>          99.80%  noploop  noploop            [.] noploop
>           0.19%  noploop  [kernel.kallsyms]  [k] radix_tree_gang_lookup
>
> Signed-off-by: Stephane Eranian<eranian@google.com>

I love the feature and it works nicely, but there has been push back on 
adding more synthesized events.  Also the size of the patch is a bit 
much to take in. If there is no objection to the synthesized can you 
break the patch up -- e.g., introduce the events in one, synthesis and 
processing functions in another, plug into the commands, ...

David

Re: [PATCH v2 5/5] perf: make perf buildid-list work better with pipe mode

[David Ahern]

On 5/15/12 5:28 AM, Stephane Eranian wrote:
> In order for perf buildid-list to work with pipe-mode files,
> it needs to process buildids and event attr structs.
>
> $ perf record -o - noploop 2 | ./perf inject -b | perf buildid-list -i - -H
> noploop for 2 seconds
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.084 MB - (~3678 samples) ]
> 0000000000000000000000000000000000000000 [kernel.kallsyms]
> 3a0d0629efe74a8da3eeba372cdbd74ad9b8f5d5 /usr/local/bin/noploop
>
> The reason [kernel.kallsyms] shows a 0 build-id comes from the
> way buildids are injected in the stream. The buildid for the kernel
> is provided by a BUILD_ID record. The [kernel.kallsyms] is provided
> by a MMAP record. There is no clean and obvious way to link the two,
> unfortunately. In regular mode, the kernel buildid is generated from
> reading the ELF image or kallsyms and perf knows to associate
> [kernel.kallsyms] to it. Later on, when perf processes the
> [kernel.kallsyms] MMAP record, it will already have a dso for it.
>
> So for now, make sure perf buildid-list shows the buildids for
> everything but the kernel image.
>
> Signed-off-by: Stephane Eranian<eranian@google.com>
> ---
>   tools/perf/builtin-buildid-list.c |    6 +++++-
>   tools/perf/util/build-id.c        |    2 ++
>   2 files changed, 7 insertions(+), 1 deletions(-)

Reviewed-and-tested-by: David Ahern <dsahern@gmail.com>

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Stephane Eranian]

On Wed, May 16, 2012 at 5:34 AM, David Ahern <dsahern@gmail.com> wrote:
> On 5/15/12 5:28 AM, Stephane Eranian wrote:
>>
>> This patch adds the meta-data header support for perf record
>> when used in pipe mode: perf record -o -
>>
>> Up until now, meta-data was only available when perf record
>> was used in "regular" mode, i.e., generating a perf.data file.
>> For users depending on pipe mode, no host, event header information
>> was gathered. This patch addresses this limitation.
>>
>> The difficulty in pipe mode is that information needs to be written
>> sequentially to the pipe. Meta data headers are usually generated
>> (and also expected) at the beginning of the file (or piped output).
>> To solve this problem, we introduce new synthetic record types,
>> one for each meta-data type. The approach is similar to what
>> is already used for BUILD_ID and TRACING_DATA.
>>
>> We have modified util/header.c such that the same routines are used
>> to generate and read the meta-data information regardless of pipe-mode
>> vs. regular mode. To make this work, we added a new struct called
>> feat_fd which encapsulates all the information necessary to read or
>> write meta-data information to a file/pipe or from a file/pipe.
>>
>> It should be noted that there is a limitation with the current
>> perf in terms of endianess in pipe mode. Perf assumes the records
>> are generated using the same endianess during collection and
>> analysis.  That is always the case with the example shown below.
>> However, one could also do:
>> $ perf record -o - noploop 2 | perf inject -b>perf.data
>> $ cat perf.data | perf report -i -
>>
>> With this patch, it is possible to get:
>>  $ perf record -o - noploop 2 | perf inject -b | perf report -i -
>>       # ========
>>       # captured on: Fri Jan 20 18:13:55 2012
>>       # ========
>>       #
>>       # hostname : quad
>>       # os release : 3.2.0-rc7-tip
>>       # perf version : 3.2.0
>>       # arch : x86_64
>>       # nrcpus online : 4
>>       # nrcpus avail : 4
>>       # cpudesc : Intel(R) Core(TM)2 Quad CPU Q6600 @ 2.40GHz
>>       # cpuid : GenuineIntel,6,15,11
>>       # total memory : 8092884 kB
>>       ...
>>       # HEADER_CPU_TOPOLOGY info available, use -I to display
>>       noploop for 2 seconds
>>       [ perf record: Woken up 1 times to write data ]
>>       [ perf record: Captured and wrote 0.084 MB - (~3677 samples) ]
>>         99.80%  noploop  noploop            [.] noploop
>>          0.19%  noploop  [kernel.kallsyms]  [k] radix_tree_gang_lookup
>>
>> Signed-off-by: Stephane Eranian<eranian@google.com>
>
>
> I love the feature and it works nicely, but there has been push back on
> adding more synthesized events.  Also the size of the patch is a bit much to
> take in. If there is no objection to the synthesized can you break the patch
> up -- e.g., introduce the events in one, synthesis and processing functions
> in another, plug into the commands, ...
>
Without new synthesized events, you cannot make this work. This is by
construction
of the pipe mode. Meta-data has be be injected in the stream and
therefore it needs
PERF_RECORD_* types.

> David

Re: [PATCH v2 0/5] perf tools: add meta-data header support in pipe mode

[Arnaldo Carvalho de Melo]

Em Tue, May 15, 2012 at 08:38:05PM -0600, David Ahern escreveu:
> See tools/perf/util/header.{ch}
> Runtime example:
> 
> perf script -i /tmp/perf.data -I
> 
> # ========
> # captured on: Tue May 15 20:22:15 2012
> # ========
> #
> # hostname : nxos-vdc-dev3
> # os release : 3.4.0-rc7+
> # perf version : 3.4.rc4.137.g978da3.dirty
> # arch : x86_64
> # nrcpus online : 16
> # nrcpus avail : 16
> # cpudesc : Intel(R) Xeon(R) CPU E5540 @ 2.53GHz
> # cpuid : GenuineIntel,6,26,5
> # total memory : 24680324 kB
> # cmdline : /tmp/pbuild/perf record -fo /tmp/perf.data -- /tmp/noploop
> 
> ...
> 
> 'perf report --stdio' also shows it. It gets lost in the TUI.

Yeah, we need to add a splash screen with this info when the TUI starts
and also ad a hotkey to show it at anytime.

- Arnaldo

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[David Ahern]

On 5/16/12 1:41 AM, Stephane Eranian wrote:
> On Wed, May 16, 2012 at 5:34 AM, David Ahern<dsahern@gmail.com>  wrote:
>> I love the feature and it works nicely, but there has been push back on
>> adding more synthesized events.  Also the size of the patch is a bit much to
>> take in. If there is no objection to the synthesized can you break the patch
>> up -- e.g., introduce the events in one, synthesis and processing functions
>> in another, plug into the commands, ...
>>
> Without new synthesized events, you cannot make this work. This is by
> construction
> of the pipe mode. Meta-data has be be injected in the stream and
> therefore it needs
> PERF_RECORD_* types.

Understood. I don't object to synthesized events.

Arnaldo: preferences?

David

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Arnaldo Carvalho de Melo]

Em Fri, May 18, 2012 at 10:50:45AM -0600, David Ahern escreveu:
> On 5/16/12 1:41 AM, Stephane Eranian wrote:
> >On Wed, May 16, 2012 at 5:34 AM, David Ahern<dsahern@gmail.com>  wrote:
> >>I love the feature and it works nicely, but there has been push back on
> >>adding more synthesized events.  Also the size of the patch is a bit much to
> >>take in. If there is no objection to the synthesized can you break the patch
> >>up -- e.g., introduce the events in one, synthesis and processing functions
> >>in another, plug into the commands, ...
> >>
> >Without new synthesized events, you cannot make this work. This is by
> >construction
> >of the pipe mode. Meta-data has be be injected in the stream and
> >therefore it needs
> >PERF_RECORD_* types.
> 
> Understood. I don't object to synthesized events.
> 
> Arnaldo: preferences?

PeterZ was the one objecting to adding more userspace only events,
Peter?

- Arnaldo

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Peter Zijlstra]

On Fri, 2012-05-18 at 14:19 -0300, Arnaldo Carvalho de Melo wrote:

> PeterZ was the one objecting to adding more userspace only events,

Ah, yes I was ;-)

So uhm the argument was something like perf_event_type is a kernel enum
and userspace stealing space there is going to get us into trouble
eventually since userspace doesn't register its types in our enum.

Furthermore most (if not all) the userspace thingies were setup (like in
this case single session meta-data) things. So they don't belong in the
event stream at all.

>From what I can remember all this is somehow related to how data is
passed to scripts or so and since there's only a single stdin everything
is stuffed over it.

For the data file we should simply create another section in the header;
and I think that is how it works these days.

For the script muck, I've really no idea how all that works, but why
can't you wrap the stuff in another layer; have a header up front that
says the next N bytes are meta-data and after that there's the regular
data stream.

Or take some inspiration from one of the many multi-stream stream
formats out there like mpeg or ogg or whatever.

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Stephane Eranian]

On Tue, May 22, 2012 at 7:33 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, 2012-05-18 at 14:19 -0300, Arnaldo Carvalho de Melo wrote:
>
>> PeterZ was the one objecting to adding more userspace only events,
>
> Ah, yes I was ;-)
>
> So uhm the argument was something like perf_event_type is a kernel enum
> and userspace stealing space there is going to get us into trouble
> eventually since userspace doesn't register its types in our enum.
>
Yes, I did not start this but I understand why it was done that way.

The problem is that the headers as they are written to the file need
seeking in the file to update the offset table. That is NOT possible when
you operate in pipe mode. As such you need to inject the header infos
very much like kernel PERF_RECORD_*. That is also why you have
perf inject -b. Buildids are added at the end of the run in file mode,
and that's another seek to the offset table if I recall correctly.

So we need some namespace, so we can tag certain records as
"synthetic" vs. kernel-generated. The way it was done is to assume
that the kernel is not going to have more than 63 type of record types.
That's where we run the risk of overlap one day. But for now we are
at 10 kernel record types.

As such you need to inject certain header infos in the stream of kernel
> Furthermore most (if not all) the userspace thingies were setup (like in
> this case single session meta-data) things. So they don't belong in the
> event stream at all.
>
> From what I can remember all this is somehow related to how data is
> passed to scripts or so and since there's only a single stdin everything
> is stuffed over it.
>
> For the data file we should simply create another section in the header;
> and I think that is how it works these days.
>
> For the script muck, I've really no idea how all that works, but why
> can't you wrap the stuff in another layer; have a header up front that
> says the next N bytes are meta-data and after that there's the regular
> data stream.
>
> Or take some inspiration from one of the many multi-stream stream
> formats out there like mpeg or ogg or whatever.

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Namhyung Kim]

Hi,

On Tue, 22 May 2012 19:51:15 +0200, Stephane Eranian wrote:
> On Tue, May 22, 2012 at 7:33 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Fri, 2012-05-18 at 14:19 -0300, Arnaldo Carvalho de Melo wrote:
>>
>>> PeterZ was the one objecting to adding more userspace only events,
>>
>> Ah, yes I was ;-)
>>
>> So uhm the argument was something like perf_event_type is a kernel enum
>> and userspace stealing space there is going to get us into trouble
>> eventually since userspace doesn't register its types in our enum.
>>
> Yes, I did not start this but I understand why it was done that way.
>
> The problem is that the headers as they are written to the file need
> seeking in the file to update the offset table. That is NOT possible when
> you operate in pipe mode. As such you need to inject the header infos
> very much like kernel PERF_RECORD_*. That is also why you have
> perf inject -b. Buildids are added at the end of the run in file mode,
> and that's another seek to the offset table if I recall correctly.
>

IIUC, to add build-ids we should check all dso's whether hits or not,
but it's not same as the case of meta-data since we can know it before
processing. Am I missing something?

Thanks,
Namhyung

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Arnaldo Carvalho de Melo]

Em Wed, May 23, 2012 at 09:45:12AM +0900, Namhyung Kim escreveu:
> On Tue, 22 May 2012 19:51:15 +0200, Stephane Eranian wrote:
> > On Tue, May 22, 2012 at 7:33 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> >> On Fri, 2012-05-18 at 14:19 -0300, Arnaldo Carvalho de Melo wrote:
> >>
> >>> PeterZ was the one objecting to adding more userspace only events,
> >>
> >> Ah, yes I was ;-)
> >>
> >> So uhm the argument was something like perf_event_type is a kernel enum
> >> and userspace stealing space there is going to get us into trouble
> >> eventually since userspace doesn't register its types in our enum.
> >>
> > Yes, I did not start this but I understand why it was done that way.
> >
> > The problem is that the headers as they are written to the file need
> > seeking in the file to update the offset table. That is NOT possible when
> > you operate in pipe mode. As such you need to inject the header infos
> > very much like kernel PERF_RECORD_*. That is also why you have
> > perf inject -b. Buildids are added at the end of the run in file mode,
> > and that's another seek to the offset table if I recall correctly.
> >
> 
> IIUC, to add build-ids we should check all dso's whether hits or not,
> but it's not same as the case of meta-data since we can know it before
> processing. Am I missing something?

To add build ids properly we should read it when the kernel loads a DSO,
so that when PERF_RECORD_MMAP is sent to the event stream, the build id
is there, so no need for post processing, etc.

The way we do it now is racy, long running sessions may experience DSO
updating, etc.

I think we can figure out a way to avoid the seeks Stephane mentions so
to get something done as Peter suggests.

But at the same time I don't think reusing the existing mechanism is
such a big problem as Peter makes of it :-\

- Arnaldo

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Peter Zijlstra]

On Tue, 2012-05-22 at 22:01 -0300, Arnaldo Carvalho de Melo wrote:
> 
> 
> But at the same time I don't think reusing the existing mechanism is
> such a big problem as Peter makes of it :-\ 

Probably not, but just know that I'll not take any such abuse into
consideration when adding new record types.

That pipe mess is ill-designed and I don't care about it.

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Peter Zijlstra]

On Tue, 2012-05-22 at 19:51 +0200, Stephane Eranian wrote:
> The problem is that the headers as they are written to the file need
> seeking in the file to update the offset table. That is NOT possible when
> you operate in pipe mode. 

I know that, I said as much. But it is quite possible to multiplex
streams without the need for seeking, look at these MKV containers that
contain video, multiple-audio and multiple-subtitle tracks.

The fact is, whomever wrote that pipe mode took a bonghit before
touching the keyboard. They didn't even ask if we could reserve a range
for userspace events, let alone consider if it was a good idea to begin
with.

If we ever manage to get splice() working you can't keep that pipe mode
working anyway.

Pushing everything over a single stream isn't a scalable solution anyway
in the same way that writing everything into the one file isn't the best
thing and would ideally be replaced as well.

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Stephane Eranian]

On Wed, May 23, 2012 at 10:21 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, 2012-05-22 at 19:51 +0200, Stephane Eranian wrote:
>> The problem is that the headers as they are written to the file need
>> seeking in the file to update the offset table. That is NOT possible when
>> you operate in pipe mode.
>
> I know that, I said as much. But it is quite possible to multiplex
> streams without the need for seeking, look at these MKV containers that
> contain video, multiple-audio and multiple-subtitle tracks.
>
> The fact is, whomever wrote that pipe mode took a bonghit before
> touching the keyboard. They didn't even ask if we could reserve a range
> for userspace events, let alone consider if it was a good idea to begin
> with.
You're probably right, but the fact of the matter is, it's there now and
we have lots of perf.data piling up all over the place. So you need
to maintain compatibility with those. In my patch, I am just extending
what's already there. I was not trying to rewrite the whole pipe mode + meta
data logic (though I have done some of that already in the past).

Clearly pipe-mode was an after thought, but it is useful, I know people who
use it. Obviously I use it, otherwise I would not necessarily have spent the
time fixing the meta-data headers.

But I think, for the time being, my patch addresses an issue and does
not prevent us for discussing a new file format for perf record, I have nothing
against that.


>
> If we ever manage to get splice() working you can't keep that pipe mode
> working anyway.
>
> Pushing everything over a single stream isn't a scalable solution anyway
> in the same way that writing everything into the one file isn't the best
> thing and would ideally be replaced as well.

[tip:perf/core] perf tools: rename HEADER_TRACE_INFO to HEADER_TRACING_DATA

[tip-bot for Stephane Eranian]

Commit-ID:  2eeaaa095d155d47d47d5df07571105b8ae76ffd
Gitweb:     http://git.kernel.org/tip/2eeaaa095d155d47d47d5df07571105b8ae76ffd
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Tue, 15 May 2012 13:28:13 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 May 2012 12:57:46 -0300

perf tools: rename HEADER_TRACE_INFO to HEADER_TRACING_DATA

To match the PERF_RECORD_HEADER_TRACING_DATA record type.

This is the same info as the one used for pipe mode whereas the other
one is for regular file output. This will help in the later patch to add
meta-data infos in pipe mode.

Signed-off-by: Stephane Eranian <eranian@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1337081295-10303-4-git-send-email-eranian@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |    4 ++--
 tools/perf/util/header.c    |   10 +++++-----
 tools/perf/util/header.h    |    2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 8a3dfac..9c76add 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -396,7 +396,7 @@ static void perf_record__mmap_read_all(struct perf_record *rec)
 			perf_record__mmap_read(rec, &rec->evlist->mmap[i]);
 	}
 
-	if (perf_header__has_feat(&rec->session->header, HEADER_TRACE_INFO))
+	if (perf_header__has_feat(&rec->session->header, HEADER_TRACING_DATA))
 		write_output(rec, &finished_round_event, sizeof(finished_round_event));
 }
 
@@ -478,7 +478,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		perf_header__clear_feat(&session->header, HEADER_BUILD_ID);
 
 	if (!have_tracepoints(&evsel_list->entries))
-		perf_header__clear_feat(&session->header, HEADER_TRACE_INFO);
+		perf_header__clear_feat(&session->header, HEADER_TRACING_DATA);
 
 	if (!rec->opts.branch_stack)
 		perf_header__clear_feat(&session->header, HEADER_BRANCH_STACK);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 5385980..2dd5edf 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -437,7 +437,7 @@ static bool perf_session__read_build_ids(struct perf_session *session, bool with
 	return ret;
 }
 
-static int write_trace_info(int fd, struct perf_header *h __used,
+static int write_tracing_data(int fd, struct perf_header *h __used,
 			    struct perf_evlist *evlist)
 {
 	return read_tracing_data(fd, &evlist->entries);
@@ -1472,7 +1472,7 @@ out:
 	return err;
 }
 
-static int process_trace_info(struct perf_file_section *section __unused,
+static int process_tracing_data(struct perf_file_section *section __unused,
 			      struct perf_header *ph __unused,
 			      int feat __unused, int fd)
 {
@@ -1508,11 +1508,11 @@ struct feature_ops {
 		.full_only = true }
 
 /* feature_ops not implemented: */
-#define print_trace_info		NULL
-#define print_build_id			NULL
+#define print_tracing_data	NULL
+#define print_build_id		NULL
 
 static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
-	FEAT_OPP(HEADER_TRACE_INFO,	trace_info),
+	FEAT_OPP(HEADER_TRACING_DATA,	tracing_data),
 	FEAT_OPP(HEADER_BUILD_ID,	build_id),
 	FEAT_OPA(HEADER_HOSTNAME,	hostname),
 	FEAT_OPA(HEADER_OSRELEASE,	osrelease),
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 21a6be0..2d42b3e 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -12,7 +12,7 @@
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
 	HEADER_FIRST_FEATURE	= 1,
-	HEADER_TRACE_INFO	= 1,
+	HEADER_TRACING_DATA	= 1,
 	HEADER_BUILD_ID,
 
 	HEADER_HOSTNAME,

[tip:perf/core] perf inject: Fix broken perf inject -b

[tip-bot for Stephane Eranian]

Commit-ID:  1a1ed1ba6778a5bc5702cebe276ab080a0b78115
Gitweb:     http://git.kernel.org/tip/1a1ed1ba6778a5bc5702cebe276ab080a0b78115
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Tue, 15 May 2012 13:28:11 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 May 2012 12:59:28 -0300

perf inject: Fix broken perf inject -b

perf inject -b was broken. It would not inject any build_id into the
stream. Furthermore, it would strip samples from the stream.

The reason was a missing initialization of the event attribute
structure. The perf_tool.tool.attr() callback was pointing to a simple
repipe. But there was no initialization of the internal data structures
to keep track of events and event ids. That later caused event id
lookups to fail, and sample would get removed.

The patch simply adds back the call to perf_event__process_attr() to
initialize the evlist structure and now build_ids are again injected.

Signed-off-by: Stephane Eranian <eranian@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1337081295-10303-2-git-send-email-eranian@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-inject.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 09c1061..3beab48 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -60,6 +60,11 @@ static int perf_event__repipe_tracing_data_synth(union perf_event *event,
 static int perf_event__repipe_attr(union perf_event *event,
 				   struct perf_evlist **pevlist __used)
 {
+	int ret;
+	ret = perf_event__process_attr(event, pevlist);
+	if (ret)
+		return ret;
+
 	return perf_event__repipe_synth(NULL, event, NULL);
 }
 

[tip:perf/core] perf tools: Fix piped mode read code

[tip-bot for Stephane Eranian]

Commit-ID:  444d28663936e286c752f05feca44d6041b1fce4
Gitweb:     http://git.kernel.org/tip/444d28663936e286c752f05feca44d6041b1fce4
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Tue, 15 May 2012 13:28:12 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 May 2012 12:59:52 -0300

perf tools: Fix piped mode read code

In __perf_session__process_pipe_events(), there was a risk we would read
more than what a union perf_event struct can hold. this could happen in
case, perf is reading a file which contains new record types it does not
know about and which are larger than anything it knows about.

In general, perf is supposed to skip records it does not understand, but
in pipe mode, those have to be read and ignored.  The fixed size header
contains the size of the record, but that size may be larger than union
perf_event, yet it was used as the backing to the read in:

  union perf_event event;
  void *p;

  size = event->header.size;

  p = &event;
  p += sizeof(struct perf_event_header);
  if (size - sizeof(struct perf_event_header)) {
    err = readn(self->fd, p, size - sizeof(struct perf_event_header));

We fix this by allocating a buffer based on the size reported in the
header. We reuse the buffer as much as we can. We realloc in case it
becomes too small. In the  common case, the performance impact is
negligible.

Signed-off-by: Stephane Eranian <eranian@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1337081295-10303-3-git-send-email-eranian@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   34 ++++++++++++++++++++++++++--------
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 17c9ace..93d355d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1098,8 +1098,9 @@ volatile int session_done;
 static int __perf_session__process_pipe_events(struct perf_session *self,
 					       struct perf_tool *tool)
 {
-	union perf_event event;
-	uint32_t size;
+	union perf_event *event;
+	uint32_t size, cur_size = 0;
+	void *buf = NULL;
 	int skip = 0;
 	u64 head;
 	int err;
@@ -1108,8 +1109,14 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
 	perf_tool__fill_defaults(tool);
 
 	head = 0;
+	cur_size = sizeof(union perf_event);
+
+	buf = malloc(cur_size);
+	if (!buf)
+		return -errno;
 more:
-	err = readn(self->fd, &event, sizeof(struct perf_event_header));
+	event = buf;
+	err = readn(self->fd, event, sizeof(struct perf_event_header));
 	if (err <= 0) {
 		if (err == 0)
 			goto done;
@@ -1119,13 +1126,23 @@ more:
 	}
 
 	if (self->header.needs_swap)
-		perf_event_header__bswap(&event.header);
+		perf_event_header__bswap(&event->header);
 
-	size = event.header.size;
+	size = event->header.size;
 	if (size == 0)
 		size = 8;
 
-	p = &event;
+	if (size > cur_size) {
+		void *new = realloc(buf, size);
+		if (!new) {
+			pr_err("failed to allocate memory to read event\n");
+			goto out_err;
+		}
+		buf = new;
+		cur_size = size;
+		event = buf;
+	}
+	p = event;
 	p += sizeof(struct perf_event_header);
 
 	if (size - sizeof(struct perf_event_header)) {
@@ -1141,9 +1158,9 @@ more:
 		}
 	}
 
-	if ((skip = perf_session__process_event(self, &event, tool, head)) < 0) {
+	if ((skip = perf_session__process_event(self, event, tool, head)) < 0) {
 		pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
-		       head, event.header.size, event.header.type);
+		       head, event->header.size, event->header.type);
 		err = -EINVAL;
 		goto out_err;
 	}
@@ -1158,6 +1175,7 @@ more:
 done:
 	err = 0;
 out_err:
+	free(buf);
 	perf_session__warn_about_errors(self, tool);
 	perf_session_free_sample_buffers(self);
 	return err;

[tip:perf/core] perf buildid-list: Work better with pipe mode

[tip-bot for Stephane Eranian]

Commit-ID:  299c345208ae15fbc1e1c96dc1b6ac61e4e5da10
Gitweb:     http://git.kernel.org/tip/299c345208ae15fbc1e1c96dc1b6ac61e4e5da10
Author:     Stephane Eranian <eranian@google.com>
AuthorDate: Tue, 15 May 2012 13:28:15 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 22 May 2012 13:03:54 -0300

perf buildid-list: Work better with pipe mode

In order for perf buildid-list to work with pipe-mode files, it needs to
process buildids and event attr structs.

$ perf record -o - noploop 2 | ./perf inject -b | perf buildid-list -i - -H
noploop for 2 seconds
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.084 MB - (~3678 samples) ]
0000000000000000000000000000000000000000 [kernel.kallsyms]
3a0d0629efe74a8da3eeba372cdbd74ad9b8f5d5 /usr/local/bin/noploop

The reason [kernel.kallsyms] shows a 0 build-id comes from the
way buildids are injected in the stream.

The buildid for the kernel is provided by a BUILD_ID record. The
[kernel.kallsyms] is provided by a MMAP record. There is no clean and
obvious way to link the two, unfortunately.

In regular mode, the kernel buildid is generated from reading the ELF
image or kallsyms and perf knows to associate [kernel.kallsyms] to it.
Later on, when perf processes the [kernel.kallsyms] MMAP record, it will
already have a dso for it.

So for now, make sure perf buildid-list shows the buildids for
everything but the kernel image.

Signed-off-by: Stephane Eranian <eranian@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1337081295-10303-6-git-send-email-eranian@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-buildid-list.c |    6 +++++-
 tools/perf/util/build-id.c        |    2 ++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 5248046..6b2bcfb 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -84,7 +84,11 @@ static int perf_session__list_build_ids(void)
 	if (filename__fprintf_build_id(session->filename, stdout))
 		goto out;
 
-	if (with_hits)
+	/*
+	 * in pipe-mode, the only way to get the buildids is to parse
+	 * the record stream. Buildids are stored as RECORD_HEADER_BUILD_ID
+	 */
+	if (with_hits || session->fd_pipe)
 		perf_session__process_events(session, &build_id__mark_dso_hit_ops);
 
 	perf_session__fprintf_dsos_buildid(session, stdout, with_hits);
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index dff9c7a..fd9a594 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -65,6 +65,8 @@ struct perf_tool build_id__mark_dso_hit_ops = {
 	.mmap	= perf_event__process_mmap,
 	.fork	= perf_event__process_task,
 	.exit	= perf_event__exit_del_thread,
+	.attr		 = perf_event__process_attr,
+	.build_id	 = perf_event__process_build_id,
 };
 
 char *dso__build_id_filename(struct dso *self, char *bf, size_t size)

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[David Ahern]

On 5/22/12 11:51 AM, Stephane Eranian wrote:
> The problem is that the headers as they are written to the file need
> seeking in the file to update the offset table. That is NOT possible when
> you operate in pipe mode. As such you need to inject the header infos
> very much like kernel PERF_RECORD_*. That is also why you have
> perf inject -b. Buildids are added at the end of the run in file mode,
> and that's another seek to the offset table if I recall correctly.

Perhaps I am being too simple minded here, but why not dump the features 
to the pipe as a series of structs when the perf session is created?

struct pipe_data {
	u32 length;
	u32 type;
	char data[0];		
}

It would require the features to be written to a buffer first to get the 
length, but that's manageable without too much code change. Endianness 
would need to be handled -- maybe a u8 flags at the beginning.

David

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Arnaldo Carvalho de Melo]

Em Thu, May 24, 2012 at 09:36:35AM -0600, David Ahern escreveu:
> On 5/22/12 11:51 AM, Stephane Eranian wrote:
> >The problem is that the headers as they are written to the file need
> >seeking in the file to update the offset table. That is NOT possible when
> >you operate in pipe mode. As such you need to inject the header infos
> >very much like kernel PERF_RECORD_*. That is also why you have
> >perf inject -b. Buildids are added at the end of the run in file mode,
> >and that's another seek to the offset table if I recall correctly.

> Perhaps I am being too simple minded here, but why not dump the
> features to the pipe as a series of structs when the perf session is
> created?

> struct pipe_data {
> 	u32 length;
> 	u32 type;
> 	char data[0];		
> }

> It would require the features to be written to a buffer first to get
> the length, but that's manageable without too much code change.
> Endianness would need to be handled -- maybe a u8 flags at the
> beginning.

I guess this comes down to somebody prototyping it. The way its being
done is just the easiest one, and one that at least Peter dislikes a lot
;-)

- Arnaldo

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[David Ahern]

On 5/24/12 10:19 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, May 24, 2012 at 09:36:35AM -0600, David Ahern escreveu:
>> On 5/22/12 11:51 AM, Stephane Eranian wrote:
>>> The problem is that the headers as they are written to the file need
>>> seeking in the file to update the offset table. That is NOT possible when
>>> you operate in pipe mode. As such you need to inject the header infos
>>> very much like kernel PERF_RECORD_*. That is also why you have
>>> perf inject -b. Buildids are added at the end of the run in file mode,
>>> and that's another seek to the offset table if I recall correctly.
>
>> Perhaps I am being too simple minded here, but why not dump the
>> features to the pipe as a series of structs when the perf session is
>> created?
>
>> struct pipe_data {
>> 	u32 length;
>> 	u32 type;
>> 	char data[0];		
>> }
>
>> It would require the features to be written to a buffer first to get
>> the length, but that's manageable without too much code change.
>> Endianness would need to be handled -- maybe a u8 flags at the
>> beginning.
>
> I guess this comes down to somebody prototyping it. The way its being
> done is just the easiest one, and one that at least Peter dislikes a lot
> ;-)
>
> - Arnaldo

I don't know about that 'easy' comment; that patch is fairly long. Hence 
my question about whether synthesized events are acceptable before 
spending time on it. ;-)

David

Re: [PATCH v2 4/5] perf record: add meta-data support for pipe-mode

[Arnaldo Carvalho de Melo]

Em Thu, May 24, 2012 at 10:22:04AM -0600, David Ahern escreveu:
> On 5/24/12 10:19 AM, Arnaldo Carvalho de Melo wrote:
> >Em Thu, May 24, 2012 at 09:36:35AM -0600, David Ahern escreveu:
> >>On 5/22/12 11:51 AM, Stephane Eranian wrote:
> >>>The problem is that the headers as they are written to the file need
> >>>seeking in the file to update the offset table. That is NOT possible when
> >>>you operate in pipe mode. As such you need to inject the header infos
> >>>very much like kernel PERF_RECORD_*. That is also why you have
> >>>perf inject -b. Buildids are added at the end of the run in file mode,
> >>>and that's another seek to the offset table if I recall correctly.
> >
> >>Perhaps I am being too simple minded here, but why not dump the
> >>features to the pipe as a series of structs when the perf session is
> >>created?
> >
> >>struct pipe_data {
> >>	u32 length;
> >>	u32 type;
> >>	char data[0];		
> >>}
> >
> >>It would require the features to be written to a buffer first to get
> >>the length, but that's manageable without too much code change.
> >>Endianness would need to be handled -- maybe a u8 flags at the
> >>beginning.
> >
> >I guess this comes down to somebody prototyping it. The way its being
> >done is just the easiest one, and one that at least Peter dislikes a lot
> >;-)
> 
> I don't know about that 'easy' comment; that patch is fairly long.

Easy in the sense that we didn't need to come up with any new mechanism
to insert that meta data.

> Hence my question about whether synthesized events are acceptable
> before spending time on it. ;-)

I think that we need to follow Peter suggestion, which looks to me
similar to what you're suggesting, i.e. before the event stream, that
will become multichannel anyway.

- Arnaldo