linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode
@ 2017-07-11 23:52 David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 01/16] perf header: encapsulate read and swap David Carrillo-Cisneros
                   ` (12 more replies)
  0 siblings, 13 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

v5: - Fix buffer leaking and size changes in
      perf_event__synthesize_features.
    - Remove unnecessary renaming.
    - Remove extra tabs in do_write_string.
v4: - Limit write_* functions to page_size.
    - Fixed bugs spotted by Jiri.
    - Add information about pipe-mode to some error messages. 
v3: - Fix header output for event aux record.
    - Uniformize variable naming and other cleanup.
    - Other fixes suggested by Jiri and Nahmyung.

v2: - Finer patch splitting.
    - Add only one record type with a feature id instead of one
      record per new feature (as suggested by Jiri).
    - Add perf.data documentation.

(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 series, it is possible to get:
  $ perf record -o - -e cycles sleep 1 | perf report --stdio --header
  # ========
  # captured on: Mon May 22 16:33:43 2017
  # ========
  #
  # hostname : my_hostname
  # 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 -e cycles sleep 1 
  # event : name = cycles, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1
  # CPU_TOPOLOGY info available, use -I to display
  # NUMA_TOPOLOGY info available, use -I to display
  # pmu mappings: intel_bts = 6, cpu = 4, msr = 49, uncore_cbox_10 = 36, [SNIP]
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  ...

Only patches 14 to 16 have a significant effect in user observed behavior.
All other are transparent preparatory changes or bug fixes.

David Carrillo-Cisneros (16):
  perf header: encapsulate read and swap
  perf header: add PROCESS_STR_FUN macro
  perf header: fail on write_padded error
  perf util: add const modifier to buf in "writen" function
  perf header: revamp do_write
  perf header: add struct feat_fd for write
  perf header: use struct feat_fd for print
  perf header: use struct feat_fd to process header records
  perf header: don't pass struct perf_file_section to process_##_feat
  perf header: use struct feat_fd in read header records
  perf header: make write_pmu_mappings pipe-mode friendly
  perf header: add a buffer to struct feat_fd
  perf header: change FEAT_OP* macros
  perf tool: add show_feature_header to perf_tool
  perf tools: add feature header record to pipe-mode
  perf header: add event desc to pipe-mode header

 tools/perf/Documentation/perf.data-file-format.txt |   10 +-
 tools/perf/builtin-annotate.c                      |    1 +
 tools/perf/builtin-inject.c                        |    1 +
 tools/perf/builtin-record.c                        |    7 +
 tools/perf/builtin-report.c                        |    5 +
 tools/perf/builtin-script.c                        |    4 +
 tools/perf/util/build-id.c                         |   10 +-
 tools/perf/util/build-id.h                         |    4 +-
 tools/perf/util/event.c                            |    1 +
 tools/perf/util/event.h                            |    8 +
 tools/perf/util/header.c                           | 1012 +++++++++++---------
 tools/perf/util/header.h                           |   16 +-
 tools/perf/util/session.c                          |    4 +
 tools/perf/util/tool.h                             |   10 +-
 tools/perf/util/util.c                             |    6 +-
 tools/perf/util/util.h                             |    2 +-
 16 files changed, 633 insertions(+), 468 deletions(-)

-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 01/16] perf header: encapsulate read and swap
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 02/16] perf header: add PROCESS_STR_FUN macro David Carrillo-Cisneros
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

Most callers of readn in perf header read either a 32 or a 64 bits
number, error check it and swap it, if necessary.

Create do_read_u32 and do_read_u64 to simplify these use cases.

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 5cac8d5e009a..5492cd8d51a8 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -118,25 +118,54 @@ static int do_write_string(int fd, const char *str)
 	return write_padded(fd, str, olen, len);
 }
 
+static int __do_read(int fd, void *addr, ssize_t size)
+{
+	ssize_t ret = readn(fd, addr, size);
+
+	if (ret != size)
+		return ret < 0 ? (int)ret : -1;
+	return 0;
+}
+
+static int do_read_u32(int fd, struct perf_header *ph, u32 *addr)
+{
+	int ret;
+
+	ret = __do_read(fd, addr, sizeof(*addr));
+	if (ret)
+		return ret;
+
+	if (ph->needs_swap)
+		*addr = bswap_32(*addr);
+	return 0;
+}
+
+static int do_read_u64(int fd, struct perf_header *ph, u64 *addr)
+{
+	int ret;
+
+	ret = __do_read(fd, addr, sizeof(*addr));
+	if (ret)
+		return ret;
+
+	if (ph->needs_swap)
+		*addr = bswap_64(*addr);
+	return 0;
+}
+
 static char *do_read_string(int fd, struct perf_header *ph)
 {
-	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, ph, &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
 		 * thus the actual strlen of buf
@@ -1187,24 +1216,15 @@ read_event_desc(struct perf_header *ph, int fd)
 	u64 *id;
 	void *buf = NULL;
 	u32 nre, sz, nr, i, j;
-	ssize_t ret;
 	size_t msz;
 
 	/* number of events */
-	ret = readn(fd, &nre, sizeof(nre));
-	if (ret != (ssize_t)sizeof(nre))
+	if (do_read_u32(fd, ph, &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, ph, &sz))
 		goto error;
 
-	if (ph->needs_swap)
-		sz = bswap_32(sz);
-
 	/* buffer to hold on file attr struct */
 	buf = malloc(sz);
 	if (!buf)
@@ -1226,8 +1246,7 @@ 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);
-		if (ret != (ssize_t)sz)
+		if (__do_read(fd, buf, sz))
 			goto error;
 
 		if (ph->needs_swap)
@@ -1235,16 +1254,15 @@ read_event_desc(struct perf_header *ph, int fd)
 
 		memcpy(&evsel->attr, buf, msz);
 
-		ret = readn(fd, &nr, sizeof(nr));
-		if (ret != (ssize_t)sizeof(nr))
+		if (do_read_u32(fd, ph, &nr))
 			goto error;
 
-		if (ph->needs_swap) {
-			nr = bswap_32(nr);
+		if (ph->needs_swap)
 			evsel->needs_swap = true;
-		}
 
 		evsel->name = do_read_string(fd, ph);
+		if (!evsel->name)
+			goto error;
 
 		if (!nr)
 			continue;
@@ -1256,11 +1274,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, ph, id))
 				goto error;
-			if (ph->needs_swap)
-				*id = bswap_64(*id);
 			id++;
 		}
 	}
@@ -1640,26 +1655,18 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 			  struct perf_header *ph, int 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, ph, &nr_cpus_avail);
+	if (ret)
+		return ret;
 
-	ph->env.nr_cpus_online = nr;
+	ret = do_read_u32(fd, ph, &nr_cpus_online);
+	if (ret)
+		return ret;
+	ph->env.nr_cpus_avail = (int)nr_cpus_avail;
+	ph->env.nr_cpus_online = (int)nr_cpus_online;
 	return 0;
 }
 
@@ -1683,17 +1690,13 @@ static int process_total_mem(struct perf_file_section *section __maybe_unused,
 			     struct perf_header *ph, int 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, ph, &total_mem);
+	if (ret)
 		return -1;
-
-	if (ph->needs_swap)
-		mem = bswap_64(mem);
-
-	ph->env.total_mem = mem;
+	ph->env.total_mem = (unsigned long long)total_mem;
 	return 0;
 }
 
@@ -1753,17 +1756,12 @@ static int process_cmdline(struct perf_file_section *section,
 			   struct perf_header *ph, int 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, ph, &nr))
 		return -1;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
 	ph->env.nr_cmdline = nr;
 
 	cmdline = zalloc(section->size + nr + 1);
@@ -1798,7 +1796,6 @@ static int process_cpu_topology(struct perf_file_section *section,
 				struct perf_header *ph, int fd,
 				void *data __maybe_unused)
 {
-	ssize_t ret;
 	u32 nr, i;
 	char *str;
 	struct strbuf sb;
@@ -1809,13 +1806,9 @@ static int process_cpu_topology(struct perf_file_section *section,
 	if (!ph->env.cpu)
 		return -1;
 
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
+	if (do_read_u32(fd, ph, &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)
@@ -1834,13 +1827,9 @@ static int process_cpu_topology(struct perf_file_section *section,
 	}
 	ph->env.sibling_cores = strbuf_detach(&sb, NULL);
 
-	ret = readn(fd, &nr, sizeof(nr));
-	if (ret != sizeof(nr))
+	if (do_read_u32(fd, ph, &nr))
 		return -1;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
 	ph->env.nr_sibling_threads = nr;
 	size += sizeof(u32);
 
@@ -1867,22 +1856,14 @@ static int process_cpu_topology(struct perf_file_section *section,
 	}
 
 	for (i = 0; i < (u32)cpu_nr; i++) {
-		ret = readn(fd, &nr, sizeof(nr));
-		if (ret != sizeof(nr))
+		if (do_read_u32(fd, ph, &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, ph, &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");
@@ -1906,18 +1887,13 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 				 void *data __maybe_unused)
 {
 	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, ph, &nr))
 		return -1;
 
-	if (ph->needs_swap)
-		nr = bswap_32(nr);
-
 	nodes = zalloc(sizeof(*nodes) * nr);
 	if (!nodes)
 		return -ENOMEM;
@@ -1926,24 +1902,15 @@ 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, ph, &n->node))
 			goto error;
 
-		ret = readn(fd, &n->mem_total, sizeof(u64));
-		if (ret != sizeof(u64))
+		if (do_read_u64(fd, ph, &n->mem_total))
 			goto error;
 
-		ret = readn(fd, &n->mem_free, sizeof(u64));
-		if (ret != sizeof(u64))
+		if (do_read_u64(fd, ph, &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);
 		if (!str)
 			goto error;
@@ -1967,19 +1934,14 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused
 				struct perf_header *ph, int fd,
 				void *data __maybe_unused)
 {
-	ssize_t ret;
 	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, ph, &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;
@@ -1990,10 +1952,8 @@ 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, ph, &type))
 			goto error;
-		if (ph->needs_swap)
-			type = bswap_32(type);
 
 		name = do_read_string(fd, ph);
 		if (!name)
@@ -2033,12 +1993,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, ph, &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");
@@ -2054,16 +2011,11 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 		if (!desc[i].name)
 			goto out_free;
 
-		if (readn(fd, &desc[i].leader_idx, sizeof(u32)) != sizeof(u32))
+		if (do_read_u32(fd, ph, &desc[i].leader_idx))
 			goto out_free;
 
-		if (readn(fd, &desc[i].nr_members, sizeof(u32)) != sizeof(u32))
+		if (do_read_u32(fd, ph, &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);
-		}
 	}
 
 	/*
@@ -2136,21 +2088,15 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 	struct cpu_cache_level *caches;
 	u32 cnt, i, version;
 
-	if (readn(fd, &version, sizeof(version)) != sizeof(version))
+	if (do_read_u32(fd, ph, &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, ph, &cnt))
 		return -1;
 
-	if (ph->needs_swap)
-		cnt = bswap_32(cnt);
-
 	caches = zalloc(sizeof(*caches) * cnt);
 	if (!caches)
 		return -1;
@@ -2159,10 +2105,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, ph, &c.v))\
 				goto out_free_caches;			\
-			if (ph->needs_swap)				\
-				c.v = bswap_32(c.v);			\
 
 		_R(level)
 		_R(line_size)
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 02/16] perf header: add PROCESS_STR_FUN macro
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 01/16] perf header: encapsulate read and swap David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 03/16] perf header: fail on write_padded error David Carrillo-Cisneros
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

Simplify code by adding a macro to handle the common case
of processing header features that are a simple string.

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 5492cd8d51a8..a1b16da4b34d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1602,6 +1602,23 @@ static int perf_header__read_build_ids(struct perf_header *header,
 	return err;
 }
 
+/* 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 perf_file_section *section __maybe_unused, \
+			    struct perf_header *ph, int fd,		      \
+			    void *data __maybe_unused) \
+{\
+	ph->env.__feat_env = do_read_string(fd, ph); \
+	return ph->env.__feat_env ? 0 : -ENOMEM; \
+}
+
+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_tracing_data(struct perf_file_section *section __maybe_unused,
 				struct perf_header *ph __maybe_unused,
 				int fd, void *data)
@@ -1619,38 +1636,6 @@ static int process_build_id(struct perf_file_section *section,
 	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)
-{
-	ph->env.version = do_read_string(fd, ph);
-	return ph->env.version ? 0 : -ENOMEM;
-}
-
-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;
-}
-
 static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 			  struct perf_header *ph, int fd,
 			  void *data __maybe_unused)
@@ -1670,22 +1655,6 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 	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)
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 03/16] perf header: fail on write_padded error
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 01/16] perf header: encapsulate read and swap David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 02/16] perf header: add PROCESS_STR_FUN macro David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 04/16] perf util: add const modifier to buf in "writen" function David Carrillo-Cisneros
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

Do not proceed if write_padded error failed.

Also, add comments to remind that the return value of write_*
functions in util/header.c is an erro code and not the 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 a1b16da4b34d..68dcc70ca4ca 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -73,6 +73,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) {
@@ -88,6 +89,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];
@@ -102,6 +104,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;
@@ -3199,7 +3202,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.2.725.g09c95d1e9-goog

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

* [PATCH v5 04/16] perf util: add const modifier to buf in "writen" function
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (2 preceding siblings ...)
  2017-07-11 23:52 ` [PATCH v5 03/16] perf header: fail on write_padded error David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 05/16] perf header: revamp do_write David Carrillo-Cisneros
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, 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 28c9f335006c..0d8d7b8b2949 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -259,6 +259,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);
 
@@ -286,9 +287,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 5dfb9bb6482d..fc6555e48422 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -49,7 +49,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);
 
 size_t hex_width(u64 v);
 int hex2u64(const char *ptr, u64 *val);
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 05/16] perf header: revamp do_write
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (3 preceding siblings ...)
  2017-07-11 23:52 ` [PATCH v5 04/16] perf util: add const modifier to buf in "writen" function David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 06/16] perf header: add struct feat_fd for write David Carrillo-Cisneros
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

Now that writen takes a const buffer, use it in do_write instead
of duplicating its functionality.

Export do_write to use it consistently in header.c and
build_id.c .

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

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 168cc49654e7..84d2ea51e557 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -345,7 +345,7 @@ 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;
 
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 68dcc70ca4ca..c50d8c471215 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -74,17 +74,13 @@ 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(int fd, const void *buf, size_t size)
 {
-	while (size) {
-		int ret = write(fd, buf, size);
-
-		if (ret < 0)
-			return -errno;
+	ssize_t ret;
 
-		size -= ret;
-		buf += ret;
-	}
+	ret  = writen(fd, buf, size);
+	if (ret != (ssize_t)size)
+		return ret < 0 ? (int)ret : -1;
 
 	return 0;
 }
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index d30109b421ee..e98489c8bba7 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -144,6 +144,8 @@ bool is_perf_magic(u64 magic);
 
 #define NAME_ALIGN 64
 
+int do_write(int fd, const void *buf, size_t size);
+
 int write_padded(int fd, const void *bf, size_t count, size_t count_aligned);
 
 /*
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 06/16] perf header: add struct feat_fd for write
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (4 preceding siblings ...)
  2017-07-11 23:52 ` [PATCH v5 05/16] perf header: revamp do_write David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 07/16] perf header: use struct feat_fd for print David Carrillo-Cisneros
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

Introduce struct feat_fd. This patch uses it as a wrapper
around fd in write_* functions for feature headers. Next
patches will extend its functionality to other feature
header functions.

This patch does not change behavior.

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

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 84d2ea51e557..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;
@@ -352,7 +352,8 @@ static int write_buildid(const char *name, size_t name_len, u8 *build_id,
 	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 8a89b195c1fc..1f5121e8cc6b 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 c50d8c471215..5d2ff9ed053c 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -58,6 +58,11 @@ struct perf_file_attr {
 	struct perf_file_section	ids;
 };
 
+struct feat_fd {
+	struct perf_header	*ph;
+	int			fd;
+};
+
 void perf_header__set_feat(struct perf_header *header, int feat)
 {
 	set_bit(feat, header->adds_features);
@@ -74,11 +79,11 @@ bool perf_header__has_feat(const struct perf_header *header, int feat)
 }
 
 /* Return: 0 if succeded, -ERR if failed. */
-int do_write(int fd, const void *buf, size_t size)
+int do_write(struct feat_fd *ff, const void *buf, size_t size)
 {
 	ssize_t ret;
 
-	ret  = writen(fd, buf, size);
+	ret  = writen(ff->fd, buf, size);
 	if (ret != (ssize_t)size)
 		return ret < 0 ? (int)ret : -1;
 
@@ -86,13 +91,14 @@ int do_write(int fd, const void *buf, size_t size)
 }
 
 /* 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 *ff, const void *bf,
+		 size_t count, size_t count_aligned)
 {
 	static const char zero_buf[NAME_ALIGN];
-	int err = do_write(fd, bf, count);
+	int err = do_write(ff, bf, count);
 
 	if (!err)
-		err = do_write(fd, zero_buf, count_aligned - count);
+		err = do_write(ff, zero_buf, count_aligned - count);
 
 	return err;
 }
@@ -101,7 +107,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 *ff, const char *str)
 {
 	u32 len, olen;
 	int ret;
@@ -110,11 +116,11 @@ static int do_write_string(int fd, const char *str)
 	len = PERF_ALIGN(olen, NAME_ALIGN);
 
 	/* write len, incl. \0 */
-	ret = do_write(fd, &len, sizeof(len));
+	ret = do_write(ff, &len, sizeof(len));
 	if (ret < 0)
 		return ret;
 
-	return write_padded(fd, str, olen, len);
+	return write_padded(ff, str, olen, len);
 }
 
 static int __do_read(int fd, void *addr, ssize_t size)
@@ -177,25 +183,24 @@ 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 *ff,
+			      struct perf_evlist *evlist)
 {
-	return read_tracing_data(fd, &evlist->entries);
+	return read_tracing_data(ff->fd, &evlist->entries);
 }
 
-
-static int write_build_id(int fd, struct perf_header *h,
+static int write_build_id(struct feat_fd *ff,
 			  struct perf_evlist *evlist __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(h, struct perf_session, header);
+	session = container_of(ff->ph, struct perf_session, header);
 
 	if (!perf_session__read_build_ids(session, true))
 		return -1;
 
-	err = perf_session__write_buildid_table(session, fd);
+	err = perf_session__write_buildid_table(session, ff);
 	if (err < 0) {
 		pr_debug("failed to write buildid table\n");
 		return err;
@@ -205,7 +210,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 *ff,
 			  struct perf_evlist *evlist __maybe_unused)
 {
 	struct utsname uts;
@@ -215,10 +220,10 @@ static int write_hostname(int fd, struct perf_header *h __maybe_unused,
 	if (ret < 0)
 		return -1;
 
-	return do_write_string(fd, uts.nodename);
+	return do_write_string(ff, uts.nodename);
 }
 
-static int write_osrelease(int fd, struct perf_header *h __maybe_unused,
+static int write_osrelease(struct feat_fd *ff,
 			   struct perf_evlist *evlist __maybe_unused)
 {
 	struct utsname uts;
@@ -228,10 +233,10 @@ static int write_osrelease(int fd, struct perf_header *h __maybe_unused,
 	if (ret < 0)
 		return -1;
 
-	return do_write_string(fd, uts.release);
+	return do_write_string(ff, uts.release);
 }
 
-static int write_arch(int fd, struct perf_header *h __maybe_unused,
+static int write_arch(struct feat_fd *ff,
 		      struct perf_evlist *evlist __maybe_unused)
 {
 	struct utsname uts;
@@ -241,16 +246,16 @@ static int write_arch(int fd, struct perf_header *h __maybe_unused,
 	if (ret < 0)
 		return -1;
 
-	return do_write_string(fd, uts.machine);
+	return do_write_string(ff, uts.machine);
 }
 
-static int write_version(int fd, struct perf_header *h __maybe_unused,
+static int write_version(struct feat_fd *ff,
 			 struct perf_evlist *evlist __maybe_unused)
 {
-	return do_write_string(fd, perf_version_string);
+	return do_write_string(ff, perf_version_string);
 }
 
-static int __write_cpudesc(int fd, const char *cpuinfo_proc)
+static int __write_cpudesc(struct feat_fd *ff, const char *cpuinfo_proc)
 {
 	FILE *file;
 	char *buf = NULL;
@@ -300,14 +305,14 @@ static int __write_cpudesc(int fd, const char *cpuinfo_proc)
 		}
 		p++;
 	}
-	ret = do_write_string(fd, s);
+	ret = do_write_string(ff, s);
 done:
 	free(buf);
 	fclose(file);
 	return ret;
 }
 
-static int write_cpudesc(int fd, struct perf_header *h __maybe_unused,
+static int write_cpudesc(struct feat_fd *ff,
 		       struct perf_evlist *evlist __maybe_unused)
 {
 #ifndef CPUINFO_PROC
@@ -318,7 +323,7 @@ static int write_cpudesc(int fd, struct perf_header *h __maybe_unused,
 
 	for (i = 0; i < ARRAY_SIZE(cpuinfo_procs); i++) {
 		int ret;
-		ret = __write_cpudesc(fd, cpuinfo_procs[i]);
+		ret = __write_cpudesc(ff, cpuinfo_procs[i]);
 		if (ret >= 0)
 			return ret;
 	}
@@ -326,7 +331,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 *ff,
 			struct perf_evlist *evlist __maybe_unused)
 {
 	long nr;
@@ -341,14 +346,14 @@ static int write_nrcpus(int fd, struct perf_header *h __maybe_unused,
 
 	nra = (u32)(nr & UINT_MAX);
 
-	ret = do_write(fd, &nrc, sizeof(nrc));
+	ret = do_write(ff, &nrc, sizeof(nrc));
 	if (ret < 0)
 		return ret;
 
-	return do_write(fd, &nra, sizeof(nra));
+	return do_write(ff, &nra, sizeof(nra));
 }
 
-static int write_event_desc(int fd, struct perf_header *h __maybe_unused,
+static int write_event_desc(struct feat_fd *ff,
 			    struct perf_evlist *evlist)
 {
 	struct perf_evsel *evsel;
@@ -360,7 +365,7 @@ static int write_event_desc(int fd, struct perf_header *h __maybe_unused,
 	/*
 	 * write number of events
 	 */
-	ret = do_write(fd, &nre, sizeof(nre));
+	ret = do_write(ff, &nre, sizeof(nre));
 	if (ret < 0)
 		return ret;
 
@@ -368,12 +373,12 @@ static int write_event_desc(int fd, struct perf_header *h __maybe_unused,
 	 * size of perf_event_attr struct
 	 */
 	sz = (u32)sizeof(evsel->attr);
-	ret = do_write(fd, &sz, sizeof(sz));
+	ret = do_write(ff, &sz, sizeof(sz));
 	if (ret < 0)
 		return ret;
 
 	evlist__for_each_entry(evlist, evsel) {
-		ret = do_write(fd, &evsel->attr, sz);
+		ret = do_write(ff, &evsel->attr, sz);
 		if (ret < 0)
 			return ret;
 		/*
@@ -384,27 +389,27 @@ static int write_event_desc(int fd, struct perf_header *h __maybe_unused,
 		 * type of ids,
 		 */
 		nri = evsel->ids;
-		ret = do_write(fd, &nri, sizeof(nri));
+		ret = do_write(ff, &nri, sizeof(nri));
 		if (ret < 0)
 			return ret;
 
 		/*
 		 * write event string as passed on cmdline
 		 */
-		ret = do_write_string(fd, perf_evsel__name(evsel));
+		ret = do_write_string(ff, perf_evsel__name(evsel));
 		if (ret < 0)
 			return ret;
 		/*
 		 * write unique ids for this event
 		 */
-		ret = do_write(fd, evsel->id, evsel->ids * sizeof(u64));
+		ret = do_write(ff, evsel->id, evsel->ids * sizeof(u64));
 		if (ret < 0)
 			return ret;
 	}
 	return 0;
 }
 
-static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
+static int write_cmdline(struct feat_fd *ff,
 			 struct perf_evlist *evlist __maybe_unused)
 {
 	char buf[MAXPATHLEN];
@@ -422,16 +427,16 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	/* account for binary path */
 	n = perf_env.nr_cmdline + 1;
 
-	ret = do_write(fd, &n, sizeof(n));
+	ret = do_write(ff, &n, sizeof(n));
 	if (ret < 0)
 		return ret;
 
-	ret = do_write_string(fd, buf);
+	ret = do_write_string(ff, buf);
 	if (ret < 0)
 		return ret;
 
 	for (i = 0 ; i < perf_env.nr_cmdline; i++) {
-		ret = do_write_string(fd, perf_env.cmdline_argv[i]);
+		ret = do_write_string(ff, perf_env.cmdline_argv[i]);
 		if (ret < 0)
 			return ret;
 	}
@@ -584,8 +589,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 *ff,
+			      struct perf_evlist *evlist __maybe_unused)
 {
 	struct cpu_topo *tp;
 	u32 i;
@@ -595,21 +600,21 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 	if (!tp)
 		return -1;
 
-	ret = do_write(fd, &tp->core_sib, sizeof(tp->core_sib));
+	ret = do_write(ff, &tp->core_sib, sizeof(tp->core_sib));
 	if (ret < 0)
 		goto done;
 
 	for (i = 0; i < tp->core_sib; i++) {
-		ret = do_write_string(fd, tp->core_siblings[i]);
+		ret = do_write_string(ff, tp->core_siblings[i]);
 		if (ret < 0)
 			goto done;
 	}
-	ret = do_write(fd, &tp->thread_sib, sizeof(tp->thread_sib));
+	ret = do_write(ff, &tp->thread_sib, sizeof(tp->thread_sib));
 	if (ret < 0)
 		goto done;
 
 	for (i = 0; i < tp->thread_sib; i++) {
-		ret = do_write_string(fd, tp->thread_siblings[i]);
+		ret = do_write_string(ff, tp->thread_siblings[i]);
 		if (ret < 0)
 			break;
 	}
@@ -619,11 +624,11 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 		goto done;
 
 	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
-		ret = do_write(fd, &perf_env.cpu[j].core_id,
+		ret = do_write(ff, &perf_env.cpu[j].core_id,
 			       sizeof(perf_env.cpu[j].core_id));
 		if (ret < 0)
 			return ret;
-		ret = do_write(fd, &perf_env.cpu[j].socket_id,
+		ret = do_write(ff, &perf_env.cpu[j].socket_id,
 			       sizeof(perf_env.cpu[j].socket_id));
 		if (ret < 0)
 			return ret;
@@ -635,8 +640,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 *ff,
+			   struct perf_evlist *evlist __maybe_unused)
 {
 	char *buf = NULL;
 	FILE *fp;
@@ -656,7 +661,7 @@ static int write_total_mem(int fd, struct perf_header *h __maybe_unused,
 	if (!ret) {
 		n = sscanf(buf, "%*s %"PRIu64, &mem);
 		if (n == 1)
-			ret = do_write(fd, &mem, sizeof(mem));
+			ret = do_write(ff, &mem, sizeof(mem));
 	} else
 		ret = -1;
 	free(buf);
@@ -664,7 +669,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 *ff, int node)
 {
 	char str[MAXPATHLEN];
 	char field[32];
@@ -694,11 +699,11 @@ static int write_topo_node(int fd, int node)
 	fclose(fp);
 	fp = NULL;
 
-	ret = do_write(fd, &mem_total, sizeof(u64));
+	ret = do_write(ff, &mem_total, sizeof(u64));
 	if (ret)
 		goto done;
 
-	ret = do_write(fd, &mem_free, sizeof(u64));
+	ret = do_write(ff, &mem_free, sizeof(u64));
 	if (ret)
 		goto done;
 
@@ -716,7 +721,7 @@ static int write_topo_node(int fd, int node)
 	if (p)
 		*p = '\0';
 
-	ret = do_write_string(fd, buf);
+	ret = do_write_string(ff, buf);
 done:
 	free(buf);
 	if (fp)
@@ -724,8 +729,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 *ff,
+			       struct perf_evlist *evlist __maybe_unused)
 {
 	char *buf = NULL;
 	size_t len = 0;
@@ -752,17 +757,17 @@ static int write_numa_topology(int fd, struct perf_header *h __maybe_unused,
 
 	nr = (u32)node_map->nr;
 
-	ret = do_write(fd, &nr, sizeof(nr));
+	ret = do_write(ff, &nr, sizeof(nr));
 	if (ret < 0)
 		goto done;
 
 	for (i = 0; i < nr; i++) {
 		j = (u32)node_map->map[i];
-		ret = do_write(fd, &j, sizeof(j));
+		ret = do_write(ff, &j, sizeof(j));
 		if (ret < 0)
 			break;
 
-		ret = write_topo_node(fd, i);
+		ret = write_topo_node(ff, i);
 		if (ret < 0)
 			break;
 	}
@@ -785,16 +790,16 @@ 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 *ff,
 			      struct perf_evlist *evlist __maybe_unused)
 {
 	struct perf_pmu *pmu = NULL;
-	off_t offset = lseek(fd, 0, SEEK_CUR);
+	off_t offset = lseek(ff->fd, 0, SEEK_CUR);
 	__u32 pmu_num = 0;
 	int ret;
 
 	/* write real pmu_num later */
-	ret = do_write(fd, &pmu_num, sizeof(pmu_num));
+	ret = do_write(ff, &pmu_num, sizeof(pmu_num));
 	if (ret < 0)
 		return ret;
 
@@ -803,18 +808,18 @@ static int write_pmu_mappings(int fd, struct perf_header *h __maybe_unused,
 			continue;
 		pmu_num++;
 
-		ret = do_write(fd, &pmu->type, sizeof(pmu->type));
+		ret = do_write(ff, &pmu->type, sizeof(pmu->type));
 		if (ret < 0)
 			return ret;
 
-		ret = do_write_string(fd, pmu->name);
+		ret = do_write_string(ff, pmu->name);
 		if (ret < 0)
 			return ret;
 	}
 
-	if (pwrite(fd, &pmu_num, sizeof(pmu_num), offset) != sizeof(pmu_num)) {
+	if (pwrite(ff->fd, &pmu_num, sizeof(pmu_num), offset) != sizeof(pmu_num)) {
 		/* discard all */
-		lseek(fd, offset, SEEK_SET);
+		lseek(ff->fd, offset, SEEK_SET);
 		return -1;
 	}
 
@@ -833,14 +838,14 @@ 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 *ff,
 			    struct perf_evlist *evlist)
 {
 	u32 nr_groups = evlist->nr_groups;
 	struct perf_evsel *evsel;
 	int ret;
 
-	ret = do_write(fd, &nr_groups, sizeof(nr_groups));
+	ret = do_write(ff, &nr_groups, sizeof(nr_groups));
 	if (ret < 0)
 		return ret;
 
@@ -851,15 +856,15 @@ static int write_group_desc(int fd, struct perf_header *h __maybe_unused,
 			u32 leader_idx = evsel->idx;
 			u32 nr_members = evsel->nr_members;
 
-			ret = do_write_string(fd, name);
+			ret = do_write_string(ff, name);
 			if (ret < 0)
 				return ret;
 
-			ret = do_write(fd, &leader_idx, sizeof(leader_idx));
+			ret = do_write(ff, &leader_idx, sizeof(leader_idx));
 			if (ret < 0)
 				return ret;
 
-			ret = do_write(fd, &nr_members, sizeof(nr_members));
+			ret = do_write(ff, &nr_members, sizeof(nr_members));
 			if (ret < 0)
 				return ret;
 		}
@@ -876,7 +881,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 *ff,
 		       struct perf_evlist *evlist __maybe_unused)
 {
 	char buffer[64];
@@ -888,25 +893,24 @@ static int write_cpuid(int fd, struct perf_header *h __maybe_unused,
 
 	return -1;
 write_it:
-	return do_write_string(fd, buffer);
+	return do_write_string(ff, 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 *ff __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 *ff,
 			  struct perf_evlist *evlist __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(h, struct perf_session, header);
+	session = container_of(ff->ph, struct perf_session, header);
 
-	err = auxtrace_index__write(fd, &session->auxtrace_index);
+	err = auxtrace_index__write(ff->fd, &session->auxtrace_index);
 	if (err < 0)
 		pr_err("Failed to write auxtrace index\n");
 	return err;
@@ -1053,8 +1057,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 *ff,
+		       struct perf_evlist *evlist __maybe_unused)
 {
 	struct cpu_cache_level caches[MAX_CACHES];
 	u32 cnt = 0, i, version = 1;
@@ -1066,11 +1070,11 @@ static int write_cache(int fd, struct perf_header *h __maybe_unused,
 
 	qsort(&caches, cnt, sizeof(struct cpu_cache_level), cpu_cache_level__sort);
 
-	ret = do_write(fd, &version, sizeof(u32));
+	ret = do_write(ff, &version, sizeof(u32));
 	if (ret < 0)
 		goto out;
 
-	ret = do_write(fd, &cnt, sizeof(u32));
+	ret = do_write(ff, &cnt, sizeof(u32));
 	if (ret < 0)
 		goto out;
 
@@ -1078,7 +1082,7 @@ static int write_cache(int fd, struct perf_header *h __maybe_unused,
 		struct cpu_cache_level *c = &caches[i];
 
 		#define _W(v)					\
-			ret = do_write(fd, &c->v, sizeof(u32));	\
+			ret = do_write(ff, &c->v, sizeof(u32));	\
 			if (ret < 0)				\
 				goto out;
 
@@ -1089,7 +1093,7 @@ static int write_cache(int fd, struct perf_header *h __maybe_unused,
 		#undef _W
 
 		#define _W(v)						\
-			ret = do_write_string(fd, (const char *) c->v);	\
+			ret = do_write_string(ff, (const char *) c->v);	\
 			if (ret < 0)					\
 				goto out;
 
@@ -1105,8 +1109,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 *ff __maybe_unused,
 		      struct perf_evlist *evlist __maybe_unused)
 {
 	return 0;
@@ -2104,7 +2107,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 *ff, 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);
@@ -2213,29 +2216,29 @@ 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 *ff, 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(ff->ph, type)) {
 		if (!feat_ops[type].write)
 			return -1;
 
-		(*p)->offset = lseek(fd, 0, SEEK_CUR);
+		(*p)->offset = lseek(ff->fd, 0, SEEK_CUR);
 
-		err = feat_ops[type].write(fd, h, evlist);
+		err = feat_ops[type].write(ff, 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(ff->fd, (*p)->offset, SEEK_SET);
 
 			return -1;
 		}
-		(*p)->size = lseek(fd, 0, SEEK_CUR) - (*p)->offset;
+		(*p)->size = lseek(ff->fd, 0, SEEK_CUR) - (*p)->offset;
 		(*p)++;
 	}
 	return ret;
@@ -2245,12 +2248,18 @@ static int perf_header__adds_write(struct perf_header *header,
 				   struct perf_evlist *evlist, int fd)
 {
 	int nr_sections;
+	struct feat_fd ff;
 	struct perf_file_section *feat_sec, *p;
 	int sec_size;
 	u64 sec_start;
 	int feat;
 	int err;
 
+	ff = (struct feat_fd){
+		.fd  = fd,
+		.ph = header,
+	};
+
 	nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS);
 	if (!nr_sections)
 		return 0;
@@ -2265,7 +2274,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(&ff, feat, &p, evlist))
 			perf_header__clear_feat(header, feat);
 	}
 
@@ -2274,7 +2283,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(&ff, feat_sec, sec_size);
 	if (err < 0)
 		pr_debug("failed to write feature section\n");
 	free(feat_sec);
@@ -2284,14 +2293,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 ff;
 	int err;
 
+	ff = (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(&ff, &f_header, sizeof(f_header));
 	if (err < 0) {
 		pr_debug("failed to write perf pipe header\n");
 		return err;
@@ -2308,21 +2320,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 ff;
 	u64 attr_offset;
 	int err;
 
+	ff = (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(&ff, 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(ff.fd, 0, SEEK_CUR);
 
 	evlist__for_each_entry(evlist, evsel) {
 		f_attr = (struct perf_file_attr){
@@ -2332,7 +2346,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(&ff, &f_attr, sizeof(f_attr));
 		if (err < 0) {
 			pr_debug("failed to write perf header attribute\n");
 			return err;
@@ -2367,7 +2381,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(&ff, &f_header, sizeof(f_header));
 	if (err < 0) {
 		pr_debug("failed to write perf header\n");
 		return err;
@@ -2642,6 +2656,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 ff = {
+		.fd = STDOUT_FILENO,
+		.ph = ph,
+	};
 	ssize_t ret;
 
 	ret = readn(fd, header, sizeof(*header));
@@ -2656,7 +2674,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(&ff, header, sizeof(*header)) < 0)
 		return -1;
 
 	return 0;
@@ -3164,6 +3182,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 ff;
 	int err __maybe_unused = 0;
 
 	/*
@@ -3198,7 +3217,8 @@ int perf_event__synthesize_tracing_data(struct perf_tool *tool, int fd,
 	 */
 	tracing_data_put(tdata);
 
-	if (write_padded(fd, NULL, 0, padding))
+	ff = (struct feat_fd){ .fd = fd };
+	if (write_padded(&ff, NULL, 0, padding))
 		return -1;
 
 	return aligned_size;
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index e98489c8bba7..9d8dcd5eb727 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -144,9 +144,12 @@ bool is_perf_magic(u64 magic);
 
 #define NAME_ALIGN 64
 
-int do_write(int fd, const void *buf, size_t size);
+struct feat_fd;
 
-int write_padded(int fd, const void *bf, size_t count, size_t count_aligned);
+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.2.725.g09c95d1e9-goog

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

* [PATCH v5 07/16] perf header: use struct feat_fd for print
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (5 preceding siblings ...)
  2017-07-11 23:52 ` [PATCH v5 06/16] perf header: add struct feat_fd for write David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:52 ` [PATCH v5 08/16] perf header: use struct feat_fd to process header records David Carrillo-Cisneros
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

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

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 5d2ff9ed053c..de9409c14346 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1115,62 +1115,56 @@ static int write_stat(struct feat_fd *ff __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 *ff, FILE *fp)
 {
-	fprintf(fp, "# hostname : %s\n", ph->env.hostname);
+	fprintf(fp, "# hostname : %s\n", ff->ph->env.hostname);
 }
 
-static void print_osrelease(struct perf_header *ph, int fd __maybe_unused,
-			    FILE *fp)
+static void print_osrelease(struct feat_fd *ff, FILE *fp)
 {
-	fprintf(fp, "# os release : %s\n", ph->env.os_release);
+	fprintf(fp, "# os release : %s\n", ff->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 *ff, FILE *fp)
 {
-	fprintf(fp, "# arch : %s\n", ph->env.arch);
+	fprintf(fp, "# arch : %s\n", ff->ph->env.arch);
 }
 
-static void print_cpudesc(struct perf_header *ph, int fd __maybe_unused,
-			  FILE *fp)
+static void print_cpudesc(struct feat_fd *ff, FILE *fp)
 {
-	fprintf(fp, "# cpudesc : %s\n", ph->env.cpu_desc);
+	fprintf(fp, "# cpudesc : %s\n", ff->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 *ff, 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", ff->ph->env.nr_cpus_online);
+	fprintf(fp, "# nrcpus avail : %u\n", ff->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 *ff, FILE *fp)
 {
-	fprintf(fp, "# perf version : %s\n", ph->env.version);
+	fprintf(fp, "# perf version : %s\n", ff->ph->env.version);
 }
 
-static void print_cmdline(struct perf_header *ph, int fd __maybe_unused,
-			  FILE *fp)
+static void print_cmdline(struct feat_fd *ff, FILE *fp)
 {
 	int nr, i;
 
-	nr = ph->env.nr_cmdline;
+	nr = ff->ph->env.nr_cmdline;
 
 	fprintf(fp, "# cmdline : ");
 
 	for (i = 0; i < nr; i++)
-		fprintf(fp, "%s ", ph->env.cmdline_argv[i]);
+		fprintf(fp, "%s ", ff->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 *ff, FILE *fp)
 {
+	struct perf_header *ph = ff->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;
@@ -1296,9 +1290,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 *ff, FILE *fp)
 {
-	struct perf_evsel *evsel, *events = read_event_desc(ph, fd);
+	struct perf_evsel *evsel, *events = read_event_desc(ff->ph, ff->fd);
 	u32 j;
 	u64 *id;
 
@@ -1328,20 +1322,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 *ff, FILE *fp)
 {
-	fprintf(fp, "# total memory : %Lu kB\n", ph->env.total_mem);
+	fprintf(fp, "# total memory : %llu kB\n", ff->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 *ff, 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 < ff->ph->env.nr_numa_nodes; i++) {
+		n = &ff->ph->env.numa_nodes[i];
 
 		fprintf(fp, "# node%u meminfo  : total = %"PRIu64" kB,"
 			    " free = %"PRIu64" kB\n",
@@ -1352,56 +1344,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 *ff, FILE *fp)
 {
-	fprintf(fp, "# cpuid : %s\n", ph->env.cpuid);
+	fprintf(fp, "# cpuid : %s\n", ff->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 *ff __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 *ff __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 *ff __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 *ff, 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 < ff->ph->env.caches_cnt; i++) {
 		fprintf(fp, "#  ");
-		cpu_cache_level__fprintf(fp, &ph->env.caches[i]);
+		cpu_cache_level__fprintf(fp, &ff->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 *ff, FILE *fp)
 {
 	const char *delimiter = "# pmu mappings: ";
 	char *str, *tmp;
 	u32 pmu_num;
 	u32 type;
 
-	pmu_num = ph->env.nr_pmu_mappings;
+	pmu_num = ff->ph->env.nr_pmu_mappings;
 	if (!pmu_num) {
 		fprintf(fp, "# pmu mappings: not available\n");
 		return;
 	}
 
-	str = ph->env.pmu_mappings;
+	str = ff->ph->env.pmu_mappings;
 
 	while (pmu_num) {
 		type = strtoul(str, &tmp, 0);
@@ -1424,14 +1411,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 *ff, FILE *fp)
 {
 	struct perf_session *session;
 	struct perf_evsel *evsel;
 	u32 nr = 0;
 
-	session = container_of(ph, struct perf_session, header);
+	session = container_of(ff->ph, struct perf_session, header);
 
 	evlist__for_each_entry(session->evlist, evsel) {
 		if (perf_evsel__is_group_leader(evsel) &&
@@ -2108,7 +2094,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 
 struct feature_ops {
 	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
-	void (*print)(struct perf_header *h, int fd, FILE *fp);
+	void (*print)(struct feat_fd *ff, FILE *fp);
 	int (*process)(struct perf_file_section *section,
 		       struct perf_header *h, int fd, void *data);
 	const char *name;
@@ -2161,6 +2147,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 ff;
 
 	if (lseek(fd, section->offset, SEEK_SET) == (off_t)-1) {
 		pr_debug("Failed to lseek to %" PRIu64 " offset for feature "
@@ -2174,8 +2161,13 @@ static int perf_file_section__fprintf_info(struct perf_file_section *section,
 	if (!feat_ops[feat].print)
 		return 0;
 
+	ff = (struct  feat_fd) {
+		.fd = fd,
+		.ph = ph,
+	};
+
 	if (!feat_ops[feat].full_only || hd->full)
-		feat_ops[feat].print(ph, fd, hd->fp);
+		feat_ops[feat].print(&ff, hd->fp);
 	else
 		fprintf(hd->fp, "# %s info available, use -I to display\n",
 			feat_ops[feat].name);
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 08/16] perf header: use struct feat_fd to process header records
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (6 preceding siblings ...)
  2017-07-11 23:52 ` [PATCH v5 07/16] perf header: use struct feat_fd for print David Carrillo-Cisneros
@ 2017-07-11 23:52 ` David Carrillo-Cisneros
  2017-07-11 23:53 ` [PATCH v5 09/16] perf header: don't pass struct perf_file_section to process_##_feat David Carrillo-Cisneros
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

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

This patch does not change behavior.

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index de9409c14346..afa87fb323d5 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1593,11 +1593,10 @@ static int perf_header__read_build_ids(struct perf_header *header,
 /* 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 perf_file_section *section __maybe_unused, \
-			    struct perf_header *ph, int fd,		      \
-			    void *data __maybe_unused) \
+			    struct feat_fd *ff, void *data __maybe_unused) \
 {\
-	ph->env.__feat_env = do_read_string(fd, ph); \
-	return ph->env.__feat_env ? 0 : -ENOMEM; \
+	ff->ph->env.__feat_env = do_read_string(ff->fd, ff->ph); \
+	return ff->ph->env.__feat_env ? 0 : -ENOMEM; \
 }
 
 FEAT_PROCESS_STR_FUN(hostname, hostname);
@@ -1608,52 +1607,49 @@ FEAT_PROCESS_STR_FUN(cpudesc, cpu_desc);
 FEAT_PROCESS_STR_FUN(cpuid, cpuid);
 
 static int process_tracing_data(struct perf_file_section *section __maybe_unused,
-				struct perf_header *ph __maybe_unused,
-				int fd, void *data)
+				struct feat_fd *ff, void *data)
 {
-	ssize_t ret = trace_report(fd, data, false);
+	ssize_t ret = trace_report(ff->fd, data, false);
+
 	return ret < 0 ? -1 : 0;
 }
 
 static int process_build_id(struct perf_file_section *section,
-			    struct perf_header *ph, int fd,
-			    void *data __maybe_unused)
+			    struct feat_fd *ff, void *data __maybe_unused)
 {
-	if (perf_header__read_build_ids(ph, fd, section->offset, section->size))
+	if (perf_header__read_build_ids(ff->ph, ff->fd, section->offset, section->size))
 		pr_debug("Failed to read buildids, continuing...\n");
 	return 0;
 }
 
 static int process_nrcpus(struct perf_file_section *section __maybe_unused,
-			  struct perf_header *ph, int fd,
-			  void *data __maybe_unused)
+			  struct feat_fd *ff, void *data __maybe_unused)
 {
 	int ret;
 	u32 nr_cpus_avail, nr_cpus_online;
 
-	ret = do_read_u32(fd, ph, &nr_cpus_avail);
+	ret = do_read_u32(ff->fd, ff->ph, &nr_cpus_avail);
 	if (ret)
 		return ret;
 
-	ret = do_read_u32(fd, ph, &nr_cpus_online);
+	ret = do_read_u32(ff->fd, ff->ph, &nr_cpus_online);
 	if (ret)
 		return ret;
-	ph->env.nr_cpus_avail = (int)nr_cpus_avail;
-	ph->env.nr_cpus_online = (int)nr_cpus_online;
+	ff->ph->env.nr_cpus_avail = (int)nr_cpus_avail;
+	ff->ph->env.nr_cpus_online = (int)nr_cpus_online;
 	return 0;
 }
 
 static int process_total_mem(struct perf_file_section *section __maybe_unused,
-			     struct perf_header *ph, int fd,
-			     void *data __maybe_unused)
+			     struct feat_fd *ff, void *data __maybe_unused)
 {
 	u64 total_mem;
 	int ret;
 
-	ret = do_read_u64(fd, ph, &total_mem);
+	ret = do_read_u64(ff->fd, ff->ph, &total_mem);
 	if (ret)
 		return -1;
-	ph->env.total_mem = (unsigned long long)total_mem;
+	ff->ph->env.total_mem = (unsigned long long)total_mem;
 	return 0;
 }
 
@@ -1691,16 +1687,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)
+		   struct feat_fd *ff, 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(ff->ph, ff->fd);
 
 	if (!events)
 		return 0;
 
-	session = container_of(header, struct perf_session, header);
+	session = container_of(ff->ph, struct perf_session, header);
 	for (evsel = events; evsel->attr.size; evsel++)
 		perf_evlist__set_event_name(session->evlist, evsel);
 
@@ -1709,17 +1704,16 @@ 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 perf_file_section *section __maybe_unused,
+			   struct feat_fd *ff, void *data __maybe_unused)
 {
 	char *str, *cmdline = NULL, **argv = NULL;
 	u32 nr, i, len = 0;
 
-	if (do_read_u32(fd, ph, &nr))
+	if (do_read_u32(ff->fd, ff->ph, &nr))
 		return -1;
 
-	ph->env.nr_cmdline = nr;
+	ff->ph->env.nr_cmdline = nr;
 
 	cmdline = zalloc(section->size + nr + 1);
 	if (!cmdline)
@@ -1730,7 +1724,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(ff->fd, ff->ph);
 		if (!str)
 			goto error;
 
@@ -1739,8 +1733,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;
+	ff->ph->env.cmdline = cmdline;
+	ff->ph->env.cmdline_argv = (const char **) argv;
 	return 0;
 
 error:
@@ -1749,21 +1743,21 @@ 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 perf_file_section *section __maybe_unused,
+				struct feat_fd *ff, void *data __maybe_unused)
 {
 	u32 nr, i;
 	char *str;
 	struct strbuf sb;
-	int cpu_nr = ph->env.nr_cpus_avail;
+	int cpu_nr = ff->ph->env.nr_cpus_avail;
 	u64 size = 0;
+	struct perf_header *ph = ff->ph;
 
 	ph->env.cpu = calloc(cpu_nr, sizeof(*ph->env.cpu));
 	if (!ph->env.cpu)
 		return -1;
 
-	if (do_read_u32(fd, ph, &nr))
+	if (do_read_u32(ff->fd, ph, &nr))
 		goto free_cpu;
 
 	ph->env.nr_sibling_cores = nr;
@@ -1772,7 +1766,7 @@ static int process_cpu_topology(struct perf_file_section *section,
 		goto free_cpu;
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(fd, ph);
+		str = do_read_string(ff->fd, ph);
 		if (!str)
 			goto error;
 
@@ -1784,14 +1778,14 @@ static int process_cpu_topology(struct perf_file_section *section,
 	}
 	ph->env.sibling_cores = strbuf_detach(&sb, NULL);
 
-	if (do_read_u32(fd, ph, &nr))
+	if (do_read_u32(ff->fd, ph, &nr))
 		return -1;
 
 	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(ff->fd, ph);
 		if (!str)
 			goto error;
 
@@ -1813,12 +1807,12 @@ static int process_cpu_topology(struct perf_file_section *section,
 	}
 
 	for (i = 0; i < (u32)cpu_nr; i++) {
-		if (do_read_u32(fd, ph, &nr))
+		if (do_read_u32(ff->fd, ph, &nr))
 			goto free_cpu;
 
 		ph->env.cpu[i].core_id = nr;
 
-		if (do_read_u32(fd, ph, &nr))
+		if (do_read_u32(ff->fd, ph, &nr))
 			goto free_cpu;
 
 		if (nr != (u32)-1 && nr > (u32)cpu_nr) {
@@ -1840,15 +1834,14 @@ static int process_cpu_topology(struct perf_file_section *section,
 }
 
 static int process_numa_topology(struct perf_file_section *section __maybe_unused,
-				 struct perf_header *ph, int fd,
-				 void *data __maybe_unused)
+				 struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct numa_node *nodes, *n;
 	u32 nr, i;
 	char *str;
 
 	/* nr nodes */
-	if (do_read_u32(fd, ph, &nr))
+	if (do_read_u32(ff->fd, ff->ph, &nr))
 		return -1;
 
 	nodes = zalloc(sizeof(*nodes) * nr);
@@ -1859,16 +1852,16 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 		n = &nodes[i];
 
 		/* node number */
-		if (do_read_u32(fd, ph, &n->node))
+		if (do_read_u32(ff->fd, ff->ph, &n->node))
 			goto error;
 
-		if (do_read_u64(fd, ph, &n->mem_total))
+		if (do_read_u64(ff->fd, ff->ph, &n->mem_total))
 			goto error;
 
-		if (do_read_u64(fd, ph, &n->mem_free))
+		if (do_read_u64(ff->fd, ff->ph, &n->mem_free))
 			goto error;
 
-		str = do_read_string(fd, ph);
+		str = do_read_string(ff->fd, ff->ph);
 		if (!str)
 			goto error;
 
@@ -1878,8 +1871,8 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 
 		free(str);
 	}
-	ph->env.nr_numa_nodes = nr;
-	ph->env.numa_nodes = nodes;
+	ff->ph->env.nr_numa_nodes = nr;
+	ff->ph->env.numa_nodes = nodes;
 	return 0;
 
 error:
@@ -1888,15 +1881,14 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse
 }
 
 static int process_pmu_mappings(struct perf_file_section *section __maybe_unused,
-				struct perf_header *ph, int fd,
-				void *data __maybe_unused)
+				struct feat_fd *ff, void *data __maybe_unused)
 {
 	char *name;
 	u32 pmu_num;
 	u32 type;
 	struct strbuf sb;
 
-	if (do_read_u32(fd, ph, &pmu_num))
+	if (do_read_u32(ff->fd, ff->ph, &pmu_num))
 		return -1;
 
 	if (!pmu_num) {
@@ -1904,15 +1896,15 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused
 		return 0;
 	}
 
-	ph->env.nr_pmu_mappings = pmu_num;
+	ff->ph->env.nr_pmu_mappings = pmu_num;
 	if (strbuf_init(&sb, 128) < 0)
 		return -1;
 
 	while (pmu_num) {
-		if (do_read_u32(fd, ph, &type))
+		if (do_read_u32(ff->fd, ff->ph, &type))
 			goto error;
 
-		name = do_read_string(fd, ph);
+		name = do_read_string(ff->fd, ff->ph);
 		if (!name)
 			goto error;
 
@@ -1923,12 +1915,12 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused
 			goto error;
 
 		if (!strcmp(name, "msr"))
-			ph->env.msr_pmu_type = type;
+			ff->ph->env.msr_pmu_type = type;
 
 		free(name);
 		pmu_num--;
 	}
-	ph->env.pmu_mappings = strbuf_detach(&sb, NULL);
+	ff->ph->env.pmu_mappings = strbuf_detach(&sb, NULL);
 	return 0;
 
 error:
@@ -1937,8 +1929,7 @@ static int process_pmu_mappings(struct perf_file_section *section __maybe_unused
 }
 
 static int process_group_desc(struct perf_file_section *section __maybe_unused,
-			      struct perf_header *ph, int fd,
-			      void *data __maybe_unused)
+			      struct feat_fd *ff, void *data __maybe_unused)
 {
 	size_t ret = -1;
 	u32 i, nr, nr_groups;
@@ -1950,10 +1941,10 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 		u32 nr_members;
 	} *desc;
 
-	if (do_read_u32(fd, ph, &nr_groups))
+	if (do_read_u32(ff->fd, ff->ph, &nr_groups))
 		return -1;
 
-	ph->env.nr_groups = nr_groups;
+	ff->ph->env.nr_groups = nr_groups;
 	if (!nr_groups) {
 		pr_debug("group desc not available\n");
 		return 0;
@@ -1964,21 +1955,21 @@ 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(ff->fd, ff->ph);
 		if (!desc[i].name)
 			goto out_free;
 
-		if (do_read_u32(fd, ph, &desc[i].leader_idx))
+		if (do_read_u32(ff->fd, ff->ph, &desc[i].leader_idx))
 			goto out_free;
 
-		if (do_read_u32(fd, ph, &desc[i].nr_members))
+		if (do_read_u32(ff->fd, ff->ph, &desc[i].nr_members))
 			goto out_free;
 	}
 
 	/*
 	 * Rebuild group relationship based on the group_desc
 	 */
-	session = container_of(ph, struct perf_session, header);
+	session = container_of(ff->ph, struct perf_session, header);
 	session->evlist->nr_groups = nr_groups;
 
 	i = nr = 0;
@@ -2022,36 +2013,34 @@ 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 perf_file_section *section __maybe_unused,
+			    struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
-	session = container_of(ph, struct perf_session, header);
+	session = container_of(ff->ph, struct perf_session, header);
 
-	err = auxtrace_index__process(fd, section->size, session,
-				      ph->needs_swap);
+	err = auxtrace_index__process(ff->fd, section->size, session,
+				      ff->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)
+			 struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct cpu_cache_level *caches;
 	u32 cnt, i, version;
 
-	if (do_read_u32(fd, ph, &version))
+	if (do_read_u32(ff->fd, ff->ph, &version))
 		return -1;
 
 	if (version != 1)
 		return -1;
 
-	if (do_read_u32(fd, ph, &cnt))
+	if (do_read_u32(ff->fd, ff->ph, &cnt))
 		return -1;
 
 	caches = zalloc(sizeof(*caches) * cnt);
@@ -2062,7 +2051,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 		struct cpu_cache_level c;
 
 		#define _R(v)						\
-			if (do_read_u32(fd, ph, &c.v))\
+			if (do_read_u32(ff->fd, ff->ph, &c.v))\
 				goto out_free_caches;			\
 
 		_R(level)
@@ -2071,9 +2060,9 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 		_R(ways)
 		#undef _R
 
-		#define _R(v)				\
-			c.v = do_read_string(fd, ph);	\
-			if (!c.v)			\
+		#define _R(v)					\
+			c.v = do_read_string(ff->fd, ff->ph);	\
+			if (!c.v)				\
 				goto out_free_caches;
 
 		_R(type)
@@ -2084,8 +2073,8 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 		caches[i] = c;
 	}
 
-	ph->env.caches = caches;
-	ph->env.caches_cnt = cnt;
+	ff->ph->env.caches = caches;
+	ff->ph->env.caches_cnt = cnt;
 	return 0;
 out_free_caches:
 	free(caches);
@@ -2096,7 +2085,7 @@ struct feature_ops {
 	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
 	void (*print)(struct feat_fd *ff, FILE *fp);
 	int (*process)(struct perf_file_section *section,
-		       struct perf_header *h, int fd, void *data);
+		       struct feat_fd *ff, void *data);
 	const char *name;
 	bool full_only;
 };
@@ -2627,6 +2616,11 @@ static int perf_file_section__process(struct perf_file_section *section,
 				      struct perf_header *ph,
 				      int feat, int fd, void *data)
 {
+	struct feat_fd ff = {
+		.fd	= fd,
+		.ph	= ph,
+	};
+
 	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);
@@ -2641,7 +2635,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(section, &ff, data);
 }
 
 static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 09/16] perf header: don't pass struct perf_file_section to process_##_feat
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (7 preceding siblings ...)
  2017-07-11 23:52 ` [PATCH v5 08/16] perf header: use struct feat_fd to process header records David Carrillo-Cisneros
@ 2017-07-11 23:53 ` David Carrillo-Cisneros
  2017-07-11 23:53 ` [PATCH v5 10/16] perf header: use struct feat_fd in read header records David Carrillo-Cisneros
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

struct perf_file_section is used in process_##_feat as container for
size and offset in the file descriptor. These attributes are meaninful
in pipe-mode but struct perf_file_section is not.

Add offset and size variables to struct feat_fd to store
perf_file_section's values in file-mode. Later on, the same variables can
be reused for pipe-mode.

This patch does not change behavior.

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index afa87fb323d5..6552ad2748aa 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -61,6 +61,8 @@ struct perf_file_attr {
 struct feat_fd {
 	struct perf_header	*ph;
 	int			fd;
+	ssize_t			offset;
+	size_t			size;
 };
 
 void perf_header__set_feat(struct perf_header *header, int feat)
@@ -1592,8 +1594,7 @@ static int perf_header__read_build_ids(struct perf_header *header,
 
 /* 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 perf_file_section *section __maybe_unused, \
-			    struct feat_fd *ff, void *data __maybe_unused) \
+static int process_##__feat(struct feat_fd *ff, void *data __maybe_unused) \
 {\
 	ff->ph->env.__feat_env = do_read_string(ff->fd, ff->ph); \
 	return ff->ph->env.__feat_env ? 0 : -ENOMEM; \
@@ -1606,24 +1607,21 @@ FEAT_PROCESS_STR_FUN(arch, arch);
 FEAT_PROCESS_STR_FUN(cpudesc, cpu_desc);
 FEAT_PROCESS_STR_FUN(cpuid, cpuid);
 
-static int process_tracing_data(struct perf_file_section *section __maybe_unused,
-				struct feat_fd *ff, void *data)
+static int process_tracing_data(struct feat_fd *ff, void *data)
 {
 	ssize_t ret = trace_report(ff->fd, data, false);
 
 	return ret < 0 ? -1 : 0;
 }
 
-static int process_build_id(struct perf_file_section *section,
-			    struct feat_fd *ff, void *data __maybe_unused)
+static int process_build_id(struct feat_fd *ff, void *data __maybe_unused)
 {
-	if (perf_header__read_build_ids(ff->ph, ff->fd, section->offset, section->size))
+	if (perf_header__read_build_ids(ff->ph, ff->fd, ff->offset, ff->size))
 		pr_debug("Failed to read buildids, continuing...\n");
 	return 0;
 }
 
-static int process_nrcpus(struct perf_file_section *section __maybe_unused,
-			  struct feat_fd *ff, void *data __maybe_unused)
+static int process_nrcpus(struct feat_fd *ff, void *data __maybe_unused)
 {
 	int ret;
 	u32 nr_cpus_avail, nr_cpus_online;
@@ -1640,8 +1638,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 	return 0;
 }
 
-static int process_total_mem(struct perf_file_section *section __maybe_unused,
-			     struct feat_fd *ff, void *data __maybe_unused)
+static int process_total_mem(struct feat_fd *ff, void *data __maybe_unused)
 {
 	u64 total_mem;
 	int ret;
@@ -1686,8 +1683,7 @@ perf_evlist__set_event_name(struct perf_evlist *evlist,
 }
 
 static int
-process_event_desc(struct perf_file_section *section __maybe_unused,
-		   struct feat_fd *ff, void *data __maybe_unused)
+process_event_desc(struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct perf_session *session;
 	struct perf_evsel *evsel, *events = read_event_desc(ff->ph, ff->fd);
@@ -1704,8 +1700,7 @@ process_event_desc(struct perf_file_section *section __maybe_unused,
 	return 0;
 }
 
-static int process_cmdline(struct perf_file_section *section __maybe_unused,
-			   struct feat_fd *ff, void *data __maybe_unused)
+static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
 {
 	char *str, *cmdline = NULL, **argv = NULL;
 	u32 nr, i, len = 0;
@@ -1715,7 +1710,7 @@ static int process_cmdline(struct perf_file_section *section __maybe_unused,
 
 	ff->ph->env.nr_cmdline = nr;
 
-	cmdline = zalloc(section->size + nr + 1);
+	cmdline = zalloc(ff->size + nr + 1);
 	if (!cmdline)
 		return -1;
 
@@ -1743,8 +1738,7 @@ static int process_cmdline(struct perf_file_section *section __maybe_unused,
 	return -1;
 }
 
-static int process_cpu_topology(struct perf_file_section *section __maybe_unused,
-				struct feat_fd *ff, void *data __maybe_unused)
+static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 {
 	u32 nr, i;
 	char *str;
@@ -1801,7 +1795,7 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 	 * The header may be from old perf,
 	 * which doesn't include core id and socket id information.
 	 */
-	if (section->size <= size) {
+	if (ff->size <= size) {
 		zfree(&ph->env.cpu);
 		return 0;
 	}
@@ -1833,8 +1827,7 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 	return -1;
 }
 
-static int process_numa_topology(struct perf_file_section *section __maybe_unused,
-				 struct feat_fd *ff, void *data __maybe_unused)
+static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct numa_node *nodes, *n;
 	u32 nr, i;
@@ -1880,8 +1873,7 @@ 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 feat_fd *ff, void *data __maybe_unused)
+static int process_pmu_mappings(struct feat_fd *ff, void *data __maybe_unused)
 {
 	char *name;
 	u32 pmu_num;
@@ -1928,8 +1920,7 @@ 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 feat_fd *ff, void *data __maybe_unused)
+static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 {
 	size_t ret = -1;
 	u32 i, nr, nr_groups;
@@ -2013,23 +2004,21 @@ static int process_group_desc(struct perf_file_section *section __maybe_unused,
 	return ret;
 }
 
-static int process_auxtrace(struct perf_file_section *section __maybe_unused,
-			    struct feat_fd *ff, void *data __maybe_unused)
+static int process_auxtrace(struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct perf_session *session;
 	int err;
 
 	session = container_of(ff->ph, struct perf_session, header);
 
-	err = auxtrace_index__process(ff->fd, section->size, session,
+	err = auxtrace_index__process(ff->fd, ff->size, session,
 				      ff->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 feat_fd *ff, void *data __maybe_unused)
+static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct cpu_cache_level *caches;
 	u32 cnt, i, version;
@@ -2084,8 +2073,7 @@ static int process_cache(struct perf_file_section *section __maybe_unused,
 struct feature_ops {
 	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
 	void (*print)(struct feat_fd *ff, FILE *fp);
-	int (*process)(struct perf_file_section *section,
-		       struct feat_fd *ff, void *data);
+	int (*process)(struct feat_fd *ff, void *data);
 	const char *name;
 	bool full_only;
 };
@@ -2619,6 +2607,8 @@ static int perf_file_section__process(struct perf_file_section *section,
 	struct feat_fd ff = {
 		.fd	= fd,
 		.ph	= ph,
+		.size	= section->size,
+		.offset	= section->offset,
 	};
 
 	if (lseek(fd, section->offset, SEEK_SET) == (off_t)-1) {
@@ -2635,7 +2625,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, &ff, data);
+	return feat_ops[feat].process(&ff, data);
 }
 
 static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 10/16] perf header: use struct feat_fd in read header records
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (8 preceding siblings ...)
  2017-07-11 23:53 ` [PATCH v5 09/16] perf header: don't pass struct perf_file_section to process_##_feat David Carrillo-Cisneros
@ 2017-07-11 23:53 ` David Carrillo-Cisneros
  2017-07-11 23:53 ` [PATCH v5 11/16] perf header: make write_pmu_mappings pipe-mode friendly David Carrillo-Cisneros
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

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

This patch does not change behavior.

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 6552ad2748aa..e9f1d293d4ba 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -125,54 +125,54 @@ static int do_write_string(struct feat_fd *ff, const char *str)
 	return write_padded(ff, str, olen, len);
 }
 
-static int __do_read(int fd, void *addr, ssize_t size)
+static int __do_read(struct feat_fd *ff, void *addr, ssize_t size)
 {
-	ssize_t ret = readn(fd, addr, size);
+	ssize_t ret = readn(ff->fd, addr, size);
 
 	if (ret != size)
 		return ret < 0 ? (int)ret : -1;
 	return 0;
 }
 
-static int do_read_u32(int fd, struct perf_header *ph, u32 *addr)
+static int do_read_u32(struct feat_fd *ff, u32 *addr)
 {
 	int ret;
 
-	ret = __do_read(fd, addr, sizeof(*addr));
+	ret = __do_read(ff, addr, sizeof(*addr));
 	if (ret)
 		return ret;
 
-	if (ph->needs_swap)
+	if (ff->ph->needs_swap)
 		*addr = bswap_32(*addr);
 	return 0;
 }
 
-static int do_read_u64(int fd, struct perf_header *ph, u64 *addr)
+static int do_read_u64(struct feat_fd *ff, u64 *addr)
 {
 	int ret;
 
-	ret = __do_read(fd, addr, sizeof(*addr));
+	ret = __do_read(ff, addr, sizeof(*addr));
 	if (ret)
 		return ret;
 
-	if (ph->needs_swap)
+	if (ff->ph->needs_swap)
 		*addr = bswap_64(*addr);
 	return 0;
 }
 
-static char *do_read_string(int fd, struct perf_header *ph)
+static char *do_read_string(struct feat_fd *ff)
 {
 	u32 len;
 	char *buf;
 
-	if (do_read_u32(fd, ph, &len))
+	if (do_read_u32(ff, &len))
 		return NULL;
 
 	buf = malloc(len);
 	if (!buf)
 		return NULL;
 
-	if (!__do_read(fd, buf, len)) {
+	if (!__do_read(ff, buf, len)) {
 		/*
 		 * strings are padded by zeroes
 		 * thus the actual strlen of buf
@@ -1207,8 +1207,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 *ff)
 {
 	struct perf_evsel *evsel, *events = NULL;
 	u64 *id;
@@ -1217,10 +1216,10 @@ read_event_desc(struct perf_header *ph, int fd)
 	size_t msz;
 
 	/* number of events */
-	if (do_read_u32(fd, ph, &nre))
+	if (do_read_u32(ff, &nre))
 		goto error;
 
-	if (do_read_u32(fd, ph, &sz))
+	if (do_read_u32(ff, &sz))
 		goto error;
 
 	/* buffer to hold on file attr struct */
@@ -1244,21 +1243,21 @@ read_event_desc(struct perf_header *ph, int fd)
 		 * must read entire on-file attr struct to
 		 * sync up with layout.
 		 */
-		if (__do_read(fd, buf, sz))
+		if (__do_read(ff, buf, sz))
 			goto error;
 
-		if (ph->needs_swap)
+		if (ff->ph->needs_swap)
 			perf_event__attr_swap(buf);
 
 		memcpy(&evsel->attr, buf, msz);
 
-		if (do_read_u32(fd, ph, &nr))
+		if (do_read_u32(ff, &nr))
 			goto error;
 
-		if (ph->needs_swap)
+		if (ff->ph->needs_swap)
 			evsel->needs_swap = true;
 
-		evsel->name = do_read_string(fd, ph);
+		evsel->name = do_read_string(ff);
 		if (!evsel->name)
 			goto error;
 
@@ -1272,7 +1271,7 @@ read_event_desc(struct perf_header *ph, int fd)
 		evsel->id = id;
 
 		for (j = 0 ; j < nr; j++) {
-			if (do_read_u64(fd, ph, id))
+			if (do_read_u64(ff, id))
 				goto error;
 			id++;
 		}
@@ -1294,7 +1293,7 @@ static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
 
 static void print_event_desc(struct feat_fd *ff, FILE *fp)
 {
-	struct perf_evsel *evsel, *events = read_event_desc(ff->ph, ff->fd);
+	struct perf_evsel *evsel, *events = read_event_desc(ff);
 	u32 j;
 	u64 *id;
 
@@ -1596,7 +1595,7 @@ static int perf_header__read_build_ids(struct perf_header *header,
 #define FEAT_PROCESS_STR_FUN(__feat, __feat_env) \
 static int process_##__feat(struct feat_fd *ff, void *data __maybe_unused) \
 {\
-	ff->ph->env.__feat_env = do_read_string(ff->fd, ff->ph); \
+	ff->ph->env.__feat_env = do_read_string(ff); \
 	return ff->ph->env.__feat_env ? 0 : -ENOMEM; \
 }
 
@@ -1626,11 +1625,11 @@ static int process_nrcpus(struct feat_fd *ff, void *data __maybe_unused)
 	int ret;
 	u32 nr_cpus_avail, nr_cpus_online;
 
-	ret = do_read_u32(ff->fd, ff->ph, &nr_cpus_avail);
+	ret = do_read_u32(ff, &nr_cpus_avail);
 	if (ret)
 		return ret;
 
-	ret = do_read_u32(ff->fd, ff->ph, &nr_cpus_online);
+	ret = do_read_u32(ff, &nr_cpus_online);
 	if (ret)
 		return ret;
 	ff->ph->env.nr_cpus_avail = (int)nr_cpus_avail;
@@ -1643,7 +1642,7 @@ static int process_total_mem(struct feat_fd *ff, void *data __maybe_unused)
 	u64 total_mem;
 	int ret;
 
-	ret = do_read_u64(ff->fd, ff->ph, &total_mem);
+	ret = do_read_u64(ff, &total_mem);
 	if (ret)
 		return -1;
 	ff->ph->env.total_mem = (unsigned long long)total_mem;
@@ -1686,7 +1685,7 @@ static int
 process_event_desc(struct feat_fd *ff, void *data __maybe_unused)
 {
 	struct perf_session *session;
-	struct perf_evsel *evsel, *events = read_event_desc(ff->ph, ff->fd);
+	struct perf_evsel *evsel, *events = read_event_desc(ff);
 
 	if (!events)
 		return 0;
@@ -1705,7 +1704,7 @@ static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
 	char *str, *cmdline = NULL, **argv = NULL;
 	u32 nr, i, len = 0;
 
-	if (do_read_u32(ff->fd, ff->ph, &nr))
+	if (do_read_u32(ff, &nr))
 		return -1;
 
 	ff->ph->env.nr_cmdline = nr;
@@ -1719,7 +1718,7 @@ static int process_cmdline(struct feat_fd *ff, void *data __maybe_unused)
 		goto error;
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(ff->fd, ff->ph);
+		str = do_read_string(ff);
 		if (!str)
 			goto error;
 
@@ -1751,7 +1750,7 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 	if (!ph->env.cpu)
 		return -1;
 
-	if (do_read_u32(ff->fd, ph, &nr))
+	if (do_read_u32(ff, &nr))
 		goto free_cpu;
 
 	ph->env.nr_sibling_cores = nr;
@@ -1760,7 +1759,7 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 		goto free_cpu;
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(ff->fd, ph);
+		str = do_read_string(ff);
 		if (!str)
 			goto error;
 
@@ -1772,14 +1771,14 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 	}
 	ph->env.sibling_cores = strbuf_detach(&sb, NULL);
 
-	if (do_read_u32(ff->fd, ph, &nr))
+	if (do_read_u32(ff, &nr))
 		return -1;
 
 	ph->env.nr_sibling_threads = nr;
 	size += sizeof(u32);
 
 	for (i = 0; i < nr; i++) {
-		str = do_read_string(ff->fd, ph);
+		str = do_read_string(ff);
 		if (!str)
 			goto error;
 
@@ -1801,12 +1800,12 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused)
 	}
 
 	for (i = 0; i < (u32)cpu_nr; i++) {
-		if (do_read_u32(ff->fd, ph, &nr))
+		if (do_read_u32(ff, &nr))
 			goto free_cpu;
 
 		ph->env.cpu[i].core_id = nr;
 
-		if (do_read_u32(ff->fd, ph, &nr))
+		if (do_read_u32(ff, &nr))
 			goto free_cpu;
 
 		if (nr != (u32)-1 && nr > (u32)cpu_nr) {
@@ -1834,7 +1833,7 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
 	char *str;
 
 	/* nr nodes */
-	if (do_read_u32(ff->fd, ff->ph, &nr))
+	if (do_read_u32(ff, &nr))
 		return -1;
 
 	nodes = zalloc(sizeof(*nodes) * nr);
@@ -1845,16 +1844,16 @@ static int process_numa_topology(struct feat_fd *ff, void *data __maybe_unused)
 		n = &nodes[i];
 
 		/* node number */
-		if (do_read_u32(ff->fd, ff->ph, &n->node))
+		if (do_read_u32(ff, &n->node))
 			goto error;
 
-		if (do_read_u64(ff->fd, ff->ph, &n->mem_total))
+		if (do_read_u64(ff, &n->mem_total))
 			goto error;
 
-		if (do_read_u64(ff->fd, ff->ph, &n->mem_free))
+		if (do_read_u64(ff, &n->mem_free))
 			goto error;
 
-		str = do_read_string(ff->fd, ff->ph);
+		str = do_read_string(ff);
 		if (!str)
 			goto error;
 
@@ -1880,7 +1879,7 @@ static int process_pmu_mappings(struct feat_fd *ff, void *data __maybe_unused)
 	u32 type;
 	struct strbuf sb;
 
-	if (do_read_u32(ff->fd, ff->ph, &pmu_num))
+	if (do_read_u32(ff, &pmu_num))
 		return -1;
 
 	if (!pmu_num) {
@@ -1893,10 +1892,10 @@ static int process_pmu_mappings(struct feat_fd *ff, void *data __maybe_unused)
 		return -1;
 
 	while (pmu_num) {
-		if (do_read_u32(ff->fd, ff->ph, &type))
+		if (do_read_u32(ff, &type))
 			goto error;
 
-		name = do_read_string(ff->fd, ff->ph);
+		name = do_read_string(ff);
 		if (!name)
 			goto error;
 
@@ -1932,7 +1931,7 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 		u32 nr_members;
 	} *desc;
 
-	if (do_read_u32(ff->fd, ff->ph, &nr_groups))
+	if (do_read_u32(ff, &nr_groups))
 		return -1;
 
 	ff->ph->env.nr_groups = nr_groups;
@@ -1946,14 +1945,14 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 		return -1;
 
 	for (i = 0; i < nr_groups; i++) {
-		desc[i].name = do_read_string(ff->fd, ff->ph);
+		desc[i].name = do_read_string(ff);
 		if (!desc[i].name)
 			goto out_free;
 
-		if (do_read_u32(ff->fd, ff->ph, &desc[i].leader_idx))
+		if (do_read_u32(ff, &desc[i].leader_idx))
 			goto out_free;
 
-		if (do_read_u32(ff->fd, ff->ph, &desc[i].nr_members))
+		if (do_read_u32(ff, &desc[i].nr_members))
 			goto out_free;
 	}
 
@@ -2023,13 +2022,13 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
 	struct cpu_cache_level *caches;
 	u32 cnt, i, version;
 
-	if (do_read_u32(ff->fd, ff->ph, &version))
+	if (do_read_u32(ff, &version))
 		return -1;
 
 	if (version != 1)
 		return -1;
 
-	if (do_read_u32(ff->fd, ff->ph, &cnt))
+	if (do_read_u32(ff, &cnt))
 		return -1;
 
 	caches = zalloc(sizeof(*caches) * cnt);
@@ -2040,7 +2039,7 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
 		struct cpu_cache_level c;
 
 		#define _R(v)						\
-			if (do_read_u32(ff->fd, ff->ph, &c.v))\
+			if (do_read_u32(ff, &c.v))\
 				goto out_free_caches;			\
 
 		_R(level)
@@ -2050,7 +2049,7 @@ static int process_cache(struct feat_fd *ff, void *data __maybe_unused)
 		#undef _R
 
 		#define _R(v)					\
-			c.v = do_read_string(ff->fd, ff->ph);	\
+			c.v = do_read_string(ff);		\
 			if (!c.v)				\
 				goto out_free_caches;
 
-- 
2.13.2.725.g09c95d1e9-goog

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

* [PATCH v5 11/16] perf header: make write_pmu_mappings pipe-mode friendly
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (9 preceding siblings ...)
  2017-07-11 23:53 ` [PATCH v5 10/16] perf header: use struct feat_fd in read header records David Carrillo-Cisneros
@ 2017-07-11 23:53 ` David Carrillo-Cisneros
  2017-07-12  1:39 ` [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Ahern
  2017-07-12 14:31 ` Jiri Olsa
  12 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-11 23:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, David Ahern, Namhyung Kim,
	Stephane Eranian, Paul Turner, David Carrillo-Cisneros

In pipe-mode, we will operate over a buffer instead of a file descriptor
but write_pmu_mappings uses lseek to move over the perf.data file.

Refactor write_pmu_mappings to avoid the usage of lseek and allow
reusing the same logic in pipe-mode (next patch).

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

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index e9f1d293d4ba..623cfbf72116 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -796,11 +796,19 @@ static int write_pmu_mappings(struct feat_fd *ff,
 			      struct perf_evlist *evlist __maybe_unused)
 {
 	struct perf_pmu *pmu = NULL;
-	off_t offset = lseek(ff->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(ff, &pmu_num, sizeof(pmu_num));
 	if (ret < 0)
 		return ret;
@@ -808,7 +816,6 @@ static int write_pmu_mappings(struct feat_fd *ff,
 	while ((pmu = perf_pmu__scan(pmu))) {
 		if (!pmu->name)
 			continue;
-		pmu_num++;
 
 		ret = do_write(ff, &pmu->type, sizeof(pmu->type));
 		if (ret < 0)
@@ -819,12 +826,6 @@ static int write_pmu_mappings(struct feat_fd *ff,
 			return ret;
 	}
 
-	if (pwrite(ff->fd, &pmu_num, sizeof(pmu_num), offset) != sizeof(pmu_num)) {
-		/* discard all */
-		lseek(ff->fd, offset, SEEK_SET);
-		return -1;
-	}
-
 	return 0;
 }
 
-- 
2.13.2.725.g09c95d1e9-goog

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

* Re: [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (10 preceding siblings ...)
  2017-07-11 23:53 ` [PATCH v5 11/16] perf header: make write_pmu_mappings pipe-mode friendly David Carrillo-Cisneros
@ 2017-07-12  1:39 ` David Ahern
  2017-07-18  4:37   ` David Carrillo-Cisneros
  2017-07-12 14:31 ` Jiri Olsa
  12 siblings, 1 reply; 16+ messages in thread
From: David Ahern @ 2017-07-12  1:39 UTC (permalink / raw)
  To: David Carrillo-Cisneros, linux-kernel
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Simon Que, Wang Nan, Jiri Olsa,
	He Kuang, Masami Hiramatsu, Namhyung Kim, Stephane Eranian,
	Paul Turner

On 7/11/17 5:52 PM, David Carrillo-Cisneros wrote:
...
> (This is a rebased and updated version of Stephane Eranian's version
>  in https://patchwork.kernel.org/patch/1499081/)

...
> With this series, it is possible to get:
>   $ perf record -o - -e cycles sleep 1 | perf report --stdio --header
>   # ========
>   # captured on: Mon May 22 16:33:43 2017
>   # ========
>   #
>   # hostname : my_hostname
>   # 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 -e cycles sleep 1 
>   # event : name = cycles, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1
>   # CPU_TOPOLOGY info available, use -I to display
>   # NUMA_TOPOLOGY info available, use -I to display
>   # pmu mappings: intel_bts = 6, cpu = 4, msr = 49, uncore_cbox_10 = 36, [SNIP]
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.000 MB - ]
>   ...

I like it; much needed functionality.

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

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

* Re: [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode
  2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
                   ` (11 preceding siblings ...)
  2017-07-12  1:39 ` [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Ahern
@ 2017-07-12 14:31 ` Jiri Olsa
  2017-07-18  4:39   ` David Carrillo-Cisneros
  12 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2017-07-12 14:31 UTC (permalink / raw)
  To: David Carrillo-Cisneros
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Andi Kleen,
	Simon Que, Wang Nan, Jiri Olsa, He Kuang, Masami Hiramatsu,
	David Ahern, Namhyung Kim, Stephane Eranian, Paul Turner

On Tue, Jul 11, 2017 at 04:52:51PM -0700, David Carrillo-Cisneros wrote:
> v5: - Fix buffer leaking and size changes in
>       perf_event__synthesize_features.
>     - Remove unnecessary renaming.
>     - Remove extra tabs in do_write_string.

SNIP

> 
> Only patches 14 to 16 have a significant effect in user observed behavior.
> All other are transparent preparatory changes or bug fixes.
> 
> David Carrillo-Cisneros (16):
>   perf header: encapsulate read and swap
>   perf header: add PROCESS_STR_FUN macro
>   perf header: fail on write_padded error
>   perf util: add const modifier to buf in "writen" function
>   perf header: revamp do_write
>   perf header: add struct feat_fd for write
>   perf header: use struct feat_fd for print
>   perf header: use struct feat_fd to process header records
>   perf header: don't pass struct perf_file_section to process_##_feat
>   perf header: use struct feat_fd in read header records
>   perf header: make write_pmu_mappings pipe-mode friendly

hum, not sure if that's an issue on my side, but I can see
only patches 0-11 (above portion), missing patches 12-16:

>   perf header: add a buffer to struct feat_fd
>   perf header: change FEAT_OP* macros
>   perf tool: add show_feature_header to perf_tool
>   perf tools: add feature header record to pipe-mode
>   perf header: add event desc to pipe-mode header


jirka

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

* Re: [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode
  2017-07-12  1:39 ` [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Ahern
@ 2017-07-18  4:37   ` David Carrillo-Cisneros
  0 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-18  4:37 UTC (permalink / raw)
  To: David Ahern
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Andi Kleen,
	Simon Que, Wang Nan, Jiri Olsa, He Kuang, Masami Hiramatsu,
	Namhyung Kim, Stephane Eranian, Paul Turner

On Tue, Jul 11, 2017 at 6:39 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/11/17 5:52 PM, David Carrillo-Cisneros wrote:
> ...
>> (This is a rebased and updated version of Stephane Eranian's version
>>  in https://patchwork.kernel.org/patch/1499081/)
>
> ...
>> With this series, it is possible to get:
>>   $ perf record -o - -e cycles sleep 1 | perf report --stdio --header
>>   # ========
>>   # captured on: Mon May 22 16:33:43 2017
>>   # ========
>>   #
>>   # hostname : my_hostname
>>   # 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 -e cycles sleep 1
>>   # event : name = cycles, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1
>>   # CPU_TOPOLOGY info available, use -I to display
>>   # NUMA_TOPOLOGY info available, use -I to display
>>   # pmu mappings: intel_bts = 6, cpu = 4, msr = 49, uncore_cbox_10 = 36, [SNIP]
>>   [ perf record: Woken up 1 times to write data ]
>>   [ perf record: Captured and wrote 0.000 MB - ]
>>   ...
>
> I like it; much needed functionality.

Great to read! Hopefully v6 will make it.

David

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

* Re: [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode
  2017-07-12 14:31 ` Jiri Olsa
@ 2017-07-18  4:39   ` David Carrillo-Cisneros
  0 siblings, 0 replies; 16+ messages in thread
From: David Carrillo-Cisneros @ 2017-07-18  4:39 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Alexander Shishkin, Andi Kleen,
	Simon Que, Wang Nan, Jiri Olsa, He Kuang, Masami Hiramatsu,
	David Ahern, Namhyung Kim, Stephane Eranian, Paul Turner

On Wed, Jul 12, 2017 at 7:31 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Tue, Jul 11, 2017 at 04:52:51PM -0700, David Carrillo-Cisneros wrote:
>> v5: - Fix buffer leaking and size changes in
>>       perf_event__synthesize_features.
>>     - Remove unnecessary renaming.
>>     - Remove extra tabs in do_write_string.
>
> SNIP
>
>>
>> Only patches 14 to 16 have a significant effect in user observed behavior.
>> All other are transparent preparatory changes or bug fixes.
>>
>> David Carrillo-Cisneros (16):
>>   perf header: encapsulate read and swap
>>   perf header: add PROCESS_STR_FUN macro
>>   perf header: fail on write_padded error
>>   perf util: add const modifier to buf in "writen" function
>>   perf header: revamp do_write
>>   perf header: add struct feat_fd for write
>>   perf header: use struct feat_fd for print
>>   perf header: use struct feat_fd to process header records
>>   perf header: don't pass struct perf_file_section to process_##_feat
>>   perf header: use struct feat_fd in read header records
>>   perf header: make write_pmu_mappings pipe-mode friendly
>
> hum, not sure if that's an issue on my side, but I can see
> only patches 0-11 (above portion), missing patches 12-16:

Mmm, no idea. I reposted a new version and also uploaded it to
https://github.com/ccdavid/linux/tree/toup_july16_pipeheaders_02

>
>>   perf header: add a buffer to struct feat_fd
>>   perf header: change FEAT_OP* macros
>>   perf tool: add show_feature_header to perf_tool
>>   perf tools: add feature header record to pipe-mode
>>   perf header: add event desc to pipe-mode header
>
>
> jirka

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

end of thread, other threads:[~2017-07-18  4:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-11 23:52 [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 01/16] perf header: encapsulate read and swap David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 02/16] perf header: add PROCESS_STR_FUN macro David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 03/16] perf header: fail on write_padded error David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 04/16] perf util: add const modifier to buf in "writen" function David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 05/16] perf header: revamp do_write David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 06/16] perf header: add struct feat_fd for write David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 07/16] perf header: use struct feat_fd for print David Carrillo-Cisneros
2017-07-11 23:52 ` [PATCH v5 08/16] perf header: use struct feat_fd to process header records David Carrillo-Cisneros
2017-07-11 23:53 ` [PATCH v5 09/16] perf header: don't pass struct perf_file_section to process_##_feat David Carrillo-Cisneros
2017-07-11 23:53 ` [PATCH v5 10/16] perf header: use struct feat_fd in read header records David Carrillo-Cisneros
2017-07-11 23:53 ` [PATCH v5 11/16] perf header: make write_pmu_mappings pipe-mode friendly David Carrillo-Cisneros
2017-07-12  1:39 ` [PATCH v5 00/16] perf tool: add meta-data header support for pipe-mode David Ahern
2017-07-18  4:37   ` David Carrillo-Cisneros
2017-07-12 14:31 ` Jiri Olsa
2017-07-18  4:39   ` David Carrillo-Cisneros

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