[PATCH 0/7] perf tool: add meta-data header support for pipe-mode

[PATCH 0/7] perf tool: add meta-data header support for pipe-mode

[David Carrillo-Cisneros]

(This is a rebased and updated version of Stephane Eranian's version
 in https://patchwork.kernel.org/patch/1499081/)
 
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, neither host or event header
information were 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.

With this patch, it is possible to get:
  $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  # hostname : myhost
  # os release : 4.11.0-dbx-up_perf
  # perf version : 4.11.rc6.g6277c80
  # arch : x86_64
  # nrcpus online : 72
  # nrcpus avail : 72
  # cpudesc : Intel(R) Xeon(R) CPU E5-2696 v3 @ 2.30GHz
  # cpuid : GenuineIntel,6,63,2
  # total memory : 263457192 kB
  # cmdline : /root/perf record -o - -e cycles -c 100000 sleep 1
  # HEADER_CPU_TOPOLOGY info available, use -I to display
  # HEADER_NUMA_TOPOLOGY info available, use -I to display
  # pmu mappings: intel_bts = 6, uncore_imc_4 = 22, uncore_sbox_1 = 47, uncore_cbox_5 = 33, uncore_ha_0 = 16, uncore_cbox
  Percent |      Source code & Disassembly of kcore for cycles (9 samples)
  ...


David Carrillo-Cisneros (7):
  perf header: fail on write_padded error
  perf util: add const modifier to buf in "writen" function
  perf header: use struct feat_fd for write
  perf header: use struct feat_fd for print
  perf header: use struct feat_fd for process and read
  perf tool: make show-info in perf report a tool attribute
  perf tools: add feature header record to pipe-mode

 tools/perf/builtin-annotate.c |   1 +
 tools/perf/builtin-inject.c   |   1 +
 tools/perf/builtin-record.c   |   6 +
 tools/perf/builtin-report.c   |   6 +-
 tools/perf/builtin-script.c   |   6 +-
 tools/perf/util/build-id.c    |  10 +-
 tools/perf/util/build-id.h    |   4 +-
 tools/perf/util/event.c       |  13 +
 tools/perf/util/event.h       |  19 +
 tools/perf/util/header.c      | 905 +++++++++++++++++++++++-------------------
 tools/perf/util/header.h      |  17 +-
 tools/perf/util/session.c     |  12 +
 tools/perf/util/tool.h        |   4 +-
 tools/perf/util/util.c        |   6 +-
 tools/perf/util/util.h        |   2 +-
 15 files changed, 594 insertions(+), 418 deletions(-)

-- 
2.13.0.303.g4ebf302169-goog

[PATCH 1/7] perf header: fail on write_padded error

[David Carrillo-Cisneros]

Do not proceed if write_padded error failed.

Also, add comments to remind that return value of write_*
functions in util/header.c do not return number of bytes written.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/header.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 948b2c5efb65..2415d41282d8 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -72,6 +72,7 @@ bool perf_header__has_feat(const struct perf_header *header, int feat)
 	return test_bit(feat, header->adds_features);
 }
 
+/* Return: 0 if succeded, -ERR if failed. */
 static int do_write(int fd, const void *buf, size_t size)
 {
 	while (size) {
@@ -87,6 +88,7 @@ static int do_write(int fd, const void *buf, size_t size)
 	return 0;
 }
 
+/* Return: 0 if succeded, -ERR if failed. */
 int write_padded(int fd, const void *bf, size_t count, size_t count_aligned)
 {
 	static const char zero_buf[NAME_ALIGN];
@@ -101,6 +103,7 @@ int write_padded(int fd, const void *bf, size_t count, size_t count_aligned)
 #define string_size(str)						\
 	(PERF_ALIGN((strlen(str) + 1), NAME_ALIGN) + sizeof(u32))
 
+/* Return: 0 if succeded, -ERR if failed. */
 static int do_write_string(int fd, const char *str)
 {
 	u32 len, olen;
@@ -3277,7 +3280,8 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 	 */
 	tracing_data_put(tdata);
 
-	write_padded(fd, NULL, 0, padding);
+	if (write_padded(fd, NULL, 0, padding))
+		return -1;
 
 	return aligned_size;
 }
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 2/7] perf util: add const modifier to buf in "writen" function

[David Carrillo-Cisneros]

Make buf in helper function "writen" constant to simplify
the life of its callers.

This requires to hack a cast of buf prior to passing it to "ion"
which is simpler than the alternative of reworking the "ion"
function to provide a read and a write paths, the latter with
constant buf argument.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/util.c | 6 ++++--
 tools/perf/util/util.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 6450c75a6f5b..31ca7661e5fb 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -260,6 +260,7 @@ static ssize_t ion(bool is_read, int fd, void *buf, size_t n)
 	size_t left = n;
 
 	while (left) {
+		/* buf must be treated as const if !is_read. */
 		ssize_t ret = is_read ? read(fd, buf, left) :
 					write(fd, buf, left);
 
@@ -287,9 +288,10 @@ ssize_t readn(int fd, void *buf, size_t n)
 /*
  * Write exactly 'n' bytes or return an error.
  */
-ssize_t writen(int fd, void *buf, size_t n)
+ssize_t writen(int fd, const void *buf, size_t n)
 {
-	return ion(false, fd, buf, n);
+	/* ion does not modify buf. */
+	return ion(false, fd, (void *)buf, n);
 }
 
 size_t hex_width(u64 v)
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 3852b6d3270a..e0881bf67fc9 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -57,7 +57,7 @@ int copyfile_mode(const char *from, const char *to, mode_t mode);
 int copyfile_offset(int fromfd, loff_t from_ofs, int tofd, loff_t to_ofs, u64 size);
 
 ssize_t readn(int fd, void *buf, size_t n);
-ssize_t writen(int fd, void *buf, size_t n);
+ssize_t writen(int fd, const void *buf, size_t n);
 
 struct perf_event_attr;
 
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 3/7] perf header: use struct feat_fd for write

[David Carrillo-Cisneros]

As preparation for using header records in pipe mode, replace
int fd with struct feat_fd fd in write functions for all header
record types.

Record types that are not used in pipe-mode (such as auxtrace) will
never be called with fd->buf set and fail if so.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/build-id.c |  10 ++-
 tools/perf/util/build-id.h |   4 +-
 tools/perf/util/header.c   | 194 +++++++++++++++++++++++++++++----------------
 tools/perf/util/header.h   |   7 +-
 4 files changed, 140 insertions(+), 75 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 168cc49654e7..292e90db3924 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -330,7 +330,7 @@ bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size)
 		else
 
 static int write_buildid(const char *name, size_t name_len, u8 *build_id,
-			 pid_t pid, u16 misc, int fd)
+			 pid_t pid, u16 misc, struct feat_fd *fd)
 {
 	int err;
 	struct build_id_event b;
@@ -345,14 +345,15 @@ static int write_buildid(const char *name, size_t name_len, u8 *build_id,
 	b.header.misc = misc;
 	b.header.size = sizeof(b) + len;
 
-	err = writen(fd, &b, sizeof(b));
+	err = do_write(fd, &b, sizeof(b));
 	if (err < 0)
 		return err;
 
 	return write_padded(fd, name, name_len + 1, len);
 }
 
-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 = 0;
 	char nm[PATH_MAX];
@@ -397,7 +398,8 @@ static int machine__write_buildid_table(struct machine *machine, int fd)
 	return err;
 }
 
-int perf_session__write_buildid_table(struct perf_session *session, int fd)
+int perf_session__write_buildid_table(struct perf_session *session,
+				      struct feat_fd *fd)
 {
 	struct rb_node *nd;
 	int err = machine__write_buildid_table(&session->machines.host, fd);
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index a96081121179..84e5e8a52970 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -9,6 +9,7 @@
 
 extern struct perf_tool build_id__mark_dso_hit_ops;
 struct dso;
+struct feat_fd;
 
 int build_id__sprintf(const u8 *build_id, int len, char *bf);
 int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id);
@@ -26,7 +27,8 @@ int build_id__mark_dso_hit(struct perf_tool *tool, union perf_event *event,
 int dsos__hit_all(struct perf_session *session);
 
 bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
-int perf_session__write_buildid_table(struct perf_session *session, int fd);
+int perf_session__write_buildid_table(struct perf_session *session,
+				      struct feat_fd *fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
 char *build_id_cache__origname(const char *sbuild_id);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2415d41282d8..5ca603b7a7a3 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -57,6 +57,14 @@ struct perf_file_attr {
 	struct perf_file_section	ids;
 };
 
+struct feat_fd {
+	struct perf_header *ph;
+	int fd;
+	void *buf;	/* Either buf != NULL or fd >= 0 */
+	ssize_t offset;
+	size_t size;
+};
+
 void perf_header__set_feat(struct perf_header *header, int feat)
 {
 	set_bit(feat, header->adds_features);
@@ -73,23 +81,36 @@ bool perf_header__has_feat(const struct perf_header *header, int feat)
 }
 
 /* Return: 0 if succeded, -ERR if failed. */
-static int do_write(int fd, const void *buf, size_t size)
+int do_write(struct feat_fd *fd, const void *buf, size_t size)
 {
-	while (size) {
-		int ret = write(fd, buf, size);
+	void *addr;
 
-		if (ret < 0)
-			return -errno;
+	if (!fd->buf) {
+		ssize_t ret = writen(fd->fd, buf, size);
 
-		size -= ret;
-		buf += ret;
+		if (ret != (ssize_t)size)
+			return ret < 0 ? (int)ret : -1;
+		return 0;
 	}
+retry:
+	if (size > (fd->size - fd->offset)) {
+		addr = realloc(fd->buf, fd->size << 1);
+		if (!addr)
+			return -ENOSPC;
+		fd->buf = addr;
+		fd->size <<= 1;
+		goto retry;
+	}
+
+	memcpy(fd->buf + fd->offset, buf, size);
+	fd->offset += size;
 
 	return 0;
 }
 
 /* Return: 0 if succeded, -ERR if failed. */
-int write_padded(int fd, const void *bf, size_t count, size_t count_aligned)
+int write_padded(struct feat_fd *fd, const void *bf,
+		 size_t count, size_t count_aligned)
 {
 	static const char zero_buf[NAME_ALIGN];
 	int err = do_write(fd, bf, count);
@@ -104,7 +125,7 @@ int write_padded(int fd, const void *bf, size_t count, size_t count_aligned)
 	(PERF_ALIGN((strlen(str) + 1), NAME_ALIGN) + sizeof(u32))
 
 /* Return: 0 if succeded, -ERR if failed. */
-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;
@@ -151,24 +172,31 @@ static char *do_read_string(int fd, struct perf_header *ph)
 	return NULL;
 }
 
-static int write_tracing_data(int fd, struct perf_header *h __maybe_unused,
-			    struct perf_evlist *evlist)
+static int write_tracing_data(struct feat_fd *fd,
+			      struct perf_evlist *evlist)
 {
-	return read_tracing_data(fd, &evlist->entries);
+	if (fd->buf) {
+		pr_err("Unsupported write_tracing_data to memory buffer.\n");
+		return -1;
+	}
+	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 __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(h, struct perf_session, header);
+	session = container_of(fd->ph, struct perf_session, header);
 
 	if (!perf_session__read_build_ids(session, true))
 		return -1;
 
+	if (fd->buf) {
+		pr_err("Unsupported write_build_id to memory buffer.\n");
+		return -1;
+	}
 	err = perf_session__write_buildid_table(session, fd);
 	if (err < 0) {
 		pr_debug("failed to write buildid table\n");
@@ -179,7 +207,7 @@ static int write_build_id(int fd, struct perf_header *h,
 	return 0;
 }
 
-static int write_hostname(int fd, struct perf_header *h __maybe_unused,
+static int write_hostname(struct feat_fd *fd,
 			  struct perf_evlist *evlist __maybe_unused)
 {
 	struct utsname uts;
@@ -192,7 +220,7 @@ static int write_hostname(int fd, struct perf_header *h __maybe_unused,
 	return do_write_string(fd, uts.nodename);
 }
 
-static int write_osrelease(int fd, struct perf_header *h __maybe_unused,
+static int write_osrelease(struct feat_fd *fd,
 			   struct perf_evlist *evlist __maybe_unused)
 {
 	struct utsname uts;
@@ -205,7 +233,7 @@ static int write_osrelease(int fd, struct perf_header *h __maybe_unused,
 	return do_write_string(fd, uts.release);
 }
 
-static int write_arch(int fd, struct perf_header *h __maybe_unused,
+static int write_arch(struct feat_fd *fd,
 		      struct perf_evlist *evlist __maybe_unused)
 {
 	struct utsname uts;
@@ -218,13 +246,13 @@ static int write_arch(int fd, struct perf_header *h __maybe_unused,
 	return do_write_string(fd, uts.machine);
 }
 
-static int write_version(int fd, struct perf_header *h __maybe_unused,
+static int write_version(struct feat_fd *fd,
 			 struct perf_evlist *evlist __maybe_unused)
 {
 	return do_write_string(fd, perf_version_string);
 }
 
-static int __write_cpudesc(int fd, const char *cpuinfo_proc)
+static int __write_cpudesc(struct feat_fd *fd, const char *cpuinfo_proc)
 {
 	FILE *file;
 	char *buf = NULL;
@@ -281,7 +309,7 @@ static int __write_cpudesc(int fd, const char *cpuinfo_proc)
 	return ret;
 }
 
-static int write_cpudesc(int fd, struct perf_header *h __maybe_unused,
+static int write_cpudesc(struct feat_fd *fd,
 		       struct perf_evlist *evlist __maybe_unused)
 {
 #ifndef CPUINFO_PROC
@@ -300,7 +328,7 @@ static int write_cpudesc(int fd, struct perf_header *h __maybe_unused,
 }
 
 
-static int write_nrcpus(int fd, struct perf_header *h __maybe_unused,
+static int write_nrcpus(struct feat_fd *fd,
 			struct perf_evlist *evlist __maybe_unused)
 {
 	long nr;
@@ -322,7 +350,7 @@ static int write_nrcpus(int fd, struct perf_header *h __maybe_unused,
 	return do_write(fd, &nra, sizeof(nra));
 }
 
-static int write_event_desc(int fd, struct perf_header *h __maybe_unused,
+static int write_event_desc(struct feat_fd *fd,
 			    struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel;
@@ -378,7 +406,7 @@ static int write_event_desc(int fd, struct perf_header *h __maybe_unused,
 	return 0;
 }
 
-static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
+static int write_cmdline(struct feat_fd *fd,
 			 struct perf_evlist *evlist __maybe_unused)
 {
 	char buf[MAXPATHLEN];
@@ -558,8 +586,8 @@ static struct cpu_topo *build_cpu_topology(void)
 	return tp;
 }
 
-static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
-			  struct perf_evlist *evlist __maybe_unused)
+static int write_cpu_topology(struct feat_fd *fd,
+			      struct perf_evlist *evlist __maybe_unused)
 {
 	struct cpu_topo *tp;
 	u32 i;
@@ -609,8 +637,8 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 
 
 
-static int write_total_mem(int fd, struct perf_header *h __maybe_unused,
-			  struct perf_evlist *evlist __maybe_unused)
+static int write_total_mem(struct feat_fd *fd,
+			   struct perf_evlist *evlist __maybe_unused)
 {
 	char *buf = NULL;
 	FILE *fp;
@@ -638,7 +666,7 @@ static int write_total_mem(int fd, struct perf_header *h __maybe_unused,
 	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];
@@ -698,8 +726,8 @@ static int write_topo_node(int fd, int node)
 	return ret;
 }
 
-static int write_numa_topology(int fd, struct perf_header *h __maybe_unused,
-			  struct perf_evlist *evlist __maybe_unused)
+static int write_numa_topology(struct feat_fd *fd,
+			       struct perf_evlist *evlist __maybe_unused)
 {
 	char *buf = NULL;
 	size_t len = 0;
@@ -759,15 +787,23 @@ static int write_numa_topology(int fd, struct perf_header *h __maybe_unused,
  * };
  */
 
-static int write_pmu_mappings(int fd, struct perf_header *h __maybe_unused,
+static int write_pmu_mappings(struct feat_fd *fd,
 			      struct perf_evlist *evlist __maybe_unused)
 {
 	struct perf_pmu *pmu = NULL;
-	off_t offset = lseek(fd, 0, SEEK_CUR);
-	__u32 pmu_num = 0;
+	u32 pmu_num = 0;
 	int ret;
 
-	/* write real pmu_num later */
+	/*
+	 * Do a first pass to count number of pmu to avoid lseek so this
+	 * works in pipe mode as well.
+	 */
+	while ((pmu = perf_pmu__scan(pmu))) {
+		if (!pmu->name)
+			continue;
+		pmu_num++;
+	}
+
 	ret = do_write(fd, &pmu_num, sizeof(pmu_num));
 	if (ret < 0)
 		return ret;
@@ -775,7 +811,6 @@ static int write_pmu_mappings(int fd, struct perf_header *h __maybe_unused,
 	while ((pmu = perf_pmu__scan(pmu))) {
 		if (!pmu->name)
 			continue;
-		pmu_num++;
 
 		ret = do_write(fd, &pmu->type, sizeof(pmu->type));
 		if (ret < 0)
@@ -786,12 +821,6 @@ static int write_pmu_mappings(int fd, struct perf_header *h __maybe_unused,
 			return ret;
 	}
 
-	if (pwrite(fd, &pmu_num, sizeof(pmu_num), offset) != sizeof(pmu_num)) {
-		/* discard all */
-		lseek(fd, offset, SEEK_SET);
-		return -1;
-	}
-
 	return 0;
 }
 
@@ -807,7 +836,7 @@ static int write_pmu_mappings(int fd, struct perf_header *h __maybe_unused,
  *	}[nr_groups];
  * };
  */
-static int write_group_desc(int fd, struct perf_header *h __maybe_unused,
+static int write_group_desc(struct feat_fd *fd,
 			    struct perf_evlist *evlist)
 {
 	u32 nr_groups = evlist->nr_groups;
@@ -850,7 +879,7 @@ int __weak get_cpuid(char *buffer __maybe_unused, size_t sz __maybe_unused)
 	return -1;
 }
 
-static int write_cpuid(int fd, struct perf_header *h __maybe_unused,
+static int write_cpuid(struct feat_fd *fd,
 		       struct perf_evlist *evlist __maybe_unused)
 {
 	char buffer[64];
@@ -865,22 +894,25 @@ static int write_cpuid(int fd, struct perf_header *h __maybe_unused,
 	return do_write_string(fd, buffer);
 }
 
-static int write_branch_stack(int fd __maybe_unused,
-			      struct perf_header *h __maybe_unused,
-		       struct perf_evlist *evlist __maybe_unused)
+static int write_branch_stack(struct feat_fd *fd __maybe_unused,
+			      struct perf_evlist *evlist __maybe_unused)
 {
 	return 0;
 }
 
-static int write_auxtrace(int fd, struct perf_header *h,
+static int write_auxtrace(struct feat_fd *fd,
 			  struct perf_evlist *evlist __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(h, struct perf_session, header);
+	if (fd->buf) {
+		pr_err("Unsupported write_auxtrace to memory buffer.\n");
+		return -1;
+	}
+	session = container_of(fd->ph, struct perf_session, header);
 
-	err = auxtrace_index__write(fd, &session->auxtrace_index);
+	err = auxtrace_index__write(fd->fd, &session->auxtrace_index);
 	if (err < 0)
 		pr_err("Failed to write auxtrace index\n");
 	return err;
@@ -1027,8 +1059,8 @@ static int build_caches(struct cpu_cache_level caches[], u32 size, u32 *cntp)
 
 #define MAX_CACHES 2000
 
-static int write_cache(int fd, struct perf_header *h __maybe_unused,
-			  struct perf_evlist *evlist __maybe_unused)
+static int write_cache(struct feat_fd *fd,
+		       struct perf_evlist *evlist __maybe_unused)
 {
 	struct cpu_cache_level caches[MAX_CACHES];
 	u32 cnt = 0, i, version = 1;
@@ -1079,8 +1111,7 @@ static int write_cache(int fd, struct perf_header *h __maybe_unused,
 	return ret;
 }
 
-static int write_stat(int fd __maybe_unused,
-		      struct perf_header *h __maybe_unused,
+static int write_stat(struct feat_fd *fd __maybe_unused,
 		      struct perf_evlist *evlist __maybe_unused)
 {
 	return 0;
@@ -2186,7 +2217,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 }
 
 struct feature_ops {
-	int (*write)(int fd, struct perf_header *h, struct perf_evlist *evlist);
+	int (*write)(struct feat_fd *fd, struct perf_evlist *evlist);
 	void (*print)(struct perf_header *h, int fd, FILE *fp);
 	int (*process)(struct perf_file_section *section,
 		       struct perf_header *h, int fd, void *data);
@@ -2295,7 +2326,7 @@ 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 *fd, struct perf_header *h, int type,
 			 struct perf_file_section **p,
 			 struct perf_evlist *evlist)
 {
@@ -2306,18 +2337,22 @@ static int do_write_feat(int fd, struct perf_header *h, int type,
 		if (!feat_ops[type].write)
 			return -1;
 
-		(*p)->offset = lseek(fd, 0, SEEK_CUR);
+		if (fd->buf) {
+			pr_err("do_write_feat to memory buffer\n");
+			return -1;
+		}
+		(*p)->offset = lseek(fd->fd, 0, SEEK_CUR);
 
-		err = feat_ops[type].write(fd, h, evlist);
+		err = feat_ops[type].write(fd, evlist);
 		if (err < 0) {
 			pr_debug("failed to write feature %s\n", feat_ops[type].name);
 
 			/* undo anything written */
-			lseek(fd, (*p)->offset, SEEK_SET);
+			lseek(fd->fd, (*p)->offset, SEEK_SET);
 
 			return -1;
 		}
-		(*p)->size = lseek(fd, 0, SEEK_CUR) - (*p)->offset;
+		(*p)->size = lseek(fd->fd, 0, SEEK_CUR) - (*p)->offset;
 		(*p)++;
 	}
 	return ret;
@@ -2327,12 +2362,22 @@ 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
+	 */
+	fdd = (struct feat_fd){
+		.fd  = fd,
+		.ph = header,
+	};
+
 	nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
 	if (!nr_sections)
 		return 0;
@@ -2347,7 +2392,7 @@ 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, header, feat, &p, evlist))
 			perf_header__clear_feat(header, feat);
 	}
 
@@ -2356,7 +2401,7 @@ static int perf_header__adds_write(struct perf_header *header,
 	 * 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);
@@ -2366,14 +2411,17 @@ static int perf_header__adds_write(struct perf_header *header,
 int perf_header__write_pipe(int fd)
 {
 	struct perf_pipe_file_header f_header;
+	struct feat_fd fdd;
 	int err;
 
+	fdd = (struct feat_fd){ .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;
@@ -2390,21 +2438,23 @@ int perf_session__write_header(struct perf_session *session,
 	struct perf_file_attr   f_attr;
 	struct perf_header *header = &session->header;
 	struct perf_evsel *evsel;
+	struct feat_fd fdd;
 	u64 attr_offset;
 	int err;
 
+	fdd = (struct feat_fd){ .fd = fd};
 	lseek(fd, sizeof(f_header), SEEK_SET);
 
 	evlist__for_each_entry(session->evlist, evsel) {
 		evsel->id_offset = lseek(fd, 0, SEEK_CUR);
-		err = do_write(fd, evsel->id, evsel->ids * sizeof(u64));
+		err = do_write(&fdd, evsel->id, evsel->ids * sizeof(u64));
 		if (err < 0) {
 			pr_debug("failed to write perf header\n");
 			return err;
 		}
 	}
 
-	attr_offset = lseek(fd, 0, SEEK_CUR);
+	attr_offset = lseek(fdd.fd, 0, SEEK_CUR);
 
 	evlist__for_each_entry(evlist, evsel) {
 		f_attr = (struct perf_file_attr){
@@ -2414,7 +2464,7 @@ int perf_session__write_header(struct perf_session *session,
 				.size   = evsel->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;
@@ -2449,7 +2499,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;
@@ -2724,6 +2774,10 @@ 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 = {
+		.fd = STDOUT_FILENO,
+		.ph = ph,
+	};
 	ssize_t ret;
 
 	ret = readn(fd, header, sizeof(*header));
@@ -2738,7 +2792,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;
@@ -3246,6 +3300,7 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 	union perf_event ev;
 	struct tracing_data *tdata;
 	ssize_t size = 0, aligned_size = 0, padding;
+	struct feat_fd fdd;
 	int err __maybe_unused = 0;
 
 	/*
@@ -3280,7 +3335,8 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 	 */
 	tracing_data_put(tdata);
 
-	if (write_padded(fd, NULL, 0, padding))
+	fdd = (struct feat_fd){ .fd = fd };
+	if (write_padded(&fdd, NULL, 0, padding))
 		return -1;
 
 	return aligned_size;
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index d30109b421ee..9d8dcd5eb727 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -144,7 +144,12 @@ bool is_perf_magic(u64 magic);
 
 #define NAME_ALIGN 64
 
-int write_padded(int fd, const void *bf, size_t count, size_t count_aligned);
+struct feat_fd;
+
+int do_write(struct feat_fd *fd, const void *buf, size_t size);
+
+int write_padded(struct feat_fd *fd, const void *bf,
+		 size_t count, size_t count_aligned);
 
 /*
  * arch specific callback
-- 
2.13.0.303.g4ebf302169-goog

Re: [PATCH 3/7] perf header: use struct feat_fd for write

[Jiri Olsa]

On Wed, May 17, 2017 at 09:15:58PM -0700, David Carrillo-Cisneros wrote:
> As preparation for using header records in pipe mode, replace
> int fd with struct feat_fd fd in write functions for all header
> record types.
> 
> Record types that are not used in pipe-mode (such as auxtrace) will
> never be called with fd->buf set and fail if so.

please separate the struct feat_fd addition and
the fd->buf support

thanks,
jirka

Re: [PATCH 3/7] perf header: use struct feat_fd for write

[David Carrillo-Cisneros]

On Thu, May 18, 2017 at 9:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, May 17, 2017 at 09:15:58PM -0700, David Carrillo-Cisneros wrote:
>> As preparation for using header records in pipe mode, replace
>> int fd with struct feat_fd fd in write functions for all header
>> record types.
>>
>> Record types that are not used in pipe-mode (such as auxtrace) will
>> never be called with fd->buf set and fail if so.
>
> please separate the struct feat_fd addition and
> the fd->buf support

Will do.

[PATCH 4/7] perf header: use struct feat_fd for print

[David Carrillo-Cisneros]

As preparation for using header records in pipe mode, replace
int fd with struct feat_fd fd in print functions for all header
record types.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/header.c | 104 ++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 55 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 5ca603b7a7a3..e28770d12c1c 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1117,62 +1117,56 @@ static int write_stat(struct feat_fd *fd __maybe_unused,
 	return 0;
 }
 
-static void print_hostname(struct perf_header *ph, int fd __maybe_unused,
-			   FILE *fp)
+static void print_hostname(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# hostname : %s\n", ph->env.hostname);
+	fprintf(fp, "# hostname : %s\n", fd->ph->env.hostname);
 }
 
-static void print_osrelease(struct perf_header *ph, int fd __maybe_unused,
-			    FILE *fp)
+static void print_osrelease(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# os release : %s\n", ph->env.os_release);
+	fprintf(fp, "# os release : %s\n", fd->ph->env.os_release);
 }
 
-static void print_arch(struct perf_header *ph, int fd __maybe_unused, FILE *fp)
+static void print_arch(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# arch : %s\n", ph->env.arch);
+	fprintf(fp, "# arch : %s\n", fd->ph->env.arch);
 }
 
-static void print_cpudesc(struct perf_header *ph, int fd __maybe_unused,
-			  FILE *fp)
+static void print_cpudesc(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# cpudesc : %s\n", ph->env.cpu_desc);
+	fprintf(fp, "# cpudesc : %s\n", fd->ph->env.cpu_desc);
 }
 
-static void print_nrcpus(struct perf_header *ph, int fd __maybe_unused,
-			 FILE *fp)
+static void print_nrcpus(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# nrcpus online : %u\n", ph->env.nr_cpus_online);
-	fprintf(fp, "# nrcpus avail : %u\n", ph->env.nr_cpus_avail);
+	fprintf(fp, "# nrcpus online : %u\n", fd->ph->env.nr_cpus_online);
+	fprintf(fp, "# nrcpus avail : %u\n", fd->ph->env.nr_cpus_avail);
 }
 
-static void print_version(struct perf_header *ph, int fd __maybe_unused,
-			  FILE *fp)
+static void print_version(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# perf version : %s\n", ph->env.version);
+	fprintf(fp, "# perf version : %s\n", fd->ph->env.version);
 }
 
-static void print_cmdline(struct perf_header *ph, int fd __maybe_unused,
-			  FILE *fp)
+static void print_cmdline(struct feat_fd *fd, FILE *fp)
 {
 	int nr, i;
 
-	nr = ph->env.nr_cmdline;
+	nr = fd->ph->env.nr_cmdline;
 
 	fprintf(fp, "# cmdline : ");
 
 	for (i = 0; i < nr; i++)
-		fprintf(fp, "%s ", ph->env.cmdline_argv[i]);
+		fprintf(fp, "%s ", fd->ph->env.cmdline_argv[i]);
 	fputc('\n', fp);
 }
 
-static void print_cpu_topology(struct perf_header *ph, int fd __maybe_unused,
-			       FILE *fp)
+static void print_cpu_topology(struct feat_fd *fd, FILE *fp)
 {
+	struct perf_header *ph = fd->ph;
+	int cpu_nr = ph->env.nr_cpus_avail;
 	int nr, i;
 	char *str;
-	int cpu_nr = ph->env.nr_cpus_avail;
 
 	nr = ph->env.nr_sibling_cores;
 	str = ph->env.sibling_cores;
@@ -1312,9 +1306,9 @@ static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
 	return fprintf(fp, ", %s = %s", name, val);
 }
 
-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_evsel *evsel, *events = read_event_desc(ph, fd);
+	struct perf_evsel *evsel, *events = read_event_desc(fd->ph, fd->fd);
 	u32 j;
 	u64 *id;
 
@@ -1344,20 +1338,18 @@ static void print_event_desc(struct perf_header *ph, int fd, FILE *fp)
 	free_event_desc(events);
 }
 
-static void print_total_mem(struct perf_header *ph, int fd __maybe_unused,
-			    FILE *fp)
+static void print_total_mem(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# total memory : %Lu kB\n", ph->env.total_mem);
+	fprintf(fp, "# total memory : %llu kB\n", fd->ph->env.total_mem);
 }
 
-static void print_numa_topology(struct perf_header *ph, int fd __maybe_unused,
-				FILE *fp)
+static void print_numa_topology(struct feat_fd *fd, FILE *fp)
 {
 	int i;
 	struct numa_node *n;
 
-	for (i = 0; i < ph->env.nr_numa_nodes; i++) {
-		n = &ph->env.numa_nodes[i];
+	for (i = 0; i < fd->ph->env.nr_numa_nodes; i++) {
+		n = &fd->ph->env.numa_nodes[i];
 
 		fprintf(fp, "# node%u meminfo  : total = %"PRIu64" kB,"
 			    " free = %"PRIu64" kB\n",
@@ -1368,56 +1360,51 @@ static void print_numa_topology(struct perf_header *ph, int fd __maybe_unused,
 	}
 }
 
-static void print_cpuid(struct perf_header *ph, int fd __maybe_unused, FILE *fp)
+static void print_cpuid(struct feat_fd *fd, FILE *fp)
 {
-	fprintf(fp, "# cpuid : %s\n", ph->env.cpuid);
+	fprintf(fp, "# cpuid : %s\n", fd->ph->env.cpuid);
 }
 
-static void print_branch_stack(struct perf_header *ph __maybe_unused,
-			       int fd __maybe_unused, FILE *fp)
+static void print_branch_stack(struct feat_fd *fd __maybe_unused, FILE *fp)
 {
 	fprintf(fp, "# contains samples with branch stack\n");
 }
 
-static void print_auxtrace(struct perf_header *ph __maybe_unused,
-			   int fd __maybe_unused, FILE *fp)
+static void print_auxtrace(struct feat_fd *fd __maybe_unused, FILE *fp)
 {
 	fprintf(fp, "# contains AUX area data (e.g. instruction trace)\n");
 }
 
-static void print_stat(struct perf_header *ph __maybe_unused,
-		       int fd __maybe_unused, FILE *fp)
+static void print_stat(struct feat_fd *fd __maybe_unused, FILE *fp)
 {
 	fprintf(fp, "# contains stat data\n");
 }
 
-static void print_cache(struct perf_header *ph __maybe_unused,
-			int fd __maybe_unused, FILE *fp __maybe_unused)
+static void print_cache(struct feat_fd *fd, FILE *fp __maybe_unused)
 {
 	int i;
 
 	fprintf(fp, "# CPU cache info:\n");
-	for (i = 0; i < ph->env.caches_cnt; i++) {
+	for (i = 0; i < fd->ph->env.caches_cnt; i++) {
 		fprintf(fp, "#  ");
-		cpu_cache_level__fprintf(fp, &ph->env.caches[i]);
+		cpu_cache_level__fprintf(fp, &fd->ph->env.caches[i]);
 	}
 }
 
-static void print_pmu_mappings(struct perf_header *ph, int fd __maybe_unused,
-			       FILE *fp)
+static void print_pmu_mappings(struct feat_fd *fd, FILE *fp)
 {
 	const char *delimiter = "# pmu mappings: ";
 	char *str, *tmp;
 	u32 pmu_num;
 	u32 type;
 
-	pmu_num = ph->env.nr_pmu_mappings;
+	pmu_num = fd->ph->env.nr_pmu_mappings;
 	if (!pmu_num) {
 		fprintf(fp, "# pmu mappings: not available\n");
 		return;
 	}
 
-	str = ph->env.pmu_mappings;
+	str = fd->ph->env.pmu_mappings;
 
 	while (pmu_num) {
 		type = strtoul(str, &tmp, 0);
@@ -1440,14 +1427,13 @@ static void print_pmu_mappings(struct perf_header *ph, int fd __maybe_unused,
 	fprintf(fp, "# pmu mappings: unable to read\n");
 }
 
-static void print_group_desc(struct perf_header *ph, int fd __maybe_unused,
-			     FILE *fp)
+static void print_group_desc(struct feat_fd *fd, FILE *fp)
 {
 	struct perf_session *session;
 	struct perf_evsel *evsel;
 	u32 nr = 0;
 
-	session = container_of(ph, struct perf_session, header);
+	session = container_of(fd->ph, struct perf_session, header);
 
 	evlist__for_each_entry(session->evlist, evsel) {
 		if (perf_evsel__is_group_leader(evsel) &&
@@ -2218,7 +2204,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 
 struct feature_ops {
 	int (*write)(struct feat_fd *fd, struct perf_evlist *evlist);
-	void (*print)(struct perf_header *h, int fd, FILE *fp);
+	void (*print)(struct feat_fd *fd, FILE *fp);
 	int (*process)(struct perf_file_section *section,
 		       struct perf_header *h, int fd, void *data);
 	const char *name;
@@ -2271,6 +2257,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 "
@@ -2284,8 +2271,15 @@ static int perf_file_section__fprintf_info(struct perf_file_section *section,
 	if (!feat_ops[feat].print)
 		return 0;
 
+	fdd = (struct  feat_fd) {
+		.fd = fd,
+		.ph = ph,
+		.size = section->size,
+		.offset = section->offset,
+	};
+
 	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);
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 5/7] perf header: use struct feat_fd for process and read

[David Carrillo-Cisneros]

As preparation for using header records in pipe mode, replace
int fd with struct feat_fd fd in process and read functions for
all header record types.

To reduce the boiler plate, define and use the FEAT_PROCESS_STR_FUN
macro for the common case of header records that are a simple string.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/header.c | 405 ++++++++++++++++++-----------------------------
 tools/perf/util/header.h |   1 +
 2 files changed, 156 insertions(+), 250 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index e28770d12c1c..981f5e9685e2 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -141,27 +141,67 @@ static int do_write_string(struct feat_fd *fd, const char *str)
 	return write_padded(fd, str, olen, len);
 }
 
-static char *do_read_string(int fd, struct perf_header *ph)
+static int __do_read(struct feat_fd *fd, void *addr, ssize_t size)
+{
+	if (!fd->buf) {
+		ssize_t ret = readn(fd->fd, addr, size);
+
+		if (ret != (ssize_t)size)
+			return ret < 0 ? (int)ret : -1;
+		return 0;
+	}
+
+	assert((ssize_t)fd->size > fd->offset);
+	if (size > (ssize_t)fd->size - fd->offset)
+		return -1;
+
+	memcpy(addr, fd->buf + fd->offset, size);
+	fd->offset += size;
+
+	return 0;
+}
+
+static int do_read_u32(struct feat_fd *fd, u32 *addr)
+{
+	int ret;
+
+	ret = __do_read(fd, addr, sizeof(*addr));
+	if (ret)
+		return ret;
+
+	if (fd->ph->needs_swap)
+		*addr = bswap_32(*addr);
+	return 0;
+}
+
+static int do_read_u64(struct feat_fd *fd, u64 *addr)
+{
+	int ret;
+
+	ret = __do_read(fd, addr, sizeof(*addr));
+	if (ret)
+		return ret;
+
+	if (fd->ph->needs_swap)
+		*addr = bswap_64(*addr);
+	return 0;
+}
+
+static char *do_read_string(struct feat_fd *fd)
 {
-	ssize_t sz, ret;
 	u32 len;
 	char *buf;
 
-	sz = readn(fd, &len, sizeof(len));
-	if (sz < (ssize_t)sizeof(len))
+	if (do_read_u32(fd, &len))
 		return NULL;
 
-	if (ph->needs_swap)
-		len = bswap_32(len);
-
 	buf = malloc(len);
 	if (!buf)
 		return NULL;
 
-	ret = readn(fd, buf, len);
-	if (ret == (ssize_t)len) {
+	if (!__do_read(fd, buf, len)) {
 		/*
-		 * strings are padded by zeroes
+		 * note that strings are padded by zeroes
 		 * thus the actual strlen of buf
 		 * may be less than len
 		 */
@@ -1207,8 +1247,7 @@ static void free_event_desc(struct perf_evsel *events)
 	free(events);
 }
 
-static struct perf_evsel *
-read_event_desc(struct perf_header *ph, int fd)
+static struct perf_evsel *read_event_desc(struct feat_fd *fd)
 {
 	struct perf_evsel *evsel, *events = NULL;
 	u64 *id;
@@ -1218,20 +1257,12 @@ read_event_desc(struct perf_header *ph, int fd)
 	size_t msz;
 
 	/* number of events */
-	ret = readn(fd, &nre, sizeof(nre));
-	if (ret != (ssize_t)sizeof(nre))
+	if (do_read_u32(fd, &nre))
 		goto error;
 
-	if (ph->needs_swap)
-		nre = bswap_32(nre);
-
-	ret = readn(fd, &sz, sizeof(sz));
-	if (ret != (ssize_t)sizeof(sz))
+	if (do_read_u32(fd, &sz))
 		goto error;
 
-	if (ph->needs_swap)
-		sz = bswap_32(sz);
-
 	/* buffer to hold on file attr struct */
 	buf = malloc(sz);
 	if (!buf)
@@ -1253,25 +1284,22 @@ read_event_desc(struct perf_header *ph, int fd)
 		 * must read entire on-file attr struct to
 		 * sync up with layout.
 		 */
-		ret = readn(fd, buf, sz);
+		ret = __do_read(fd, buf, sz);
 		if (ret != (ssize_t)sz)
 			goto error;
 
-		if (ph->needs_swap)
+		if (fd->ph->needs_swap)
 			perf_event__attr_swap(buf);
 
 		memcpy(&evsel->attr, buf, msz);
 
-		ret = readn(fd, &nr, sizeof(nr));
-		if (ret != (ssize_t)sizeof(nr))
+		if (do_read_u32(fd, &nr))
 			goto error;
 
-		if (ph->needs_swap) {
-			nr = bswap_32(nr);
+		if (fd->ph->needs_swap)
 			evsel->needs_swap = true;
-		}
 
-		evsel->name = do_read_string(fd, ph);
+		evsel->name = do_read_string(fd);
 
 		if (!nr)
 			continue;
@@ -1283,11 +1311,8 @@ read_event_desc(struct perf_header *ph, int fd)
 		evsel->id = id;
 
 		for (j = 0 ; j < nr; j++) {
-			ret = readn(fd, id, sizeof(*id));
-			if (ret != (ssize_t)sizeof(*id))
+			if (do_read_u64(fd, id))
 				goto error;
-			if (ph->needs_swap)
-				*id = bswap_64(*id);
 			id++;
 		}
 	}
@@ -1308,7 +1333,7 @@ static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
 
 static void print_event_desc(struct feat_fd *fd, FILE *fp)
 {
-	struct perf_evsel *evsel, *events = read_event_desc(fd->ph, fd->fd);
+	struct perf_evsel *evsel, *events = read_event_desc(fd);
 	u32 j;
 	u64 *id;
 
@@ -1598,113 +1623,61 @@ static int perf_header__read_build_ids(struct perf_header *header,
 	return err;
 }
 
-static int process_tracing_data(struct perf_file_section *section __maybe_unused,
-				struct perf_header *ph __maybe_unused,
-				int fd, void *data)
-{
-	ssize_t ret = trace_report(fd, data, false);
-	return ret < 0 ? -1 : 0;
+/* Macro for features that simply need to read and store a string. */
+#define FEAT_PROCESS_STR_FUN(__feat, __feat_env) \
+static int process_##__feat(struct feat_fd *fd, void *data __maybe_unused) \
+{\
+	fd->ph->env.__feat_env = do_read_string(fd); \
+	return fd->ph->env.__feat_env ? 0 : -ENOMEM; \
 }
 
-static int process_build_id(struct perf_file_section *section,
-			    struct perf_header *ph, int fd,
-			    void *data __maybe_unused)
+FEAT_PROCESS_STR_FUN(hostname, hostname);
+FEAT_PROCESS_STR_FUN(osrelease, os_release);
+FEAT_PROCESS_STR_FUN(version, version);
+FEAT_PROCESS_STR_FUN(arch, arch);
+FEAT_PROCESS_STR_FUN(cpudesc, cpu_desc);
+FEAT_PROCESS_STR_FUN(cpuid, cpuid);
+
+static int process_build_id(struct feat_fd *fd, void *data __maybe_unused)
 {
-	if (perf_header__read_build_ids(ph, fd, section->offset, section->size))
+	if (perf_header__read_build_ids(fd->ph, fd->fd, fd->offset, fd->size))
 		pr_debug("Failed to read buildids, continuing...\n");
 	return 0;
 }
 
-static int process_hostname(struct perf_file_section *section __maybe_unused,
-			    struct perf_header *ph, int fd,
-			    void *data __maybe_unused)
-{
-	ph->env.hostname = do_read_string(fd, ph);
-	return ph->env.hostname ? 0 : -ENOMEM;
-}
-
-static int process_osrelease(struct perf_file_section *section __maybe_unused,
-			     struct perf_header *ph, int fd,
-			     void *data __maybe_unused)
-{
-	ph->env.os_release = do_read_string(fd, ph);
-	return ph->env.os_release ? 0 : -ENOMEM;
-}
-
-static int process_version(struct perf_file_section *section __maybe_unused,
-			   struct perf_header *ph, int fd,
-			   void *data __maybe_unused)
+static int process_tracing_data(struct feat_fd *fd, void *data)
 {
-	ph->env.version = do_read_string(fd, ph);
-	return ph->env.version ? 0 : -ENOMEM;
-}
+	ssize_t ret = trace_report(fd->fd, data, false);
 
-static int process_arch(struct perf_file_section *section __maybe_unused,
-			struct perf_header *ph,	int fd,
-			void *data __maybe_unused)
-{
-	ph->env.arch = do_read_string(fd, ph);
-	return ph->env.arch ? 0 : -ENOMEM;
+	return ret < 0 ? -1 : 0;
 }
 
-static int process_nrcpus(struct perf_file_section *section __maybe_unused,
-			  struct perf_header *ph, int fd,
-			  void *data __maybe_unused)
+static int process_nrcpus(struct feat_fd *fd, void *data __maybe_unused)
 {
-	ssize_t ret;
-	u32 nr;
-
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
-		return -1;
-
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
-	ph->env.nr_cpus_avail = nr;
-
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
-		return -1;
+	int ret;
+	u32 nr_cpus_avail, nr_cpus_online;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
+	ret = do_read_u32(fd, &nr_cpus_avail);
+	if (ret)
+		return ret;
 
-	ph->env.nr_cpus_online = nr;
+	ret = do_read_u32(fd, &nr_cpus_online);
+	if (ret)
+		return ret;
+	fd->ph->env.nr_cpus_avail = (int)nr_cpus_avail;
+	fd->ph->env.nr_cpus_online = (int)nr_cpus_online;
 	return 0;
 }
 
-static int process_cpudesc(struct perf_file_section *section __maybe_unused,
-			   struct perf_header *ph, int fd,
-			   void *data __maybe_unused)
-{
-	ph->env.cpu_desc = do_read_string(fd, ph);
-	return ph->env.cpu_desc ? 0 : -ENOMEM;
-}
-
-static int process_cpuid(struct perf_file_section *section __maybe_unused,
-			 struct perf_header *ph,  int fd,
-			 void *data __maybe_unused)
-{
-	ph->env.cpuid = do_read_string(fd, ph);
-	return ph->env.cpuid ? 0 : -ENOMEM;
-}
-
-static int process_total_mem(struct perf_file_section *section __maybe_unused,
-			     struct perf_header *ph, int fd,
-			     void *data __maybe_unused)
+static int process_total_mem(struct feat_fd *fd, void *data __maybe_unused)
 {
-	uint64_t mem;
-	ssize_t ret;
+	u64 total_mem;
+	int ret;
 
-	ret = readn(fd, &mem, sizeof(mem));
-	if (ret != sizeof(mem))
+	ret = do_read_u64(fd, &total_mem);
+	if (ret)
 		return -1;
-
-	if (ph->needs_swap)
-		mem = bswap_64(mem);
-
-	ph->env.total_mem = mem;
+	fd->ph->env.total_mem = (unsigned long long)total_mem;
 	return 0;
 }
 
@@ -1741,17 +1714,15 @@ perf_evlist__set_event_name(struct perf_evlist *evlist,
 }
 
 static int
-process_event_desc(struct perf_file_section *section __maybe_unused,
-		   struct perf_header *header, int fd,
-		   void *data __maybe_unused)
+process_event_desc(struct feat_fd *fd, void *data __maybe_unused)
 {
 	struct perf_session *session;
-	struct perf_evsel *evsel, *events = read_event_desc(header, fd);
+	struct perf_evsel *evsel, *events = read_event_desc(fd);
 
 	if (!events)
 		return 0;
 
-	session = container_of(header, struct perf_session, header);
+	session = container_of(fd->ph, struct perf_session, header);
 	for (evsel = events; evsel->attr.size; evsel++)
 		perf_evlist__set_event_name(session->evlist, evsel);
 
@@ -1760,24 +1731,17 @@ process_event_desc(struct perf_file_section *section __maybe_unused,
 	return 0;
 }
 
-static int process_cmdline(struct perf_file_section *section,
-			   struct perf_header *ph, int fd,
-			   void *data __maybe_unused)
+static int process_cmdline(struct feat_fd *fd, void *data __maybe_unused)
 {
-	ssize_t ret;
 	char *str, *cmdline = NULL, **argv = NULL;
 	u32 nr, i, len = 0;
 
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
+	if (do_read_u32(fd, &nr))
 		return -1;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
-	ph->env.nr_cmdline = nr;
+	fd->ph->env.nr_cmdline = nr;
 
-	cmdline = zalloc(section->size + nr + 1);
+	cmdline = zalloc(fd->size + nr + 1);
 	if (!cmdline)
 		return -1;
 
@@ -1786,7 +1750,7 @@ static int process_cmdline(struct perf_file_section *section,
 		goto error;
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		if (!str)
 			goto error;
 
@@ -1795,8 +1759,8 @@ static int process_cmdline(struct perf_file_section *section,
 		len += strlen(str) + 1;
 		free(str);
 	}
-	ph->env.cmdline = cmdline;
-	ph->env.cmdline_argv = (const char **) argv;
+	fd->ph->env.cmdline = cmdline;
+	fd->ph->env.cmdline_argv = (const char **) argv;
 	return 0;
 
 error:
@@ -1805,65 +1769,51 @@ static int process_cmdline(struct perf_file_section *section,
 	return -1;
 }
 
-static int process_cpu_topology(struct perf_file_section *section,
-				struct perf_header *ph, int fd,
-				void *data __maybe_unused)
+static int process_cpu_topology(struct feat_fd *fd, void *data __maybe_unused)
 {
-	ssize_t ret;
 	u32 nr, i;
 	char *str;
 	struct strbuf sb;
-	int cpu_nr = ph->env.nr_cpus_avail;
-	u64 size = 0;
+	int cpu_nr = fd->ph->env.nr_cpus_avail;
+	struct perf_header *ph = fd->ph;
+	u64 start_offset = fd->offset;
 
 	ph->env.cpu = calloc(cpu_nr, sizeof(*ph->env.cpu));
 	if (!ph->env.cpu)
 		return -1;
 
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
+	if (do_read_u32(fd, &nr))
 		goto free_cpu;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
 	ph->env.nr_sibling_cores = nr;
-	size += sizeof(u32);
 	if (strbuf_init(&sb, 128) < 0)
 		goto free_cpu;
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		if (!str)
 			goto error;
 
 		/* include a NULL character at the end */
 		if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
 			goto error;
-		size += string_size(str);
 		free(str);
 	}
 	ph->env.sibling_cores = strbuf_detach(&sb, NULL);
 
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
+	if (do_read_u32(fd, &nr))
 		return -1;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
 	ph->env.nr_sibling_threads = nr;
-	size += sizeof(u32);
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		if (!str)
 			goto error;
 
 		/* include a NULL character at the end */
 		if (strbuf_add(&sb, str, strlen(str) + 1) < 0)
 			goto error;
-		size += string_size(str);
 		free(str);
 	}
 	ph->env.sibling_threads = strbuf_detach(&sb, NULL);
@@ -1872,28 +1822,20 @@ static int process_cpu_topology(struct perf_file_section *section,
 	 * The header may be from old perf,
 	 * which doesn't include core id and socket id information.
 	 */
-	if (section->size <= size) {
+	if (fd->size <= fd->offset - start_offset) {
 		zfree(&ph->env.cpu);
 		return 0;
 	}
 
 	for (i = 0; i < (u32)cpu_nr; i++) {
-		ret = readn(fd, &nr, sizeof(nr));
-		if (ret != sizeof(nr))
+		if (do_read_u32(fd, &nr))
 			goto free_cpu;
 
-		if (ph->needs_swap)
-			nr = bswap_32(nr);
-
 		ph->env.cpu[i].core_id = nr;
 
-		ret = readn(fd, &nr, sizeof(nr));
-		if (ret != sizeof(nr))
+		if (do_read_u32(fd, &nr))
 			goto free_cpu;
 
-		if (ph->needs_swap)
-			nr = bswap_32(nr);
-
 		if (nr != (u32)-1 && nr > (u32)cpu_nr) {
 			pr_debug("socket_id number is too big."
 				 "You may need to upgrade the perf tool.\n");
@@ -1912,23 +1854,17 @@ static int process_cpu_topology(struct perf_file_section *section,
 	return -1;
 }
 
-static int process_numa_topology(struct perf_file_section *section __maybe_unused,
-				 struct perf_header *ph, int fd,
-				 void *data __maybe_unused)
+static int process_numa_topology(struct feat_fd *fd, void *data __maybe_unused)
 {
+	struct perf_header *ph = fd->ph;
 	struct numa_node *nodes, *n;
-	ssize_t ret;
 	u32 nr, i;
 	char *str;
 
 	/* nr nodes */
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
+	if (do_read_u32(fd, &nr))
 		return -1;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
 	nodes = zalloc(sizeof(*nodes) * nr);
 	if (!nodes)
 		return -ENOMEM;
@@ -1937,25 +1873,16 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 		n = &nodes[i];
 
 		/* node number */
-		ret = readn(fd, &n->node, sizeof(u32));
-		if (ret != sizeof(n->node))
+		if (do_read_u32(fd, &n->node))
 			goto error;
 
-		ret = readn(fd, &n->mem_total, sizeof(u64));
-		if (ret != sizeof(u64))
+		if (do_read_u64(fd, &n->mem_total))
 			goto error;
 
-		ret = readn(fd, &n->mem_free, sizeof(u64));
-		if (ret != sizeof(u64))
+		if (do_read_u64(fd, &n->mem_free))
 			goto error;
 
-		if (ph->needs_swap) {
-			n->node      = bswap_32(n->node);
-			n->mem_total = bswap_64(n->mem_total);
-			n->mem_free  = bswap_64(n->mem_free);
-		}
-
-		str = do_read_string(fd, ph);
+		str = do_read_string(fd);
 		if (!str)
 			goto error;
 
@@ -1974,23 +1901,17 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 	return -1;
 }
 
-static int process_pmu_mappings(struct perf_file_section *section __maybe_unused,
-				struct perf_header *ph, int fd,
-				void *data __maybe_unused)
+static int process_pmu_mappings(struct feat_fd *fd, void *data __maybe_unused)
 {
-	ssize_t ret;
+	struct perf_header *ph = fd->ph;
 	char *name;
 	u32 pmu_num;
 	u32 type;
 	struct strbuf sb;
 
-	ret = readn(fd, &pmu_num, sizeof(pmu_num));
-	if (ret != sizeof(pmu_num))
+	if (do_read_u32(fd, &pmu_num))
 		return -1;
 
-	if (ph->needs_swap)
-		pmu_num = bswap_32(pmu_num);
-
 	if (!pmu_num) {
 		pr_debug("pmu mappings not available\n");
 		return 0;
@@ -2001,12 +1922,10 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused
 		return -1;
 
 	while (pmu_num) {
-		if (readn(fd, &type, sizeof(type)) != sizeof(type))
+		if (do_read_u32(fd, &type))
 			goto error;
-		if (ph->needs_swap)
-			type = bswap_32(type);
 
-		name = do_read_string(fd, ph);
+		name = do_read_string(fd);
 		if (!name)
 			goto error;
 
@@ -2030,10 +1949,9 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused
 	return -1;
 }
 
-static int process_group_desc(struct perf_file_section *section __maybe_unused,
-			      struct perf_header *ph, int fd,
-			      void *data __maybe_unused)
+static int process_group_desc(struct feat_fd *fd, void *data __maybe_unused)
 {
+	struct perf_header *ph = fd->ph;
 	size_t ret = -1;
 	u32 i, nr, nr_groups;
 	struct perf_session *session;
@@ -2044,12 +1962,9 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 		u32 nr_members;
 	} *desc;
 
-	if (readn(fd, &nr_groups, sizeof(nr_groups)) != sizeof(nr_groups))
+	if (do_read_u32(fd, &nr_groups))
 		return -1;
 
-	if (ph->needs_swap)
-		nr_groups = bswap_32(nr_groups);
-
 	ph->env.nr_groups = nr_groups;
 	if (!nr_groups) {
 		pr_debug("group desc not available\n");
@@ -2061,20 +1976,15 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 		return -1;
 
 	for (i = 0; i < nr_groups; i++) {
-		desc[i].name = do_read_string(fd, ph);
+		desc[i].name = do_read_string(fd);
 		if (!desc[i].name)
 			goto out_free;
 
-		if (readn(fd, &desc[i].leader_idx, sizeof(u32)) != sizeof(u32))
+		if (do_read_u32(fd, &desc[i].leader_idx))
 			goto out_free;
 
-		if (readn(fd, &desc[i].nr_members, sizeof(u32)) != sizeof(u32))
+		if (do_read_u32(fd, &desc[i].nr_members))
 			goto out_free;
-
-		if (ph->needs_swap) {
-			desc[i].leader_idx = bswap_32(desc[i].leader_idx);
-			desc[i].nr_members = bswap_32(desc[i].nr_members);
-		}
 	}
 
 	/*
@@ -2124,44 +2034,35 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 	return ret;
 }
 
-static int process_auxtrace(struct perf_file_section *section,
-			    struct perf_header *ph, int fd,
-			    void *data __maybe_unused)
+static int process_auxtrace(struct feat_fd *fd, void *data __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(ph, struct perf_session, header);
+	session = container_of(fd->ph, struct perf_session, header);
 
-	err = auxtrace_index__process(fd, section->size, session,
-				      ph->needs_swap);
+	err = auxtrace_index__process(fd->fd, fd->size, session,
+				      fd->ph->needs_swap);
 	if (err < 0)
 		pr_err("Failed to process auxtrace index\n");
 	return err;
 }
 
-static int process_cache(struct perf_file_section *section __maybe_unused,
-			 struct perf_header *ph __maybe_unused, int fd __maybe_unused,
-			 void *data __maybe_unused)
+static int process_cache(struct feat_fd *fd, void *data __maybe_unused)
 {
 	struct cpu_cache_level *caches;
 	u32 cnt, i, version;
 
-	if (readn(fd, &version, sizeof(version)) != sizeof(version))
+	pr_err("process_cache\n");
+	if (do_read_u32(fd, &version))
 		return -1;
 
-	if (ph->needs_swap)
-		version = bswap_32(version);
-
 	if (version != 1)
 		return -1;
 
-	if (readn(fd, &cnt, sizeof(cnt)) != sizeof(cnt))
+	if (do_read_u32(fd, &cnt))
 		return -1;
 
-	if (ph->needs_swap)
-		cnt = bswap_32(cnt);
-
 	caches = zalloc(sizeof(*caches) * cnt);
 	if (!caches)
 		return -1;
@@ -2170,10 +2071,8 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 		struct cpu_cache_level c;
 
 		#define _R(v)						\
-			if (readn(fd, &c.v, sizeof(u32)) != sizeof(u32))\
+			if (do_read_u32(fd, &c.v))\
 				goto out_free_caches;			\
-			if (ph->needs_swap)				\
-				c.v = bswap_32(c.v);			\
 
 		_R(level)
 		_R(line_size)
@@ -2182,7 +2081,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 		#undef _R
 
 		#define _R(v)				\
-			c.v = do_read_string(fd, ph);	\
+			c.v = do_read_string(fd);	\
 			if (!c.v)			\
 				goto out_free_caches;
 
@@ -2194,8 +2093,8 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 		caches[i] = c;
 	}
 
-	ph->env.caches = caches;
-	ph->env.caches_cnt = cnt;
+	fd->ph->env.caches = caches;
+	fd->ph->env.caches_cnt = cnt;
 	return 0;
 out_free_caches:
 	free(caches);
@@ -2205,8 +2104,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 struct feature_ops {
 	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 fd, void *data);
+	int (*process)(struct feat_fd *fd, void *data);
 	const char *name;
 	bool full_only;
 };
@@ -2747,6 +2645,13 @@ static int perf_file_section__process(struct perf_file_section *section,
 				      struct perf_header *ph,
 				      int feat, int fd, void *data)
 {
+	struct feat_fd fdd = {
+		.fd = fd,
+		.ph = ph,
+		.size = section->size,
+		.offset = section->offset,
+	};
+
 	if (lseek(fd, section->offset, SEEK_SET) == (off_t)-1) {
 		pr_debug("Failed to lseek to %" PRIu64 " offset for feature "
 			  "%d, continuing...\n", section->offset, feat);
@@ -2761,7 +2666,7 @@ static int perf_file_section__process(struct perf_file_section *section,
 	if (!feat_ops[feat].process)
 		return 0;
 
-	return feat_ops[feat].process(section, ph, fd, data);
+	return feat_ops[feat].process(&fdd, data);
 }
 
 static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 9d8dcd5eb727..86ab723cbee4 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -78,6 +78,7 @@ struct perf_header {
 	struct perf_env 	env;
 };
 
+struct feat_fd;
 struct perf_evlist;
 struct perf_session;
 
-- 
2.13.0.303.g4ebf302169-goog

Re: [PATCH 5/7] perf header: use struct feat_fd for process and read

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:00PM -0700, David Carrillo-Cisneros wrote:
> As preparation for using header records in pipe mode, replace
> int fd with struct feat_fd fd in process and read functions for
> all header record types.
> 
> To reduce the boiler plate, define and use the FEAT_PROCESS_STR_FUN
> macro for the common case of header records that are a simple string.

please separate those changes, AFAICS it's following:
  - using feat_fd in read functions
  - reworking do_read_string function
  - replacing readn with new do_read* functions
  - adding FEAT_PROCESS_STR_FUN

thanks, 
jirka

Re: [PATCH 5/7] perf header: use struct feat_fd for process and read

[David Carrillo-Cisneros]

On Thu, May 18, 2017 at 9:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, May 17, 2017 at 09:16:00PM -0700, David Carrillo-Cisneros wrote:
>> As preparation for using header records in pipe mode, replace
>> int fd with struct feat_fd fd in process and read functions for
>> all header record types.
>>
>> To reduce the boiler plate, define and use the FEAT_PROCESS_STR_FUN
>> macro for the common case of header records that are a simple string.
>
> please separate those changes, AFAICS it's following:
>   - using feat_fd in read functions
>   - reworking do_read_string function
>   - replacing readn with new do_read* functions
>   - adding FEAT_PROCESS_STR_FUN

Will do.

[PATCH 6/7] perf tool: make show-info in perf report a tool attribute

[David Carrillo-Cisneros]

Although defined as exclusive of perf report and perf script, the
attribute show_full_info interacts with the full_only attributes
of feature_ops, that are defined for all commands.

Make show_full_info a tool attribute so that any perf_event__process_*
callbacks that access to feature_ops can access to it.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/builtin-report.c | 5 ++---
 tools/perf/builtin-script.c | 6 +++---
 tools/perf/util/tool.h      | 1 +
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 22478ff2b706..08c90d65a252 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -54,7 +54,6 @@ struct report {
 	struct perf_tool	tool;
 	struct perf_session	*session;
 	bool			use_tui, use_gtk, use_stdio;
-	bool			show_full_info;
 	bool			show_threads;
 	bool			inverted_callchain;
 	bool			mem_mode;
@@ -806,7 +805,7 @@ int cmd_report(int argc, const char **argv)
 		     symbol__config_symfs),
 	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_full_info,
 		    "Display extended information about perf.data file"),
 	OPT_BOOLEAN(0, "source", &symbol_conf.annotate_src,
 		    "Interleave source code with assembly code (default)"),
@@ -1005,7 +1004,7 @@ int cmd_report(int argc, const char **argv)
 
 	if ((report.header || report.header_only) && !quiet) {
 		perf_session__fprintf_info(session, stdout,
-					   report.show_full_info);
+					   report.tool.show_full_info);
 		if (report.header_only) {
 			ret = 0;
 			goto error;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index d05aec491cff..1edfa0da0a6c 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2374,7 +2374,6 @@ int process_cpu_map_event(struct perf_tool *tool __maybe_unused,
 
 int cmd_script(int argc, const char **argv)
 {
-	bool show_full_info = false;
 	bool header = false;
 	bool header_only = false;
 	bool script_started = false;
@@ -2466,7 +2465,7 @@ int cmd_script(int argc, const char **argv)
 		     "Set the maximum stack depth when parsing the callchain, "
 		     "anything beyond the specified depth will be ignored. "
 		     "Default: kernel.perf_event_max_stack or " __stringify(PERF_MAX_STACK_DEPTH)),
-	OPT_BOOLEAN('I', "show-info", &show_full_info,
+	OPT_BOOLEAN('I', "show-info", &script.tool.show_full_info,
 		    "display extended information from perf.data file"),
 	OPT_BOOLEAN('\0', "show-kernel-path", &symbol_conf.show_kernel_path,
 		    "Show the path of [kernel.kallsyms]"),
@@ -2684,7 +2683,8 @@ int cmd_script(int argc, const char **argv)
 		return -1;
 
 	if (header || header_only) {
-		perf_session__fprintf_info(session, stdout, show_full_info);
+		perf_session__fprintf_info(session, stdout,
+					   script.tool.show_full_info);
 		if (header_only)
 			goto out_delete;
 	}
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 829471a1c6d7..795b2be7b51f 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -68,6 +68,7 @@ struct perf_tool {
 	bool		ordered_events;
 	bool		ordering_requires_timestamps;
 	bool		namespace_events;
+	bool		show_full_info;
 };
 
 #endif /* __PERF_TOOL_H */
-- 
2.13.0.303.g4ebf302169-goog

[PATCH 7/7] perf tools: add feature header record to pipe-mode

[David Carrillo-Cisneros]

Add header record types to pipe-mode, reusing the functions
used in file-mode and leveraging the new struct feat_fd.

Add the perf_event__synthesize_feature event call back to
process the new header records.

Before this patch:

  $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  ...

After this patch:
  $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  # hostname : lphh7
  # os release : 4.11.0-dbx-up_perf
  # perf version : 4.11.rc6.g6277c80
  # arch : x86_64
  # nrcpus online : 72
  # nrcpus avail : 72
  # cpudesc : Intel(R) Xeon(R) CPU E5-2696 v3 @ 2.30GHz
  # cpuid : GenuineIntel,6,63,2
  # total memory : 263457192 kB
  # cmdline : /root/perf record -o - -e cycles -c 100000 sleep 1
  # HEADER_CPU_TOPOLOGY info available, use -I to display
  # HEADER_NUMA_TOPOLOGY info available, use -I to display
  # pmu mappings: intel_bts = 6, uncore_imc_4 = 22, uncore_sbox_1 = 47, uncore_cbox_5 = 33, uncore_ha_0 = 16, uncore_cbox
   Percent |      Source code & Disassembly of kcore for cycles (9 samples)
  ...

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/builtin-annotate.c |   1 +
 tools/perf/builtin-inject.c   |   1 +
 tools/perf/builtin-record.c   |   7 ++
 tools/perf/builtin-report.c   |   1 +
 tools/perf/util/event.c       |  13 +++
 tools/perf/util/event.h       |  19 ++++
 tools/perf/util/header.c      | 224 ++++++++++++++++++++++++++++++++++--------
 tools/perf/util/header.h      |   9 ++
 tools/perf/util/session.c     |  12 +++
 tools/perf/util/tool.h        |   3 +-
 10 files changed, 248 insertions(+), 42 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index ce44edc30c71..ffe28002dc4f 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -398,6 +398,7 @@ int cmd_annotate(int argc, const char **argv)
 			.attr	= perf_event__process_attr,
 			.build_id = perf_event__process_build_id,
 			.tracing_data   = perf_event__process_tracing_data,
+			.feature	= perf_event__process_feature,
 			.ordered_events = true,
 			.ordering_requires_timestamps = true,
 		},
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index ea8db38eedd1..2b8032908fb2 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -770,6 +770,7 @@ int cmd_inject(int argc, const char **argv)
 			.finished_round	= perf_event__repipe_oe_synth,
 			.build_id	= perf_event__repipe_op2_synth,
 			.id_index	= perf_event__repipe_op2_synth,
+			.feature	= perf_event__repipe_op2_synth,
 		},
 		.input_name  = "-",
 		.samples = LIST_HEAD_INIT(inject.samples),
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ee7d0a82ccd0..a1bcb72b4195 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -799,6 +799,13 @@ static int record__synthesize(struct record *rec, bool tail)
 		return 0;
 
 	if (file->is_pipe) {
+		err = perf_event__synthesize_features(
+			tool, session, rec->evlist, 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 08c90d65a252..5a6917410469 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -717,6 +717,7 @@ int cmd_report(int argc, const char **argv)
 			.id_index	 = perf_event__process_id_index,
 			.auxtrace_info	 = perf_event__process_auxtrace_info,
 			.auxtrace	 = perf_event__process_auxtrace,
+			.feature	 = perf_event__process_feature,
 			.ordered_events	 = true,
 			.ordering_requires_timestamps = true,
 		},
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 142835c0ca0a..cef1322d2993 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -56,6 +56,19 @@ static const char *perf_event__names[] = {
 	[PERF_RECORD_STAT_ROUND]		= "STAT_ROUND",
 	[PERF_RECORD_EVENT_UPDATE]		= "EVENT_UPDATE",
 	[PERF_RECORD_TIME_CONV]			= "TIME_CONV",
+	[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",
+	[PERF_RECORD_HEADER_PMU_MAPPINGS]	= "PMU_MAPPINGS",
 };
 
 static const char *perf_ns__names[] = {
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index db2de6413518..d404f50260f8 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -244,6 +244,19 @@ enum perf_user_event_type { /* above any possible kernel type */
 	PERF_RECORD_STAT_ROUND			= 77,
 	PERF_RECORD_EVENT_UPDATE		= 78,
 	PERF_RECORD_TIME_CONV			= 79,
+	PERF_RECORD_HEADER_HOSTNAME		= 80,
+	PERF_RECORD_HEADER_OSRELEASE		= 81,
+	PERF_RECORD_HEADER_VERSION		= 82,
+	PERF_RECORD_HEADER_ARCH			= 83,
+	PERF_RECORD_HEADER_NRCPUS		= 84,
+	PERF_RECORD_HEADER_CPUDESC		= 85,
+	PERF_RECORD_HEADER_CPUID		= 86,
+	PERF_RECORD_HEADER_TOTAL_MEM		= 87,
+	PERF_RECORD_HEADER_CMDLINE		= 88,
+	PERF_RECORD_HEADER_EVENT_DESC		= 89,
+	PERF_RECORD_HEADER_CPU_TOPOLOGY		= 90,
+	PERF_RECORD_HEADER_NUMA_TOPOLOGY	= 91,
+	PERF_RECORD_HEADER_PMU_MAPPINGS		= 92,
 	PERF_RECORD_HEADER_MAX
 };
 
@@ -488,6 +501,11 @@ struct time_conv_event {
 	u64 time_zero;
 };
 
+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 mmap_event		mmap;
@@ -518,6 +536,7 @@ union perf_event {
 	struct stat_event		stat;
 	struct stat_round_event		stat_round;
 	struct time_conv_event		time_conv;
+	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 981f5e9685e2..f6ce7ca80c12 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -11,6 +11,7 @@
 #include <linux/list.h>
 #include <linux/kernel.h>
 #include <linux/bitops.h>
+#include <linux/stringify.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/utsname.h>
@@ -32,6 +33,7 @@
 #include "data.h"
 #include <api/fs/fs.h>
 #include "asm/bug.h"
+#include "tool.h"
 
 #include "sane_ctype.h"
 
@@ -2107,42 +2109,80 @@ struct feature_ops {
 	int (*process)(struct feat_fd *fd, void *data);
 	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, \
-		.process = process_##func, .full_only = true }
+#define FEAT_OPP(n, func, __full_only) \
+	[HEADER_##n] = {					\
+		.name = __stringify(HEADER_##n),		\
+		.write = write_##func,				\
+		.print = print_##func,				\
+		.full_only = __full_only,			\
+		.process = process_##func,			\
+		.record_type = PERF_RECORD_HEADER_##n		\
+	}
+
+#define FEAT_OPN(n, func, __full_only) \
+	[HEADER_##n] = {					\
+		.name = __stringify(HEADER_##n),		\
+		.write = write_##func,				\
+		.print = print_##func,				\
+		.full_only = __full_only,			\
+		.process = process_##func			\
+	}
 
 /* feature_ops not implemented: */
 #define print_tracing_data	NULL
 #define print_build_id		NULL
 
+#define process_branch_stack	NULL
+#define process_stat		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_OPP(HEADER_HOSTNAME,	hostname),
-	FEAT_OPP(HEADER_OSRELEASE,	osrelease),
-	FEAT_OPP(HEADER_VERSION,	version),
-	FEAT_OPP(HEADER_ARCH,		arch),
-	FEAT_OPP(HEADER_NRCPUS,		nrcpus),
-	FEAT_OPP(HEADER_CPUDESC,	cpudesc),
-	FEAT_OPP(HEADER_CPUID,		cpuid),
-	FEAT_OPP(HEADER_TOTAL_MEM,	total_mem),
-	FEAT_OPP(HEADER_EVENT_DESC,	event_desc),
-	FEAT_OPP(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(HEADER_PMU_MAPPINGS,	pmu_mappings),
-	FEAT_OPP(HEADER_GROUP_DESC,	group_desc),
-	FEAT_OPP(HEADER_AUXTRACE,	auxtrace),
-	FEAT_OPA(HEADER_STAT,		stat),
-	FEAT_OPF(HEADER_CACHE,		cache),
+	FEAT_OPP(TRACING_DATA,	tracing_data,	false),
+	FEAT_OPP(BUILD_ID,	build_id,	false),
+	FEAT_OPP(HOSTNAME,	hostname,	false),
+	FEAT_OPP(OSRELEASE,	osrelease,	false),
+	FEAT_OPP(VERSION,	version,	false),
+	FEAT_OPP(ARCH,		arch,		false),
+	FEAT_OPP(NRCPUS,	nrcpus,		false),
+	FEAT_OPP(CPUDESC,	cpudesc,	false),
+	FEAT_OPP(CPUID,		cpuid,		false),
+	FEAT_OPP(TOTAL_MEM,	total_mem,	false),
+	FEAT_OPP(EVENT_DESC,	event_desc,	false),
+	FEAT_OPP(CMDLINE,	cmdline,	false),
+	FEAT_OPP(CPU_TOPOLOGY,	cpu_topology,	true),
+	FEAT_OPP(NUMA_TOPOLOGY,	numa_topology,	true),
+	FEAT_OPN(BRANCH_STACK,	branch_stack,	false),
+	FEAT_OPP(PMU_MAPPINGS,	pmu_mappings,	false),
+	FEAT_OPN(GROUP_DESC,	group_desc,	false),
+	FEAT_OPN(AUXTRACE,	auxtrace,	false),
+	FEAT_OPN(STAT,		stat,		false),
+	FEAT_OPN(CACHE,		cache,		true),
+};
+
+/*
+ * 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),
+	REC2FEAT(PMU_MAPPINGS),
 };
 
 struct header_print_data {
@@ -2218,33 +2258,33 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
 	return 0;
 }
 
-static int do_write_feat(struct feat_fd *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->ph, type)) {
 		if (!feat_ops[type].write)
 			return -1;
 
-		if (fd->buf) {
-			pr_err("do_write_feat to memory buffer\n");
+		if (fdd->buf) {
+			pr_debug("Called do_write_feat for memory buffer\n");
 			return -1;
 		}
-		(*p)->offset = lseek(fd->fd, 0, SEEK_CUR);
+		(*p)->offset = lseek(fdd->fd, 0, SEEK_CUR);
 
-		err = feat_ops[type].write(fd, evlist);
+		err = feat_ops[type].write(fdd, evlist);
 		if (err < 0) {
 			pr_debug("failed to write feature %s\n", feat_ops[type].name);
 
 			/* undo anything written */
-			lseek(fd->fd, (*p)->offset, SEEK_SET);
+			lseek(fdd->fd, (*p)->offset, SEEK_SET);
 
 			return -1;
 		}
-		(*p)->size = lseek(fd->fd, 0, SEEK_CUR) - (*p)->offset;
+		(*p)->size = lseek(fdd->fd, 0, SEEK_CUR) - (*p)->offset;
 		(*p)++;
 	}
 	return ret;
@@ -2261,10 +2301,6 @@ static int perf_header__adds_write(struct perf_header *header,
 	int feat;
 	int err;
 
-	/*
-	 * may write more than needed due to dropped feature, but
-	 * this is okay, reader will skip the mising entries
-	 */
 	fdd = (struct feat_fd){
 		.fd  = fd,
 		.ph = header,
@@ -2284,7 +2320,7 @@ 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(&fdd, header, feat, &p, evlist))
+		if (do_write_feat(&fdd, feat, &p, evlist))
 			perf_header__clear_feat(header, feat);
 	}
 
@@ -2941,6 +2977,112 @@ int perf_event__synthesize_attr(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 = PERF_ALIGN(sz, getpagesize());
+
+	memset(&fdd, 0, sizeof(fdd));
+
+	fdd.buf = malloc(sz);
+	if (!fdd.buf)
+		return -ENOMEM;
+
+	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;
+
+		if (!feat_ops[feat].record_type) {
+			pr_debug("No record type for header :%d\n", feat);
+			continue;
+		}
+
+		fdd.offset = sizeof(*fe);
+
+		ret = feat_ops[feat].write(&fdd, evlist);
+		if (ret || fdd.offset <= (ssize_t)sizeof(*fe)) {
+			pr_debug("Error writing feature\n");
+			continue;
+		}
+
+		/* fdd.buf may have changed 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.offset;
+
+		process(tool, fdd.buf, NULL, NULL);
+	}
+	free(fdd.buf);
+	return 0;
+}
+
+int perf_event__process_feature(struct perf_tool *tool,
+				union perf_event *event,
+				struct perf_session *session __maybe_unused)
+{
+	struct feat_fd fd = { .fd = 0 };
+	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;
+	}
+	feat = rec2feat[type];
+	if (feat == HEADER_RESERVED)
+		return -1;
+
+	if (feat > HEADER_LAST_FEATURE)
+		return 0;
+
+	if (!feat_ops[feat].process)
+		return 0;
+
+	/*
+	 * no print routine
+	 */
+	if (!feat_ops[feat].print)
+		return 0;
+
+	fd.buf  = (void *)event;
+	fd.buf += sizeof(event->header);
+	fd.size = event->header.size - sizeof(event->header);
+	fd.ph = &session->header;
+
+	if (!feat_ops[feat].full_only || tool->show_full_info) {
+		if (feat_ops[feat].process) {
+			if (feat_ops[feat].process(&fd, NULL))
+				return -1;
+		}
+		feat_ops[feat].print(&fd, stdout);
+	} else {
+		fprintf(stdout, "# %s info available, use -I to display\n",
+			feat_ops[feat].name);
+	}
+
+	return 0;
+}
+
 static struct event_update_event *
 event_update_event__new(size_t size, u64 type, u64 id)
 {
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 86ab723cbee4..fbe1a4690627 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -102,6 +102,15 @@ int perf_header__process_sections(struct perf_header *header, int fd,
 
 int perf_header__fprintf_info(struct perf_session *s, FILE *fp, bool full);
 
+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, u32 ids, u64 *id,
 				perf_event__handler_t process);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 3041c6b98191..ef788b91239e 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -256,6 +256,14 @@ static int process_event_stub(struct perf_tool *tool __maybe_unused,
 	return 0;
 }
 
+static int process_feature_stub(struct perf_tool *tool __maybe_unused,
+				union perf_event *event __maybe_unused,
+				struct perf_session *session __maybe_unused)
+{
+	dump_printf(": unhandled!\n");
+	return 0;
+}
+
 static int process_finished_round_stub(struct perf_tool *tool __maybe_unused,
 				       union perf_event *event __maybe_unused,
 				       struct ordered_events *oe __maybe_unused)
@@ -427,6 +435,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 		tool->stat_round = process_stat_round_stub;
 	if (tool->time_conv == NULL)
 		tool->time_conv = process_event_op2_stub;
+	if (tool->feature == NULL)
+		tool->feature = process_feature_stub;
 }
 
 static void swap_sample_id_all(union perf_event *event, void *data)
@@ -1370,6 +1380,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
 	case PERF_RECORD_TIME_CONV:
 		session->time_conv = event->time_conv;
 		return tool->time_conv(tool, event, session);
+	case PERF_RECORD_HEADER_HOSTNAME ... PERF_RECORD_HEADER_PMU_MAPPINGS:
+		return tool->feature(tool, event, session);
 	default:
 		return -EINVAL;
 	}
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 795b2be7b51f..70cb36e6b098 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -63,7 +63,8 @@ struct perf_tool {
 			cpu_map,
 			stat_config,
 			stat,
-			stat_round;
+			stat_round,
+			feature;
 	event_op3	auxtrace;
 	bool		ordered_events;
 	bool		ordering_requires_timestamps;
-- 
2.13.0.303.g4ebf302169-goog

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:
> Add header record types to pipe-mode, reusing the functions
> used in file-mode and leveraging the new struct feat_fd.
> 
> Add the perf_event__synthesize_feature event call back to
> process the new header records.
> 
> Before this patch:
> 
>   $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.000 MB - ]
>   ...
> 
> After this patch:
>   $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.000 MB - ]
>   # hostname : lphh7
>   # os release : 4.11.0-dbx-up_perf
>   # perf version : 4.11.rc6.g6277c80
>   # arch : x86_64
>   # nrcpus online : 72
>   # nrcpus avail : 72
>   # cpudesc : Intel(R) Xeon(R) CPU E5-2696 v3 @ 2.30GHz
>   # cpuid : GenuineIntel,6,63,2
>   # total memory : 263457192 kB
>   # cmdline : /root/perf record -o - -e cycles -c 100000 sleep 1
>   # HEADER_CPU_TOPOLOGY info available, use -I to display
>   # HEADER_NUMA_TOPOLOGY info available, use -I to display
>   # pmu mappings: intel_bts = 6, uncore_imc_4 = 22, uncore_sbox_1 = 47, uncore_cbox_5 = 33, uncore_ha_0 = 16, uncore_cbox
>    Percent |      Source code & Disassembly of kcore for cycles (9 samples)
>   ...

thanks a lot for doing this, comments comming shortly

jirka

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[David Carrillo-Cisneros]

On Thu, May 18, 2017 at 9:12 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:
>> Add header record types to pipe-mode, reusing the functions
>> used in file-mode and leveraging the new struct feat_fd.
>>
>> Add the perf_event__synthesize_feature event call back to
>> process the new header records.
>>
>> Before this patch:
>>
>>   $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
>>   [ perf record: Woken up 1 times to write data ]
>>   [ perf record: Captured and wrote 0.000 MB - ]
>>   ...
>>
>> After this patch:
>>   $ perf record -o - -e cycles -c 100000 sleep 1 | perf report --stdio
>>   [ perf record: Woken up 1 times to write data ]
>>   [ perf record: Captured and wrote 0.000 MB - ]
>>   # hostname : lphh7
>>   # os release : 4.11.0-dbx-up_perf
>>   # perf version : 4.11.rc6.g6277c80
>>   # arch : x86_64
>>   # nrcpus online : 72
>>   # nrcpus avail : 72
>>   # cpudesc : Intel(R) Xeon(R) CPU E5-2696 v3 @ 2.30GHz
>>   # cpuid : GenuineIntel,6,63,2
>>   # total memory : 263457192 kB
>>   # cmdline : /root/perf record -o - -e cycles -c 100000 sleep 1
>>   # HEADER_CPU_TOPOLOGY info available, use -I to display
>>   # HEADER_NUMA_TOPOLOGY info available, use -I to display
>>   # pmu mappings: intel_bts = 6, uncore_imc_4 = 22, uncore_sbox_1 = 47, uncore_cbox_5 = 33, uncore_ha_0 = 16, uncore_cbox
>>    Percent |      Source code & Disassembly of kcore for cycles (9 samples)
>>   ...
>
> thanks a lot for doing this, comments comming shortly
>
thanks a lot for the review. I'll do the suggested changes and send a
2nd version.

David

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:

SNIP

>  struct header_print_data {
> @@ -2218,33 +2258,33 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
>  	return 0;
>  }
>  
> -static int do_write_feat(struct feat_fd *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->ph, type)) {

please do these changes in the earlier patch that introduced
the feat_fd as parameter in do_write_feat function

thanks,
jirka

>  		if (!feat_ops[type].write)
>  			return -1;
>  
> -		if (fd->buf) {
> -			pr_err("do_write_feat to memory buffer\n");
> +		if (fdd->buf) {
> +			pr_debug("Called do_write_feat for memory buffer\n");
>  			return -1;
>  		}
> -		(*p)->offset = lseek(fd->fd, 0, SEEK_CUR);
> +		(*p)->offset = lseek(fdd->fd, 0, SEEK_CUR);
>  
> -		err = feat_ops[type].write(fd, evlist);
> +		err = feat_ops[type].write(fdd, evlist);
>  		if (err < 0) {
>  			pr_debug("failed to write feature %s\n", feat_ops[type].name);
>  
>  			/* undo anything written */
> -			lseek(fd->fd, (*p)->offset, SEEK_SET);
> +			lseek(fdd->fd, (*p)->offset, SEEK_SET);
>  
>  			return -1;
>  		}
> -		(*p)->size = lseek(fd->fd, 0, SEEK_CUR) - (*p)->offset;
> +		(*p)->size = lseek(fdd->fd, 0, SEEK_CUR) - (*p)->offset;
>  		(*p)++;
>  	}
>  	return ret;
> @@ -2261,10 +2301,6 @@ static int perf_header__adds_write(struct perf_header *header,
>  	int feat;
>  	int err;
>  
> -	/*
> -	 * may write more than needed due to dropped feature, but
> -	 * this is okay, reader will skip the mising entries
> -	 */
>  	fdd = (struct feat_fd){
>  		.fd  = fd,
>  		.ph = header,
> @@ -2284,7 +2320,7 @@ 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(&fdd, header, feat, &p, evlist))
> +		if (do_write_feat(&fdd, feat, &p, evlist))
>  			perf_header__clear_feat(header, feat);

SNIP

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:

SNIP

>  static const char *perf_ns__names[] = {
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index db2de6413518..d404f50260f8 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -244,6 +244,19 @@ enum perf_user_event_type { /* above any possible kernel type */
>  	PERF_RECORD_STAT_ROUND			= 77,
>  	PERF_RECORD_EVENT_UPDATE		= 78,
>  	PERF_RECORD_TIME_CONV			= 79,
> +	PERF_RECORD_HEADER_HOSTNAME		= 80,
> +	PERF_RECORD_HEADER_OSRELEASE		= 81,
> +	PERF_RECORD_HEADER_VERSION		= 82,
> +	PERF_RECORD_HEADER_ARCH			= 83,
> +	PERF_RECORD_HEADER_NRCPUS		= 84,
> +	PERF_RECORD_HEADER_CPUDESC		= 85,
> +	PERF_RECORD_HEADER_CPUID		= 86,
> +	PERF_RECORD_HEADER_TOTAL_MEM		= 87,
> +	PERF_RECORD_HEADER_CMDLINE		= 88,
> +	PERF_RECORD_HEADER_EVENT_DESC		= 89,
> +	PERF_RECORD_HEADER_CPU_TOPOLOGY		= 90,
> +	PERF_RECORD_HEADER_NUMA_TOPOLOGY	= 91,
> +	PERF_RECORD_HEADER_PMU_MAPPINGS		= 92,
>  	PERF_RECORD_HEADER_MAX
>  };

SNIP

> +
> +/*
> + * 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),
> +	REC2FEAT(PMU_MAPPINGS),
>  };

hum, how about instead of adding all those new events we add
just one: PERF_RECORD_FEATURE, which would carry the id of the
feature, like:

struct feature_event {
	struct perf_event_header	header;
	u64	id;
	char	data[]; /* size bytes of raw data specific to the feature */
};


this way we could also omit the reverse event to feature map

thanks,
jirka

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[David Carrillo-Cisneros]

On Thu, May 18, 2017 at 9:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:
>
> SNIP
>
>>  static const char *perf_ns__names[] = {
>> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
>> index db2de6413518..d404f50260f8 100644
>> --- a/tools/perf/util/event.h
>> +++ b/tools/perf/util/event.h
>> @@ -244,6 +244,19 @@ enum perf_user_event_type { /* above any possible kernel type */
>>       PERF_RECORD_STAT_ROUND                  = 77,
>>       PERF_RECORD_EVENT_UPDATE                = 78,
>>       PERF_RECORD_TIME_CONV                   = 79,
>> +     PERF_RECORD_HEADER_HOSTNAME             = 80,
>> +     PERF_RECORD_HEADER_OSRELEASE            = 81,
>> +     PERF_RECORD_HEADER_VERSION              = 82,
>> +     PERF_RECORD_HEADER_ARCH                 = 83,
>> +     PERF_RECORD_HEADER_NRCPUS               = 84,
>> +     PERF_RECORD_HEADER_CPUDESC              = 85,
>> +     PERF_RECORD_HEADER_CPUID                = 86,
>> +     PERF_RECORD_HEADER_TOTAL_MEM            = 87,
>> +     PERF_RECORD_HEADER_CMDLINE              = 88,
>> +     PERF_RECORD_HEADER_EVENT_DESC           = 89,
>> +     PERF_RECORD_HEADER_CPU_TOPOLOGY         = 90,
>> +     PERF_RECORD_HEADER_NUMA_TOPOLOGY        = 91,
>> +     PERF_RECORD_HEADER_PMU_MAPPINGS         = 92,
>>       PERF_RECORD_HEADER_MAX
>>  };
>
> SNIP
>
>> +
>> +/*
>> + * 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),
>> +     REC2FEAT(PMU_MAPPINGS),
>>  };
>
> hum, how about instead of adding all those new events we add
> just one: PERF_RECORD_FEATURE, which would carry the id of the
> feature, like:
>
> struct feature_event {
>         struct perf_event_header        header;
>         u64     id;
>         char    data[]; /* size bytes of raw data specific to the feature */
> };
>
>
> this way we could also omit the reverse event to feature map

I think that'd work. I'll try that.

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:

SNIP

> -		(*p)->offset = lseek(fd->fd, 0, SEEK_CUR);
> +		(*p)->offset = lseek(fdd->fd, 0, SEEK_CUR);
>  
> -		err = feat_ops[type].write(fd, evlist);
> +		err = feat_ops[type].write(fdd, evlist);
>  		if (err < 0) {
>  			pr_debug("failed to write feature %s\n", feat_ops[type].name);
>  
>  			/* undo anything written */
> -			lseek(fd->fd, (*p)->offset, SEEK_SET);
> +			lseek(fdd->fd, (*p)->offset, SEEK_SET);
>  
>  			return -1;
>  		}
> -		(*p)->size = lseek(fd->fd, 0, SEEK_CUR) - (*p)->offset;
> +		(*p)->size = lseek(fdd->fd, 0, SEEK_CUR) - (*p)->offset;
>  		(*p)++;
>  	}
>  	return ret;
> @@ -2261,10 +2301,6 @@ static int perf_header__adds_write(struct perf_header *header,
>  	int feat;
>  	int err;
>  
> -	/*
> -	 * may write more than needed due to dropped feature, but
> -	 * this is okay, reader will skip the mising entries
> -	 */

you added this comment in earlier patch, if you don't want it,
remove it in the original patch

jirka

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[Jiri Olsa]

On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:

SNIP

>  
> @@ -2107,42 +2109,80 @@ struct feature_ops {
>  	int (*process)(struct feat_fd *fd, void *data);
>  	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, \
> -		.process = process_##func, .full_only = true }
> +#define FEAT_OPP(n, func, __full_only) \
> +	[HEADER_##n] = {					\
> +		.name = __stringify(HEADER_##n),		\
> +		.write = write_##func,				\
> +		.print = print_##func,				\
> +		.full_only = __full_only,			\
> +		.process = process_##func,			\
> +		.record_type = PERF_RECORD_HEADER_##n		\
> +	}
> +
> +#define FEAT_OPN(n, func, __full_only) \
> +	[HEADER_##n] = {					\
> +		.name = __stringify(HEADER_##n),		\
> +		.write = write_##func,				\
> +		.print = print_##func,				\
> +		.full_only = __full_only,			\
> +		.process = process_##func			\
> +	}

you're adding record_type, which should not be a reason
to rename FEAT_* macros.. please do that in the separate
patch and state reason in changelog

thanks,
jirka

Re: [PATCH 7/7] perf tools: add feature header record to pipe-mode

[David Carrillo-Cisneros]

On Thu, May 18, 2017 at 9:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, May 17, 2017 at 09:16:02PM -0700, David Carrillo-Cisneros wrote:
>
> SNIP
>
>>
>> @@ -2107,42 +2109,80 @@ struct feature_ops {
>>       int (*process)(struct feat_fd *fd, void *data);
>>       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, \
>> -             .process = process_##func, .full_only = true }
>> +#define FEAT_OPP(n, func, __full_only) \
>> +     [HEADER_##n] = {                                        \
>> +             .name = __stringify(HEADER_##n),                \
>> +             .write = write_##func,                          \
>> +             .print = print_##func,                          \
>> +             .full_only = __full_only,                       \
>> +             .process = process_##func,                      \
>> +             .record_type = PERF_RECORD_HEADER_##n           \
>> +     }
>> +
>> +#define FEAT_OPN(n, func, __full_only) \
>> +     [HEADER_##n] = {                                        \
>> +             .name = __stringify(HEADER_##n),                \
>> +             .write = write_##func,                          \
>> +             .print = print_##func,                          \
>> +             .full_only = __full_only,                       \
>> +             .process = process_##func                       \
>> +     }
>
> you're adding record_type, which should not be a reason
> to rename FEAT_* macros.. please do that in the separate
> patch and state reason in changelog

will do

Re: [PATCH 0/7] perf tool: add meta-data header support for pipe-mode

[Andi Kleen]

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

The new headers need documentation in Documentation/perf.data-file-format.txt

-Andi

Re: [PATCH 0/7] perf tool: add meta-data header support for pipe-mode

[David Carrillo-Cisneros]

On Wed, May 17, 2017 at 10:10 PM, Andi Kleen <ak@linux.intel.com> wrote:
>> 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.
>
> The new headers need documentation in Documentation/perf.data-file-format.txt
Will do.