All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] perf tools: Synthetize the targeted process
@ 2009-11-11  3:51 Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 2/6] perf tools: Move the build-id storage operations to headers Frederic Weisbecker
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11  3:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Mike Galbraith, Paul Mackerras,
	Hitoshi Mitake

Don't forget to also synthetize the targeted process from perf record
or we'll miss its dso in the events and then we won't be able to deal
with its build-id.

We are missing it because it is created after the existing synthetized
tasks but before the counters are enabled and can send its mapping
event.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/builtin-record.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cf2cd25..2b45d33 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -497,13 +497,22 @@ static int __cmd_record(int argc, const char **argv)
 	if (target_pid == -1 && argc) {
 		pid = fork();
 		if (pid < 0)
-			perror("failed to fork");
+			die("failed to fork");
 
 		if (!pid) {
 			if (execvp(argv[0], (char **)argv)) {
 				perror(argv[0]);
 				exit(-1);
 			}
+		} else {
+			/*
+			 * Wait a bit for the execv'ed child to appear
+			 * and be updated in /proc
+			 * FIXME: Do you know a less heuristical solution?
+			 */
+			usleep(1000);
+			event__synthesize_thread(pid,
+						 process_synthesized_event);
 		}
 
 		child_pid = pid;
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/6] perf tools: Move the build-id storage operations to headers
  2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
@ 2009-11-11  3:51 ` Frederic Weisbecker
  2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 3/6] perf tools: Split up build id saving into fetch and write Frederic Weisbecker
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11  3:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Mike Galbraith, Paul Mackerras,
	Hitoshi Mitake

So that it makes easier to control it. Especially because we plan
to give it a feature section.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/builtin-record.c |   32 ++------------------------------
 tools/perf/util/header.c    |   41 ++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/header.h    |    2 +-
 3 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2b45d33..3044a62 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -378,39 +378,11 @@ static void open_counters(int cpu, pid_t pid)
 	nr_cpu++;
 }
 
-static bool write_buildid_table(void)
-{
-	struct dso *pos;
-	bool have_buildid = false;
-
-	list_for_each_entry(pos, &dsos, node) {
-		struct build_id_event b;
-		size_t len;
-
-		if (filename__read_build_id(pos->long_name,
-					    &b.build_id,
-					    sizeof(b.build_id)) < 0)
-			continue;
-		have_buildid = true;
-		memset(&b.header, 0, sizeof(b.header));
-		len = strlen(pos->long_name) + 1;
-		len = ALIGN(len, 64);
-		b.header.size = sizeof(b) + len;
-		write_output(&b, sizeof(b));
-		write_output(pos->long_name, len);
-	}
-
-	return have_buildid;
-}
-
 static void atexit_header(void)
 {
 	header->data_size += bytes_written;
 
-	if (write_buildid_table())
-		perf_header__set_feat(header, HEADER_BUILD_ID);
-
-	perf_header__write(header, output);
+	perf_header__write(header, output, true);
 }
 
 static int __cmd_record(int argc, const char **argv)
@@ -487,7 +459,7 @@ static int __cmd_record(int argc, const char **argv)
 	}
 
 	if (file_new)
-		perf_header__write(header, output);
+		perf_header__write(header, output, false);
 
 	if (!system_wide)
 		event__synthesize_thread(pid, process_synthesized_event);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 050f543..a4d0bbe 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2,11 +2,13 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <linux/list.h>
 
 #include "util.h"
 #include "header.h"
 #include "../perf.h"
 #include "trace-event.h"
+#include "symbol.h"
 
 /*
  * Create new perf.data header attribute:
@@ -172,7 +174,33 @@ static void do_write(int fd, void *buf, size_t size)
 	}
 }
 
-static void perf_header__adds_write(struct perf_header *self, int fd)
+static bool write_buildid_table(int fd)
+{
+	struct dso *pos;
+	bool have_buildid = false;
+
+	list_for_each_entry(pos, &dsos, node) {
+		struct build_id_event b;
+		size_t len;
+
+		if (filename__read_build_id(pos->long_name,
+					    &b.build_id,
+					    sizeof(b.build_id)) < 0)
+			continue;
+		have_buildid = true;
+		memset(&b.header, 0, sizeof(b.header));
+		len = strlen(pos->long_name) + 1;
+		len = ALIGN(len, 64);
+		b.header.size = sizeof(b) + len;
+		do_write(fd, &b, sizeof(b));
+		do_write(fd, pos->long_name, len);
+	}
+
+	return have_buildid;
+}
+
+static void
+perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
 {
 	struct perf_file_section trace_sec;
 	u64 cur_offset = lseek(fd, 0, SEEK_CUR);
@@ -196,9 +224,16 @@ static void perf_header__adds_write(struct perf_header *self, int fd)
 		 */
 		cur_offset = lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
 	}
+
+	if (at_exit) {
+		lseek(fd, self->data_offset + self->data_size, SEEK_SET);
+		if (write_buildid_table(fd))
+			perf_header__set_feat(self, HEADER_BUILD_ID);
+		lseek(fd, cur_offset, SEEK_SET);
+	}
 };
 
-void perf_header__write(struct perf_header *self, int fd)
+void perf_header__write(struct perf_header *self, int fd, bool at_exit)
 {
 	struct perf_file_header f_header;
 	struct perf_file_attr   f_attr;
@@ -236,7 +271,7 @@ void perf_header__write(struct perf_header *self, int fd)
 	if (events)
 		do_write(fd, events, self->event_size);
 
-	perf_header__adds_write(self, fd);
+	perf_header__adds_write(self, fd, at_exit);
 
 	self->data_offset = lseek(fd, 0, SEEK_CUR);
 
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 2f233c5..77186c9 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -33,7 +33,7 @@ struct perf_header {
 };
 
 struct perf_header *perf_header__read(int fd);
-void perf_header__write(struct perf_header *self, int fd);
+void perf_header__write(struct perf_header *self, int fd, bool at_exit);
 
 void perf_header__add_attr(struct perf_header *self,
 			   struct perf_header_attr *attr);
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 3/6] perf tools: Split up build id saving into fetch and write
  2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 2/6] perf tools: Move the build-id storage operations to headers Frederic Weisbecker
@ 2009-11-11  3:51 ` Frederic Weisbecker
  2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 4/6] perf tools: Read the build-ids from the header layer Frederic Weisbecker
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11  3:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Mike Galbraith, Paul Mackerras,
	Hitoshi Mitake

We are saving the build id once we stop the profiling. And only after
doing that we know if we need to set that feature in the header through
the feature bitmap.

But if we want a proper feature support in the headers, using a rule of
offset/size pairs in sections, we need to know in advance how many
features we need to set in the headers, so that we can reserve rooms
for their section headers.

The current state doesn't allow that, as it forces us to first save
the build-ids to the file right after the datas instead of planning
any structured layout.

That's why this splits up the build-ids processing in two parts: one
that fetches the build-ids from the Dso objects, and one that saves
them into the file.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/util/event.h  |    7 +++++++
 tools/perf/util/header.c |   41 +++++++++++++++++------------------------
 tools/perf/util/symbol.c |   34 ++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h |    1 +
 4 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 34c6fcb..1f771ce 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -69,6 +69,13 @@ struct build_id_event {
 	char			 filename[];
 };
 
+struct build_id_list {
+	struct build_id_event	event;
+	struct list_head	list;
+	const char		*dso_name;
+	int			len;
+};
+
 typedef union event_union {
 	struct perf_event_header	header;
 	struct ip_event			ip;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a4d0bbe..2f702c2 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -174,29 +174,18 @@ static void do_write(int fd, void *buf, size_t size)
 	}
 }
 
-static bool write_buildid_table(int fd)
+static void write_buildid_table(int fd, struct list_head *id_head)
 {
-	struct dso *pos;
-	bool have_buildid = false;
-
-	list_for_each_entry(pos, &dsos, node) {
-		struct build_id_event b;
-		size_t len;
-
-		if (filename__read_build_id(pos->long_name,
-					    &b.build_id,
-					    sizeof(b.build_id)) < 0)
-			continue;
-		have_buildid = true;
-		memset(&b.header, 0, sizeof(b.header));
-		len = strlen(pos->long_name) + 1;
-		len = ALIGN(len, 64);
-		b.header.size = sizeof(b) + len;
-		do_write(fd, &b, sizeof(b));
-		do_write(fd, pos->long_name, len);
-	}
+	struct build_id_list *iter, *next;
+
+	list_for_each_entry_safe(iter, next, id_head, list) {
+		struct build_id_event *b = &iter->event;
 
-	return have_buildid;
+		do_write(fd, b, sizeof(*b));
+		do_write(fd, (void *)iter->dso_name, iter->len);
+		list_del(&iter->list);
+		free(iter);
+	}
 }
 
 static void
@@ -226,10 +215,14 @@ perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
 	}
 
 	if (at_exit) {
-		lseek(fd, self->data_offset + self->data_size, SEEK_SET);
-		if (write_buildid_table(fd))
+		LIST_HEAD(id_list);
+
+		if (fetch_build_id_table(&id_list)) {
+			lseek(fd, self->data_offset + self->data_size, SEEK_SET);
 			perf_header__set_feat(self, HEADER_BUILD_ID);
-		lseek(fd, cur_offset, SEEK_SET);
+			write_buildid_table(fd, &id_list);
+			lseek(fd, cur_offset, SEEK_SET);
+		}
 	}
 };
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a2e95ce..9c286db 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -851,6 +851,40 @@ out_close:
 	return err;
 }
 
+bool fetch_build_id_table(struct list_head *head)
+{
+	bool have_buildid = false;
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node) {
+		struct build_id_list *new;
+		struct build_id_event b;
+		size_t len;
+
+		if (filename__read_build_id(pos->long_name,
+					    &b.build_id,
+					    sizeof(b.build_id)) < 0)
+			continue;
+		have_buildid = true;
+		memset(&b.header, 0, sizeof(b.header));
+		len = strlen(pos->long_name) + 1;
+		len = ALIGN(len, 64);
+		b.header.size = sizeof(b) + len;
+
+		new = malloc(sizeof(*new));
+		if (!new)
+			die("No memory\n");
+
+		memcpy(&new->event, &b, sizeof(b));
+		new->dso_name = pos->long_name;
+		new->len = len;
+
+		list_add_tail(&new->list, head);
+	}
+
+	return have_buildid;
+}
+
 int filename__read_build_id(const char *filename, void *bf, size_t size)
 {
 	int fd, err = -1;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f8c1899..0a34a54 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -86,6 +86,7 @@ char dso__symtab_origin(const struct dso *self);
 void dso__set_build_id(struct dso *self, void *build_id);
 
 int filename__read_build_id(const char *filename, void *bf, size_t size);
+bool fetch_build_id_table(struct list_head *head);
 int build_id__sprintf(u8 *self, int len, char *bf);
 
 int load_kernel(symbol_filter_t filter);
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 4/6] perf tools: Read the build-ids from the header layer
  2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 2/6] perf tools: Move the build-id storage operations to headers Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 3/6] perf tools: Split up build id saving into fetch and write Frederic Weisbecker
@ 2009-11-11  3:51 ` Frederic Weisbecker
  2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 5/6] perf tools: Use perf_header__set/has_feat whenever possible Frederic Weisbecker
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11  3:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Mike Galbraith, Paul Mackerras,
	Hitoshi Mitake

Keep the build-ids reading implementation in the data mapping but move
its call to the headers so that we have a better control on it
(offset seeking, size passing, etc..).

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/util/data_map.c |    8 ++------
 tools/perf/util/data_map.h |    2 ++
 tools/perf/util/header.c   |   14 ++++++++++++--
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/data_map.c b/tools/perf/util/data_map.c
index 00a9c11..66e58aa 100644
--- a/tools/perf/util/data_map.c
+++ b/tools/perf/util/data_map.c
@@ -70,8 +70,8 @@ process_event(event_t *event, unsigned long offset, unsigned long head)
 	}
 }
 
-static int perf_header__read_build_ids(const struct perf_header *self,
-				       int input, off_t file_size)
+int perf_header__read_build_ids(const struct perf_header *self,
+				int input, off_t file_size)
 {
 	off_t offset = self->data_offset + self->data_size;
 	struct build_id_event bev;
@@ -163,10 +163,6 @@ int mmap_dispatch_perf_file(struct perf_header **pheader,
 		if (curr_handler->sample_type_check(sample_type) < 0)
 			exit(-1);
 
-	if (perf_header__has_feat(header, HEADER_BUILD_ID) &&
-	    perf_header__read_build_ids(header, input, input_stat.st_size))
-		pr_debug("failed to read buildids, continuing...\n");
-
 	if (load_kernel(NULL) < 0) {
 		perror("failed to load kernel symbols");
 		return EXIT_FAILURE;
diff --git a/tools/perf/util/data_map.h b/tools/perf/util/data_map.h
index 716d105..c412281 100644
--- a/tools/perf/util/data_map.h
+++ b/tools/perf/util/data_map.h
@@ -27,5 +27,7 @@ int mmap_dispatch_perf_file(struct perf_header **pheader,
 			    int full_paths,
 			    int *cwdlen,
 			    char **cwd);
+int perf_header__read_build_ids(const struct perf_header *self,
+				int input, off_t file_size);
 
 #endif
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2f702c2..915b56e 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -9,6 +9,8 @@
 #include "../perf.h"
 #include "trace-event.h"
 #include "symbol.h"
+#include "data_map.h"
+#include "debug.h"
 
 /*
  * Create new perf.data header attribute:
@@ -322,6 +324,14 @@ static void perf_header__adds_read(struct perf_header *self, int fd)
 		trace_report(fd);
 		lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
 	}
+
+	if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
+		struct stat input_stat;
+
+		fstat(fd, &input_stat);
+		if (perf_header__read_build_ids(self, fd, input_stat.st_size))
+			pr_debug("failed to read buildids, continuing...\n");
+	}
 };
 
 struct perf_header *perf_header__read(int fd)
@@ -382,14 +392,14 @@ struct perf_header *perf_header__read(int fd)
 
 	memcpy(&self->adds_features, &f_header.adds_features, sizeof(f_header.adds_features));
 
-	perf_header__adds_read(self, fd);
-
 	self->event_offset = f_header.event_types.offset;
 	self->event_size   = f_header.event_types.size;
 
 	self->data_offset = f_header.data.offset;
 	self->data_size   = f_header.data.size;
 
+	perf_header__adds_read(self, fd);
+
 	lseek(fd, self->data_offset, SEEK_SET);
 
 	self->frozen = 1;
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 5/6] perf tools: Use perf_header__set/has_feat whenever possible
  2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
                   ` (2 preceding siblings ...)
  2009-11-11  3:51 ` [PATCH 4/6] perf tools: Read the build-ids from the header layer Frederic Weisbecker
@ 2009-11-11  3:51 ` Frederic Weisbecker
  2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
  2009-11-11  3:51 ` [PATCH 6/6] perf tools: Bring linear set of section headers for features Frederic Weisbecker
  2009-11-11  7:42 ` [tip:perf/core] perf tools: Synthetize the targeted process tip-bot for Frederic Weisbecker
  5 siblings, 1 reply; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11  3:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Mike Galbraith, Paul Mackerras,
	Hitoshi Mitake

And drop the alternate checks/sets using set_bit or other kind of
helpers.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/builtin-record.c |    4 ++--
 tools/perf/util/header.c    |   12 ++----------
 tools/perf/util/header.h    |    1 -
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3044a62..04f335e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -431,11 +431,11 @@ static int __cmd_record(int argc, const char **argv)
 		header = perf_header__new();
 
 	if (raw_samples) {
-		perf_header__feat_trace_info(header);
+		perf_header__set_feat(header, HEADER_TRACE_INFO);
 	} else {
 		for (i = 0; i < nr_counters; i++) {
 			if (attrs[i].sample_type & PERF_SAMPLE_RAW) {
-				perf_header__feat_trace_info(header);
+				perf_header__set_feat(header, HEADER_TRACE_INFO);
 				break;
 			}
 		}
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 915b56e..9709d38 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -148,11 +148,6 @@ struct perf_file_header {
 	DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS);
 };
 
-void perf_header__feat_trace_info(struct perf_header *header)
-{
-	set_bit(HEADER_TRACE_INFO, header->adds_features);
-}
-
 void perf_header__set_feat(struct perf_header *self, int feat)
 {
 	set_bit(feat, self->adds_features);
@@ -195,9 +190,8 @@ perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
 {
 	struct perf_file_section trace_sec;
 	u64 cur_offset = lseek(fd, 0, SEEK_CUR);
-	unsigned long *feat_mask = self->adds_features;
 
-	if (test_bit(HEADER_TRACE_INFO, feat_mask)) {
+	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
 		/* Write trace info */
 		trace_sec.offset = lseek(fd, sizeof(trace_sec), SEEK_CUR);
 		read_tracing_data(fd, attrs, nr_counters);
@@ -314,9 +308,7 @@ static void do_read(int fd, void *buf, size_t size)
 
 static void perf_header__adds_read(struct perf_header *self, int fd)
 {
-	const unsigned long *feat_mask = self->adds_features;
-
-	if (test_bit(HEADER_TRACE_INFO, feat_mask)) {
+	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
 		struct perf_file_section trace_sec;
 
 		do_read(fd, &trace_sec, sizeof(trace_sec));
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 77186c9..a22d70b 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -49,7 +49,6 @@ void perf_header_attr__add_id(struct perf_header_attr *self, u64 id);
 u64 perf_header__sample_type(struct perf_header *header);
 struct perf_event_attr *
 perf_header__find_attr(u64 id, struct perf_header *header);
-void perf_header__feat_trace_info(struct perf_header *header);
 void perf_header__set_feat(struct perf_header *self, int feat);
 bool perf_header__has_feat(const struct perf_header *self, int feat);
 
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 6/6] perf tools: Bring linear set of section headers for features
  2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
                   ` (3 preceding siblings ...)
  2009-11-11  3:51 ` [PATCH 5/6] perf tools: Use perf_header__set/has_feat whenever possible Frederic Weisbecker
@ 2009-11-11  3:51 ` Frederic Weisbecker
  2009-11-11  6:20   ` Peter Zijlstra
  2009-11-11  7:44   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
  2009-11-11  7:42 ` [tip:perf/core] perf tools: Synthetize the targeted process tip-bot for Frederic Weisbecker
  5 siblings, 2 replies; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11  3:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
	Arnaldo Carvalho de Melo, Mike Galbraith, Paul Mackerras,
	Hitoshi Mitake

Build a set of section headers for features right after the datas.
Each implemented feature will have one of such section header that
provides the offset and the size of the data manipulated by the feature.

The trace informations have moved after the data and are recorded
on exit time.

The new layout is as follows:

 -----------------------
                             ___
 [ magic               ]      |
 [ header size         ]      |
 [ attr size           ]      |
 [ attr content offset ]      |
 [ attr content size   ]      |
 [ data offset         ]  File Headers
 [ data size           ]      |
 [ event_types offset  ]      |
 [ event_types size    ]      |
 [ feature bitmap      ]      v 

 [ attr section        ]
 [ events section      ]

                             ___
 [         X           ]      |
 [         X           ]      |
 [         X           ]    Datas
 [         X           ]      |
 [         X           ]      v

                             ___
 [ Feature 1 offset    ]      |
 [ Feature 1 size      ] Features headers
 [ Feature 2 offset    ]      |
 [ Feature 2 size      ]      v

 [ Feature 1 content   ]
 [ Feature 2 content   ]
 -----------------------

We have as many feature's section headers as we have features in use for
the current file.

Say Feat 1 and Feat 3 are used by the file, but not Feat 2. Then the
feature headers will be like follows:

[ Feature 1 offset    ]      |
[ Feature 1 size      ] Features headers
[ Feature 3 offset    ]      |
[ Feature 3 size      ]      v

There is no hole to cover Feature 2 that is not in use here. We only
need to cover the needed headers in order, from the lowest feature bit
to the highest.

Currently we have two features: HEADER_TRACE_INFO and HEADER_BUILD_ID.
Both have their contents that follow the feature headers. Putting the
contents right after the feature headers is not mandatory though.
While we keep the feature headers right after the data and in order,
their offsets can point everywhere. We have just put the two above
feature contents in the end of the file for convenience.

The purpose of this layout change is to have a file format that scales
while keeping it simple: having such linear feature headers is less
error prone wrt forward/backward compatibility as the content of a
feature can be put anywhere, its location can even change by the
time, it's fine because its headers will tell where it is. And we know
how to find these headers, following the above rules.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
---
 tools/perf/util/data_map.c             |   11 +--
 tools/perf/util/data_map.h             |    3 +-
 tools/perf/util/header.c               |  110 ++++++++++++++++++++++----------
 tools/perf/util/include/linux/bitmap.h |    1 +
 tools/perf/util/include/linux/ctype.h  |    2 +-
 tools/perf/util/util.h                 |    3 +
 6 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/tools/perf/util/data_map.c b/tools/perf/util/data_map.c
index 66e58aa..aacb814 100644
--- a/tools/perf/util/data_map.c
+++ b/tools/perf/util/data_map.c
@@ -70,18 +70,15 @@ process_event(event_t *event, unsigned long offset, unsigned long head)
 	}
 }
 
-int perf_header__read_build_ids(const struct perf_header *self,
-				int input, off_t file_size)
+int perf_header__read_build_ids(int input, off_t size)
 {
-	off_t offset = self->data_offset + self->data_size;
 	struct build_id_event bev;
 	char filename[PATH_MAX];
+	off_t offset = lseek(input, 0, SEEK_CUR);
+	off_t limit = offset + size;
 	int err = -1;
 
-	if (lseek(input, offset, SEEK_SET) < 0)
-		return -1;
-
-	while (offset < file_size) {
+	while (offset < limit) {
 		struct dso *dso;
 		ssize_t len;
 
diff --git a/tools/perf/util/data_map.h b/tools/perf/util/data_map.h
index c412281..20b4037 100644
--- a/tools/perf/util/data_map.h
+++ b/tools/perf/util/data_map.h
@@ -27,7 +27,6 @@ int mmap_dispatch_perf_file(struct perf_header **pheader,
 			    int full_paths,
 			    int *cwdlen,
 			    char **cwd);
-int perf_header__read_build_ids(const struct perf_header *self,
-				int input, off_t file_size);
+int perf_header__read_build_ids(int input, off_t file_size);
 
 #endif
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 9709d38..ebed4f4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -186,41 +186,58 @@ static void write_buildid_table(int fd, struct list_head *id_head)
 }
 
 static void
-perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
+perf_header__adds_write(struct perf_header *self, int fd)
 {
-	struct perf_file_section trace_sec;
-	u64 cur_offset = lseek(fd, 0, SEEK_CUR);
+	LIST_HEAD(id_list);
+	int nr_sections;
+	struct perf_file_section *feat_sec;
+	int sec_size;
+	u64 sec_start;
+	int idx = 0;
+
+	if (fetch_build_id_table(&id_list))
+		perf_header__set_feat(self, HEADER_BUILD_ID);
+
+	nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
+	if (!nr_sections)
+		return;
+
+	feat_sec = calloc(sizeof(*feat_sec), nr_sections);
+	if (!feat_sec)
+		die("No memory");
+
+	sec_size = sizeof(*feat_sec) * nr_sections;
+
+	sec_start = self->data_offset + self->data_size;
+	lseek(fd, sec_start + sec_size, SEEK_SET);
 
 	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
+		struct perf_file_section *trace_sec;
+
+		trace_sec = &feat_sec[idx++];
+
 		/* Write trace info */
-		trace_sec.offset = lseek(fd, sizeof(trace_sec), SEEK_CUR);
+		trace_sec->offset = lseek(fd, 0, SEEK_CUR);
 		read_tracing_data(fd, attrs, nr_counters);
-		trace_sec.size = lseek(fd, 0, SEEK_CUR) - trace_sec.offset;
-
-		/* Write trace info headers */
-		lseek(fd, cur_offset, SEEK_SET);
-		do_write(fd, &trace_sec, sizeof(trace_sec));
-
-		/*
-		 * Update cur_offset. So that other (future)
-		 * features can set their own infos in this place. But if we are
-		 * the only feature, at least that seeks to the place the data
-		 * should begin.
-		 */
-		cur_offset = lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
+		trace_sec->size = lseek(fd, 0, SEEK_CUR) - trace_sec->offset;
 	}
 
-	if (at_exit) {
-		LIST_HEAD(id_list);
 
-		if (fetch_build_id_table(&id_list)) {
-			lseek(fd, self->data_offset + self->data_size, SEEK_SET);
-			perf_header__set_feat(self, HEADER_BUILD_ID);
-			write_buildid_table(fd, &id_list);
-			lseek(fd, cur_offset, SEEK_SET);
-		}
+	if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
+		struct perf_file_section *buildid_sec;
+
+		buildid_sec = &feat_sec[idx++];
+
+		/* Write build-ids */
+		buildid_sec->offset = lseek(fd, 0, SEEK_CUR);
+		write_buildid_table(fd, &id_list);
+		buildid_sec->size = lseek(fd, 0, SEEK_CUR) - buildid_sec->offset;
 	}
-};
+
+	lseek(fd, sec_start, SEEK_SET);
+	do_write(fd, feat_sec, sec_size);
+	free(feat_sec);
+}
 
 void perf_header__write(struct perf_header *self, int fd, bool at_exit)
 {
@@ -260,10 +277,11 @@ void perf_header__write(struct perf_header *self, int fd, bool at_exit)
 	if (events)
 		do_write(fd, events, self->event_size);
 
-	perf_header__adds_write(self, fd, at_exit);
-
 	self->data_offset = lseek(fd, 0, SEEK_CUR);
 
+	if (at_exit)
+		perf_header__adds_write(self, fd);
+
 	f_header = (struct perf_file_header){
 		.magic	   = PERF_MAGIC,
 		.size	   = sizeof(f_header),
@@ -308,22 +326,44 @@ static void do_read(int fd, void *buf, size_t size)
 
 static void perf_header__adds_read(struct perf_header *self, int fd)
 {
+	struct perf_file_section *feat_sec;
+	int nr_sections;
+	int sec_size;
+	int idx = 0;
+
+
+	nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
+	if (!nr_sections)
+		return;
+
+	feat_sec = calloc(sizeof(*feat_sec), nr_sections);
+	if (!feat_sec)
+		die("No memory");
+
+	sec_size = sizeof(*feat_sec) * nr_sections;
+
+	lseek(fd, self->data_offset + self->data_size, SEEK_SET);
+
+	do_read(fd, feat_sec, sec_size);
+
 	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
-		struct perf_file_section trace_sec;
+		struct perf_file_section *trace_sec;
 
-		do_read(fd, &trace_sec, sizeof(trace_sec));
-		lseek(fd, trace_sec.offset, SEEK_SET);
+		trace_sec = &feat_sec[idx++];
+		lseek(fd, trace_sec->offset, SEEK_SET);
 		trace_report(fd);
-		lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
 	}
 
 	if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
-		struct stat input_stat;
+		struct perf_file_section *buildid_sec;
 
-		fstat(fd, &input_stat);
-		if (perf_header__read_build_ids(self, fd, input_stat.st_size))
+		buildid_sec = &feat_sec[idx++];
+		lseek(fd, buildid_sec->offset, SEEK_SET);
+		if (perf_header__read_build_ids(fd, buildid_sec->size))
 			pr_debug("failed to read buildids, continuing...\n");
 	}
+
+	free(feat_sec);
 };
 
 struct perf_header *perf_header__read(int fd)
diff --git a/tools/perf/util/include/linux/bitmap.h b/tools/perf/util/include/linux/bitmap.h
index 821c103..9450763 100644
--- a/tools/perf/util/include/linux/bitmap.h
+++ b/tools/perf/util/include/linux/bitmap.h
@@ -1,2 +1,3 @@
 #include "../../../../include/linux/bitmap.h"
 #include "../../../../include/asm-generic/bitops/find.h"
+#include <linux/errno.h>
diff --git a/tools/perf/util/include/linux/ctype.h b/tools/perf/util/include/linux/ctype.h
index bae5783..a53d4ee 100644
--- a/tools/perf/util/include/linux/ctype.h
+++ b/tools/perf/util/include/linux/ctype.h
@@ -1 +1 @@
-#include "../../../../include/linux/ctype.h"
+#include "../util.h"
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 0daa341..f2203a0 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -315,6 +315,7 @@ static inline int has_extension(const char *filename, const char *ext)
 #undef isascii
 #undef isspace
 #undef isdigit
+#undef isxdigit
 #undef isalpha
 #undef isprint
 #undef isalnum
@@ -332,6 +333,8 @@ extern unsigned char sane_ctype[256];
 #define isascii(x) (((x) & ~0x7f) == 0)
 #define isspace(x) sane_istest(x,GIT_SPACE)
 #define isdigit(x) sane_istest(x,GIT_DIGIT)
+#define isxdigit(x)	\
+	(sane_istest(toupper(x), GIT_ALPHA | GIT_DIGIT) && toupper(x) < 'G')
 #define isalpha(x) sane_istest(x,GIT_ALPHA)
 #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
 #define isprint(x) sane_istest(x,GIT_PRINT)
-- 
1.6.2.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] perf tools: Bring linear set of section headers for features
  2009-11-11  3:51 ` [PATCH 6/6] perf tools: Bring linear set of section headers for features Frederic Weisbecker
@ 2009-11-11  6:20   ` Peter Zijlstra
  2009-11-11 16:49     ` Frederic Weisbecker
  2009-11-11  7:44   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Zijlstra @ 2009-11-11  6:20 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Ingo Molnar, LKML, Arnaldo Carvalho de Melo, Mike Galbraith,
	Paul Mackerras, Hitoshi Mitake

On Wed, 2009-11-11 at 04:51 +0100, Frederic Weisbecker wrote:
> Build a set of section headers for features right after the datas.
> Each implemented feature will have one of such section header that
> provides the offset and the size of the data manipulated by the feature.
> 
> The trace informations have moved after the data and are recorded
> on exit time.
> 
> The new layout is as follows:
> 
>  -----------------------
>                              ___
>  [ magic               ]      |
>  [ header size         ]      |
>  [ attr size           ]      |
>  [ attr content offset ]      |
>  [ attr content size   ]      |
>  [ data offset         ]  File Headers
>  [ data size           ]      |
>  [ event_types offset  ]      |
>  [ event_types size    ]      |
>  [ feature bitmap      ]      v 
> 
>  [ attr section        ]
>  [ events section      ]
> 
>                              ___
>  [         X           ]      |
>  [         X           ]      |
>  [         X           ]    Datas
>  [         X           ]      |
>  [         X           ]      v
> 
>                              ___
>  [ Feature 1 offset    ]      |
>  [ Feature 1 size      ] Features headers
>  [ Feature 2 offset    ]      |
>  [ Feature 2 size      ]      v
> 
>  [ Feature 1 content   ]
>  [ Feature 2 content   ]
>  -----------------------
> 
> We have as many feature's section headers as we have features in use for
> the current file.
> 
> Say Feat 1 and Feat 3 are used by the file, but not Feat 2. Then the
> feature headers will be like follows:
> 
> [ Feature 1 offset    ]      |
> [ Feature 1 size      ] Features headers
> [ Feature 3 offset    ]      |
> [ Feature 3 size      ]      v
> 
> There is no hole to cover Feature 2 that is not in use here. We only
> need to cover the needed headers in order, from the lowest feature bit
> to the highest.
> 
> Currently we have two features: HEADER_TRACE_INFO and HEADER_BUILD_ID.
> Both have their contents that follow the feature headers. Putting the
> contents right after the feature headers is not mandatory though.
> While we keep the feature headers right after the data and in order,
> their offsets can point everywhere. We have just put the two above
> feature contents in the end of the file for convenience.
> 
> The purpose of this layout change is to have a file format that scales
> while keeping it simple: having such linear feature headers is less
> error prone wrt forward/backward compatibility as the content of a
> feature can be put anywhere, its location can even change by the
> time, it's fine because its headers will tell where it is. And we know
> how to find these headers, following the above rules.

Does do mess up append though... be sure to add some magic for that.

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [tip:perf/core] perf tools: Synthetize the targeted process
  2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
                   ` (4 preceding siblings ...)
  2009-11-11  3:51 ` [PATCH 6/6] perf tools: Bring linear set of section headers for features Frederic Weisbecker
@ 2009-11-11  7:42 ` tip-bot for Frederic Weisbecker
  5 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2009-11-11  7:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, acme, hpa, mingo, efault, peterz, mitake,
	fweisbec, tglx, mingo

Commit-ID:  de8967214d8ce536161a1ad6538ad1cb82e7428d
Gitweb:     http://git.kernel.org/tip/de8967214d8ce536161a1ad6538ad1cb82e7428d
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Wed, 11 Nov 2009 04:51:02 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 11 Nov 2009 07:30:17 +0100

perf tools: Synthetize the targeted process

Don't forget to also synthetize the targeted process from perf
record or we'll miss its dso in the events and then we won't be
able to deal with its build-id.

We are missing it because it is created after the existing
synthetized tasks but before the counters are enabled and can
send its mapping event.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
LKML-Reference: <1257911467-28276-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-record.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ab33381..9f98b86 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -497,13 +497,22 @@ static int __cmd_record(int argc, const char **argv)
 	if (target_pid == -1 && argc) {
 		pid = fork();
 		if (pid < 0)
-			perror("failed to fork");
+			die("failed to fork");
 
 		if (!pid) {
 			if (execvp(argv[0], (char **)argv)) {
 				perror(argv[0]);
 				exit(-1);
 			}
+		} else {
+			/*
+			 * Wait a bit for the execv'ed child to appear
+			 * and be updated in /proc
+			 * FIXME: Do you know a less heuristical solution?
+			 */
+			usleep(1000);
+			event__synthesize_thread(pid,
+						 process_synthesized_event);
 		}
 
 		child_pid = pid;

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [tip:perf/core] perf tools: Move the build-id storage operations to headers
  2009-11-11  3:51 ` [PATCH 2/6] perf tools: Move the build-id storage operations to headers Frederic Weisbecker
@ 2009-11-11  7:43   ` tip-bot for Frederic Weisbecker
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2009-11-11  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, acme, hpa, mingo, efault, peterz, mitake,
	fweisbec, tglx, mingo

Commit-ID:  8671dab9d5b2f0b444b8d09792384dccbfd43d14
Gitweb:     http://git.kernel.org/tip/8671dab9d5b2f0b444b8d09792384dccbfd43d14
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Wed, 11 Nov 2009 04:51:03 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 11 Nov 2009 07:30:17 +0100

perf tools: Move the build-id storage operations to headers

So that it makes easier to control it. Especially because we
plan to give it a feature section.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
LKML-Reference: <1257911467-28276-2-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-record.c |   32 ++------------------------------
 tools/perf/util/header.c    |   41 ++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/header.h    |    2 +-
 3 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 9f98b86..c35e61b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -378,39 +378,11 @@ static void open_counters(int cpu, pid_t pid)
 	nr_cpu++;
 }
 
-static bool write_buildid_table(void)
-{
-	struct dso *pos;
-	bool have_buildid = false;
-
-	list_for_each_entry(pos, &dsos, node) {
-		struct build_id_event b;
-		size_t len;
-
-		if (filename__read_build_id(pos->long_name,
-					    &b.build_id,
-					    sizeof(b.build_id)) < 0)
-			continue;
-		have_buildid = true;
-		memset(&b.header, 0, sizeof(b.header));
-		len = strlen(pos->long_name) + 1;
-		len = ALIGN(len, 64);
-		b.header.size = sizeof(b) + len;
-		write_output(&b, sizeof(b));
-		write_output(pos->long_name, len);
-	}
-
-	return have_buildid;
-}
-
 static void atexit_header(void)
 {
 	header->data_size += bytes_written;
 
-	if (write_buildid_table())
-		perf_header__set_feat(header, HEADER_BUILD_ID);
-
-	perf_header__write(header, output);
+	perf_header__write(header, output, true);
 }
 
 static int __cmd_record(int argc, const char **argv)
@@ -487,7 +459,7 @@ static int __cmd_record(int argc, const char **argv)
 	}
 
 	if (file_new)
-		perf_header__write(header, output);
+		perf_header__write(header, output, false);
 
 	if (!system_wide)
 		event__synthesize_thread(pid, process_synthesized_event);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 050f543..a4d0bbe 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2,11 +2,13 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <linux/list.h>
 
 #include "util.h"
 #include "header.h"
 #include "../perf.h"
 #include "trace-event.h"
+#include "symbol.h"
 
 /*
  * Create new perf.data header attribute:
@@ -172,7 +174,33 @@ static void do_write(int fd, void *buf, size_t size)
 	}
 }
 
-static void perf_header__adds_write(struct perf_header *self, int fd)
+static bool write_buildid_table(int fd)
+{
+	struct dso *pos;
+	bool have_buildid = false;
+
+	list_for_each_entry(pos, &dsos, node) {
+		struct build_id_event b;
+		size_t len;
+
+		if (filename__read_build_id(pos->long_name,
+					    &b.build_id,
+					    sizeof(b.build_id)) < 0)
+			continue;
+		have_buildid = true;
+		memset(&b.header, 0, sizeof(b.header));
+		len = strlen(pos->long_name) + 1;
+		len = ALIGN(len, 64);
+		b.header.size = sizeof(b) + len;
+		do_write(fd, &b, sizeof(b));
+		do_write(fd, pos->long_name, len);
+	}
+
+	return have_buildid;
+}
+
+static void
+perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
 {
 	struct perf_file_section trace_sec;
 	u64 cur_offset = lseek(fd, 0, SEEK_CUR);
@@ -196,9 +224,16 @@ static void perf_header__adds_write(struct perf_header *self, int fd)
 		 */
 		cur_offset = lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
 	}
+
+	if (at_exit) {
+		lseek(fd, self->data_offset + self->data_size, SEEK_SET);
+		if (write_buildid_table(fd))
+			perf_header__set_feat(self, HEADER_BUILD_ID);
+		lseek(fd, cur_offset, SEEK_SET);
+	}
 };
 
-void perf_header__write(struct perf_header *self, int fd)
+void perf_header__write(struct perf_header *self, int fd, bool at_exit)
 {
 	struct perf_file_header f_header;
 	struct perf_file_attr   f_attr;
@@ -236,7 +271,7 @@ void perf_header__write(struct perf_header *self, int fd)
 	if (events)
 		do_write(fd, events, self->event_size);
 
-	perf_header__adds_write(self, fd);
+	perf_header__adds_write(self, fd, at_exit);
 
 	self->data_offset = lseek(fd, 0, SEEK_CUR);
 
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 2f233c5..77186c9 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -33,7 +33,7 @@ struct perf_header {
 };
 
 struct perf_header *perf_header__read(int fd);
-void perf_header__write(struct perf_header *self, int fd);
+void perf_header__write(struct perf_header *self, int fd, bool at_exit);
 
 void perf_header__add_attr(struct perf_header *self,
 			   struct perf_header_attr *attr);

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [tip:perf/core] perf tools: Split up build id saving into fetch and write
  2009-11-11  3:51 ` [PATCH 3/6] perf tools: Split up build id saving into fetch and write Frederic Weisbecker
@ 2009-11-11  7:43   ` tip-bot for Frederic Weisbecker
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2009-11-11  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, acme, hpa, mingo, efault, peterz, mitake,
	fweisbec, tglx, mingo

Commit-ID:  57f395a7eabb913d3605d7392be5bdb0837c9f3d
Gitweb:     http://git.kernel.org/tip/57f395a7eabb913d3605d7392be5bdb0837c9f3d
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Wed, 11 Nov 2009 04:51:04 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 11 Nov 2009 07:30:18 +0100

perf tools: Split up build id saving into fetch and write

We are saving the build id once we stop the profiling. And only
after doing that we know if we need to set that feature in the
header through the feature bitmap.

But if we want a proper feature support in the headers, using a
rule of offset/size pairs in sections, we need to know in
advance how many features we need to set in the headers, so that
we can reserve rooms for their section headers.

The current state doesn't allow that, as it forces us to first
save the build-ids to the file right after the datas instead of
planning any structured layout.

That's why this splits up the build-ids processing in two parts:
one that fetches the build-ids from the Dso objects, and one
that saves them into the file.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
LKML-Reference: <1257911467-28276-3-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/util/event.h  |    7 +++++++
 tools/perf/util/header.c |   41 +++++++++++++++++------------------------
 tools/perf/util/symbol.c |   34 ++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h |    1 +
 4 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 34c6fcb..1f771ce 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -69,6 +69,13 @@ struct build_id_event {
 	char			 filename[];
 };
 
+struct build_id_list {
+	struct build_id_event	event;
+	struct list_head	list;
+	const char		*dso_name;
+	int			len;
+};
+
 typedef union event_union {
 	struct perf_event_header	header;
 	struct ip_event			ip;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a4d0bbe..2f702c2 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -174,29 +174,18 @@ static void do_write(int fd, void *buf, size_t size)
 	}
 }
 
-static bool write_buildid_table(int fd)
+static void write_buildid_table(int fd, struct list_head *id_head)
 {
-	struct dso *pos;
-	bool have_buildid = false;
-
-	list_for_each_entry(pos, &dsos, node) {
-		struct build_id_event b;
-		size_t len;
-
-		if (filename__read_build_id(pos->long_name,
-					    &b.build_id,
-					    sizeof(b.build_id)) < 0)
-			continue;
-		have_buildid = true;
-		memset(&b.header, 0, sizeof(b.header));
-		len = strlen(pos->long_name) + 1;
-		len = ALIGN(len, 64);
-		b.header.size = sizeof(b) + len;
-		do_write(fd, &b, sizeof(b));
-		do_write(fd, pos->long_name, len);
-	}
+	struct build_id_list *iter, *next;
+
+	list_for_each_entry_safe(iter, next, id_head, list) {
+		struct build_id_event *b = &iter->event;
 
-	return have_buildid;
+		do_write(fd, b, sizeof(*b));
+		do_write(fd, (void *)iter->dso_name, iter->len);
+		list_del(&iter->list);
+		free(iter);
+	}
 }
 
 static void
@@ -226,10 +215,14 @@ perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
 	}
 
 	if (at_exit) {
-		lseek(fd, self->data_offset + self->data_size, SEEK_SET);
-		if (write_buildid_table(fd))
+		LIST_HEAD(id_list);
+
+		if (fetch_build_id_table(&id_list)) {
+			lseek(fd, self->data_offset + self->data_size, SEEK_SET);
 			perf_header__set_feat(self, HEADER_BUILD_ID);
-		lseek(fd, cur_offset, SEEK_SET);
+			write_buildid_table(fd, &id_list);
+			lseek(fd, cur_offset, SEEK_SET);
+		}
 	}
 };
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a2e95ce..9c286db 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -851,6 +851,40 @@ out_close:
 	return err;
 }
 
+bool fetch_build_id_table(struct list_head *head)
+{
+	bool have_buildid = false;
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node) {
+		struct build_id_list *new;
+		struct build_id_event b;
+		size_t len;
+
+		if (filename__read_build_id(pos->long_name,
+					    &b.build_id,
+					    sizeof(b.build_id)) < 0)
+			continue;
+		have_buildid = true;
+		memset(&b.header, 0, sizeof(b.header));
+		len = strlen(pos->long_name) + 1;
+		len = ALIGN(len, 64);
+		b.header.size = sizeof(b) + len;
+
+		new = malloc(sizeof(*new));
+		if (!new)
+			die("No memory\n");
+
+		memcpy(&new->event, &b, sizeof(b));
+		new->dso_name = pos->long_name;
+		new->len = len;
+
+		list_add_tail(&new->list, head);
+	}
+
+	return have_buildid;
+}
+
 int filename__read_build_id(const char *filename, void *bf, size_t size)
 {
 	int fd, err = -1;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f8c1899..0a34a54 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -86,6 +86,7 @@ char dso__symtab_origin(const struct dso *self);
 void dso__set_build_id(struct dso *self, void *build_id);
 
 int filename__read_build_id(const char *filename, void *bf, size_t size);
+bool fetch_build_id_table(struct list_head *head);
 int build_id__sprintf(u8 *self, int len, char *bf);
 
 int load_kernel(symbol_filter_t filter);

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [tip:perf/core] perf tools: Read the build-ids from the header layer
  2009-11-11  3:51 ` [PATCH 4/6] perf tools: Read the build-ids from the header layer Frederic Weisbecker
@ 2009-11-11  7:43   ` tip-bot for Frederic Weisbecker
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2009-11-11  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, acme, hpa, mingo, efault, peterz, mitake,
	fweisbec, tglx, mingo

Commit-ID:  4778d2e4f410c6eea32f594cb2be9590bcb28b84
Gitweb:     http://git.kernel.org/tip/4778d2e4f410c6eea32f594cb2be9590bcb28b84
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Wed, 11 Nov 2009 04:51:05 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 11 Nov 2009 07:30:18 +0100

perf tools: Read the build-ids from the header layer

Keep the build-ids reading implementation in the data mapping
but move its call to the headers so that we have a better
control on it (offset seeking, size passing, etc..).

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
LKML-Reference: <1257911467-28276-4-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/util/data_map.c |    8 ++------
 tools/perf/util/data_map.h |    2 ++
 tools/perf/util/header.c   |   14 ++++++++++++--
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/data_map.c b/tools/perf/util/data_map.c
index 00a9c11..66e58aa 100644
--- a/tools/perf/util/data_map.c
+++ b/tools/perf/util/data_map.c
@@ -70,8 +70,8 @@ process_event(event_t *event, unsigned long offset, unsigned long head)
 	}
 }
 
-static int perf_header__read_build_ids(const struct perf_header *self,
-				       int input, off_t file_size)
+int perf_header__read_build_ids(const struct perf_header *self,
+				int input, off_t file_size)
 {
 	off_t offset = self->data_offset + self->data_size;
 	struct build_id_event bev;
@@ -163,10 +163,6 @@ int mmap_dispatch_perf_file(struct perf_header **pheader,
 		if (curr_handler->sample_type_check(sample_type) < 0)
 			exit(-1);
 
-	if (perf_header__has_feat(header, HEADER_BUILD_ID) &&
-	    perf_header__read_build_ids(header, input, input_stat.st_size))
-		pr_debug("failed to read buildids, continuing...\n");
-
 	if (load_kernel(NULL) < 0) {
 		perror("failed to load kernel symbols");
 		return EXIT_FAILURE;
diff --git a/tools/perf/util/data_map.h b/tools/perf/util/data_map.h
index 716d105..c412281 100644
--- a/tools/perf/util/data_map.h
+++ b/tools/perf/util/data_map.h
@@ -27,5 +27,7 @@ int mmap_dispatch_perf_file(struct perf_header **pheader,
 			    int full_paths,
 			    int *cwdlen,
 			    char **cwd);
+int perf_header__read_build_ids(const struct perf_header *self,
+				int input, off_t file_size);
 
 #endif
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2f702c2..915b56e 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -9,6 +9,8 @@
 #include "../perf.h"
 #include "trace-event.h"
 #include "symbol.h"
+#include "data_map.h"
+#include "debug.h"
 
 /*
  * Create new perf.data header attribute:
@@ -322,6 +324,14 @@ static void perf_header__adds_read(struct perf_header *self, int fd)
 		trace_report(fd);
 		lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
 	}
+
+	if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
+		struct stat input_stat;
+
+		fstat(fd, &input_stat);
+		if (perf_header__read_build_ids(self, fd, input_stat.st_size))
+			pr_debug("failed to read buildids, continuing...\n");
+	}
 };
 
 struct perf_header *perf_header__read(int fd)
@@ -382,14 +392,14 @@ struct perf_header *perf_header__read(int fd)
 
 	memcpy(&self->adds_features, &f_header.adds_features, sizeof(f_header.adds_features));
 
-	perf_header__adds_read(self, fd);
-
 	self->event_offset = f_header.event_types.offset;
 	self->event_size   = f_header.event_types.size;
 
 	self->data_offset = f_header.data.offset;
 	self->data_size   = f_header.data.size;
 
+	perf_header__adds_read(self, fd);
+
 	lseek(fd, self->data_offset, SEEK_SET);
 
 	self->frozen = 1;

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [tip:perf/core] perf tools: Use perf_header__set/has_feat whenever possible
  2009-11-11  3:51 ` [PATCH 5/6] perf tools: Use perf_header__set/has_feat whenever possible Frederic Weisbecker
@ 2009-11-11  7:43   ` tip-bot for Frederic Weisbecker
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2009-11-11  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, acme, hpa, mingo, efault, peterz, mitake,
	fweisbec, tglx, mingo

Commit-ID:  3e13ab2d83b6867a20663c73c184f29c2fde1558
Gitweb:     http://git.kernel.org/tip/3e13ab2d83b6867a20663c73c184f29c2fde1558
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Wed, 11 Nov 2009 04:51:06 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 11 Nov 2009 07:30:19 +0100

perf tools: Use perf_header__set/has_feat whenever possible

And drop the alternate checks/sets using set_bit or other kind
of helpers.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
LKML-Reference: <1257911467-28276-5-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-record.c |    4 ++--
 tools/perf/util/header.c    |   12 ++----------
 tools/perf/util/header.h    |    1 -
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index c35e61b..326e8a7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -431,11 +431,11 @@ static int __cmd_record(int argc, const char **argv)
 		header = perf_header__new();
 
 	if (raw_samples) {
-		perf_header__feat_trace_info(header);
+		perf_header__set_feat(header, HEADER_TRACE_INFO);
 	} else {
 		for (i = 0; i < nr_counters; i++) {
 			if (attrs[i].sample_type & PERF_SAMPLE_RAW) {
-				perf_header__feat_trace_info(header);
+				perf_header__set_feat(header, HEADER_TRACE_INFO);
 				break;
 			}
 		}
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 915b56e..9709d38 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -148,11 +148,6 @@ struct perf_file_header {
 	DECLARE_BITMAP(adds_features, HEADER_FEAT_BITS);
 };
 
-void perf_header__feat_trace_info(struct perf_header *header)
-{
-	set_bit(HEADER_TRACE_INFO, header->adds_features);
-}
-
 void perf_header__set_feat(struct perf_header *self, int feat)
 {
 	set_bit(feat, self->adds_features);
@@ -195,9 +190,8 @@ perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
 {
 	struct perf_file_section trace_sec;
 	u64 cur_offset = lseek(fd, 0, SEEK_CUR);
-	unsigned long *feat_mask = self->adds_features;
 
-	if (test_bit(HEADER_TRACE_INFO, feat_mask)) {
+	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
 		/* Write trace info */
 		trace_sec.offset = lseek(fd, sizeof(trace_sec), SEEK_CUR);
 		read_tracing_data(fd, attrs, nr_counters);
@@ -314,9 +308,7 @@ static void do_read(int fd, void *buf, size_t size)
 
 static void perf_header__adds_read(struct perf_header *self, int fd)
 {
-	const unsigned long *feat_mask = self->adds_features;
-
-	if (test_bit(HEADER_TRACE_INFO, feat_mask)) {
+	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
 		struct perf_file_section trace_sec;
 
 		do_read(fd, &trace_sec, sizeof(trace_sec));
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 77186c9..a22d70b 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -49,7 +49,6 @@ void perf_header_attr__add_id(struct perf_header_attr *self, u64 id);
 u64 perf_header__sample_type(struct perf_header *header);
 struct perf_event_attr *
 perf_header__find_attr(u64 id, struct perf_header *header);
-void perf_header__feat_trace_info(struct perf_header *header);
 void perf_header__set_feat(struct perf_header *self, int feat);
 bool perf_header__has_feat(const struct perf_header *self, int feat);
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [tip:perf/core] perf tools: Bring linear set of section headers for features
  2009-11-11  3:51 ` [PATCH 6/6] perf tools: Bring linear set of section headers for features Frederic Weisbecker
  2009-11-11  6:20   ` Peter Zijlstra
@ 2009-11-11  7:44   ` tip-bot for Frederic Weisbecker
  1 sibling, 0 replies; 15+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2009-11-11  7:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, paulus, acme, hpa, mingo, efault, peterz, mitake,
	fweisbec, tglx, mingo

Commit-ID:  9e827dd00a94136b944a538bede67c944d0b740a
Gitweb:     http://git.kernel.org/tip/9e827dd00a94136b944a538bede67c944d0b740a
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Wed, 11 Nov 2009 04:51:07 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 11 Nov 2009 07:30:19 +0100

perf tools: Bring linear set of section headers for features

Build a set of section headers for features right after the
datas. Each implemented feature will have one of such section
header that provides the offset and the size of the data
manipulated by the feature.

The trace informations have moved after the data and are
recorded on exit time.

The new layout is as follows:

 -----------------------
                             ___
 [ magic               ]      |
 [ header size         ]      |
 [ attr size           ]      |
 [ attr content offset ]      |
 [ attr content size   ]      |
 [ data offset         ]  File Headers
 [ data size           ]      |
 [ event_types offset  ]      |
 [ event_types size    ]      |
 [ feature bitmap      ]      v

 [ attr section        ]
 [ events section      ]

                             ___
 [         X           ]      |
 [         X           ]      |
 [         X           ]    Datas
 [         X           ]      |
 [         X           ]      v

                             ___
 [ Feature 1 offset    ]      |
 [ Feature 1 size      ] Features headers
 [ Feature 2 offset    ]      |
 [ Feature 2 size      ]      v

 [ Feature 1 content   ]
 [ Feature 2 content   ]
 -----------------------

We have as many feature's section headers as we have features in
use for the current file.

Say Feat 1 and Feat 3 are used by the file, but not Feat 2. Then
the feature headers will be like follows:

[ Feature 1 offset    ]      |
[ Feature 1 size      ] Features headers
[ Feature 3 offset    ]      |
[ Feature 3 size      ]      v

There is no hole to cover Feature 2 that is not in use here. We
only need to cover the needed headers in order, from the lowest
feature bit to the highest.

Currently we have two features: HEADER_TRACE_INFO and
HEADER_BUILD_ID. Both have their contents that follow the
feature headers. Putting the contents right after the feature
headers is not mandatory though. While we keep the feature
headers right after the data and in order, their offsets can
point everywhere. We have just put the two above feature
contents in the end of the file for convenience.

The purpose of this layout change is to have a file format that
scales while keeping it simple: having such linear feature
headers is less error prone wrt forward/backward compatibility
as the content of a feature can be put anywhere, its location
can even change by the time, it's fine because its headers will
tell where it is. And we know how to find these headers,
following the above rules.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
LKML-Reference: <1257911467-28276-6-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/util/data_map.c             |   11 +--
 tools/perf/util/data_map.h             |    3 +-
 tools/perf/util/header.c               |  110 ++++++++++++++++++++++----------
 tools/perf/util/include/linux/bitmap.h |    1 +
 tools/perf/util/include/linux/ctype.h  |    2 +-
 tools/perf/util/util.h                 |    3 +
 6 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/tools/perf/util/data_map.c b/tools/perf/util/data_map.c
index 66e58aa..aacb814 100644
--- a/tools/perf/util/data_map.c
+++ b/tools/perf/util/data_map.c
@@ -70,18 +70,15 @@ process_event(event_t *event, unsigned long offset, unsigned long head)
 	}
 }
 
-int perf_header__read_build_ids(const struct perf_header *self,
-				int input, off_t file_size)
+int perf_header__read_build_ids(int input, off_t size)
 {
-	off_t offset = self->data_offset + self->data_size;
 	struct build_id_event bev;
 	char filename[PATH_MAX];
+	off_t offset = lseek(input, 0, SEEK_CUR);
+	off_t limit = offset + size;
 	int err = -1;
 
-	if (lseek(input, offset, SEEK_SET) < 0)
-		return -1;
-
-	while (offset < file_size) {
+	while (offset < limit) {
 		struct dso *dso;
 		ssize_t len;
 
diff --git a/tools/perf/util/data_map.h b/tools/perf/util/data_map.h
index c412281..20b4037 100644
--- a/tools/perf/util/data_map.h
+++ b/tools/perf/util/data_map.h
@@ -27,7 +27,6 @@ int mmap_dispatch_perf_file(struct perf_header **pheader,
 			    int full_paths,
 			    int *cwdlen,
 			    char **cwd);
-int perf_header__read_build_ids(const struct perf_header *self,
-				int input, off_t file_size);
+int perf_header__read_build_ids(int input, off_t file_size);
 
 #endif
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 9709d38..ebed4f4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -186,41 +186,58 @@ static void write_buildid_table(int fd, struct list_head *id_head)
 }
 
 static void
-perf_header__adds_write(struct perf_header *self, int fd, bool at_exit)
+perf_header__adds_write(struct perf_header *self, int fd)
 {
-	struct perf_file_section trace_sec;
-	u64 cur_offset = lseek(fd, 0, SEEK_CUR);
+	LIST_HEAD(id_list);
+	int nr_sections;
+	struct perf_file_section *feat_sec;
+	int sec_size;
+	u64 sec_start;
+	int idx = 0;
+
+	if (fetch_build_id_table(&id_list))
+		perf_header__set_feat(self, HEADER_BUILD_ID);
+
+	nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
+	if (!nr_sections)
+		return;
+
+	feat_sec = calloc(sizeof(*feat_sec), nr_sections);
+	if (!feat_sec)
+		die("No memory");
+
+	sec_size = sizeof(*feat_sec) * nr_sections;
+
+	sec_start = self->data_offset + self->data_size;
+	lseek(fd, sec_start + sec_size, SEEK_SET);
 
 	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
+		struct perf_file_section *trace_sec;
+
+		trace_sec = &feat_sec[idx++];
+
 		/* Write trace info */
-		trace_sec.offset = lseek(fd, sizeof(trace_sec), SEEK_CUR);
+		trace_sec->offset = lseek(fd, 0, SEEK_CUR);
 		read_tracing_data(fd, attrs, nr_counters);
-		trace_sec.size = lseek(fd, 0, SEEK_CUR) - trace_sec.offset;
-
-		/* Write trace info headers */
-		lseek(fd, cur_offset, SEEK_SET);
-		do_write(fd, &trace_sec, sizeof(trace_sec));
-
-		/*
-		 * Update cur_offset. So that other (future)
-		 * features can set their own infos in this place. But if we are
-		 * the only feature, at least that seeks to the place the data
-		 * should begin.
-		 */
-		cur_offset = lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
+		trace_sec->size = lseek(fd, 0, SEEK_CUR) - trace_sec->offset;
 	}
 
-	if (at_exit) {
-		LIST_HEAD(id_list);
 
-		if (fetch_build_id_table(&id_list)) {
-			lseek(fd, self->data_offset + self->data_size, SEEK_SET);
-			perf_header__set_feat(self, HEADER_BUILD_ID);
-			write_buildid_table(fd, &id_list);
-			lseek(fd, cur_offset, SEEK_SET);
-		}
+	if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
+		struct perf_file_section *buildid_sec;
+
+		buildid_sec = &feat_sec[idx++];
+
+		/* Write build-ids */
+		buildid_sec->offset = lseek(fd, 0, SEEK_CUR);
+		write_buildid_table(fd, &id_list);
+		buildid_sec->size = lseek(fd, 0, SEEK_CUR) - buildid_sec->offset;
 	}
-};
+
+	lseek(fd, sec_start, SEEK_SET);
+	do_write(fd, feat_sec, sec_size);
+	free(feat_sec);
+}
 
 void perf_header__write(struct perf_header *self, int fd, bool at_exit)
 {
@@ -260,10 +277,11 @@ void perf_header__write(struct perf_header *self, int fd, bool at_exit)
 	if (events)
 		do_write(fd, events, self->event_size);
 
-	perf_header__adds_write(self, fd, at_exit);
-
 	self->data_offset = lseek(fd, 0, SEEK_CUR);
 
+	if (at_exit)
+		perf_header__adds_write(self, fd);
+
 	f_header = (struct perf_file_header){
 		.magic	   = PERF_MAGIC,
 		.size	   = sizeof(f_header),
@@ -308,22 +326,44 @@ static void do_read(int fd, void *buf, size_t size)
 
 static void perf_header__adds_read(struct perf_header *self, int fd)
 {
+	struct perf_file_section *feat_sec;
+	int nr_sections;
+	int sec_size;
+	int idx = 0;
+
+
+	nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
+	if (!nr_sections)
+		return;
+
+	feat_sec = calloc(sizeof(*feat_sec), nr_sections);
+	if (!feat_sec)
+		die("No memory");
+
+	sec_size = sizeof(*feat_sec) * nr_sections;
+
+	lseek(fd, self->data_offset + self->data_size, SEEK_SET);
+
+	do_read(fd, feat_sec, sec_size);
+
 	if (perf_header__has_feat(self, HEADER_TRACE_INFO)) {
-		struct perf_file_section trace_sec;
+		struct perf_file_section *trace_sec;
 
-		do_read(fd, &trace_sec, sizeof(trace_sec));
-		lseek(fd, trace_sec.offset, SEEK_SET);
+		trace_sec = &feat_sec[idx++];
+		lseek(fd, trace_sec->offset, SEEK_SET);
 		trace_report(fd);
-		lseek(fd, trace_sec.offset + trace_sec.size, SEEK_SET);
 	}
 
 	if (perf_header__has_feat(self, HEADER_BUILD_ID)) {
-		struct stat input_stat;
+		struct perf_file_section *buildid_sec;
 
-		fstat(fd, &input_stat);
-		if (perf_header__read_build_ids(self, fd, input_stat.st_size))
+		buildid_sec = &feat_sec[idx++];
+		lseek(fd, buildid_sec->offset, SEEK_SET);
+		if (perf_header__read_build_ids(fd, buildid_sec->size))
 			pr_debug("failed to read buildids, continuing...\n");
 	}
+
+	free(feat_sec);
 };
 
 struct perf_header *perf_header__read(int fd)
diff --git a/tools/perf/util/include/linux/bitmap.h b/tools/perf/util/include/linux/bitmap.h
index 821c103..9450763 100644
--- a/tools/perf/util/include/linux/bitmap.h
+++ b/tools/perf/util/include/linux/bitmap.h
@@ -1,2 +1,3 @@
 #include "../../../../include/linux/bitmap.h"
 #include "../../../../include/asm-generic/bitops/find.h"
+#include <linux/errno.h>
diff --git a/tools/perf/util/include/linux/ctype.h b/tools/perf/util/include/linux/ctype.h
index bae5783..a53d4ee 100644
--- a/tools/perf/util/include/linux/ctype.h
+++ b/tools/perf/util/include/linux/ctype.h
@@ -1 +1 @@
-#include "../../../../include/linux/ctype.h"
+#include "../util.h"
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 9de2329..7bd5bda 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -306,6 +306,7 @@ static inline int has_extension(const char *filename, const char *ext)
 #undef isascii
 #undef isspace
 #undef isdigit
+#undef isxdigit
 #undef isalpha
 #undef isprint
 #undef isalnum
@@ -323,6 +324,8 @@ extern unsigned char sane_ctype[256];
 #define isascii(x) (((x) & ~0x7f) == 0)
 #define isspace(x) sane_istest(x,GIT_SPACE)
 #define isdigit(x) sane_istest(x,GIT_DIGIT)
+#define isxdigit(x)	\
+	(sane_istest(toupper(x), GIT_ALPHA | GIT_DIGIT) && toupper(x) < 'G')
 #define isalpha(x) sane_istest(x,GIT_ALPHA)
 #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
 #define isprint(x) sane_istest(x,GIT_PRINT)

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] perf tools: Bring linear set of section headers for features
  2009-11-11  6:20   ` Peter Zijlstra
@ 2009-11-11 16:49     ` Frederic Weisbecker
  2009-11-11 17:32       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 15+ messages in thread
From: Frederic Weisbecker @ 2009-11-11 16:49 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, LKML, Arnaldo Carvalho de Melo, Mike Galbraith,
	Paul Mackerras, Hitoshi Mitake

On Wed, Nov 11, 2009 at 07:20:04AM +0100, Peter Zijlstra wrote:
> On Wed, 2009-11-11 at 04:51 +0100, Frederic Weisbecker wrote:
> > Build a set of section headers for features right after the datas.
> > Each implemented feature will have one of such section header that
> > provides the offset and the size of the data manipulated by the feature.
> > 
> > The trace informations have moved after the data and are recorded
> > on exit time.
> > 
> > The new layout is as follows:
> > 
> >  -----------------------
> >                              ___
> >  [ magic               ]      |
> >  [ header size         ]      |
> >  [ attr size           ]      |
> >  [ attr content offset ]      |
> >  [ attr content size   ]      |
> >  [ data offset         ]  File Headers
> >  [ data size           ]      |
> >  [ event_types offset  ]      |
> >  [ event_types size    ]      |
> >  [ feature bitmap      ]      v 
> > 
> >  [ attr section        ]
> >  [ events section      ]
> > 
> >                              ___
> >  [         X           ]      |
> >  [         X           ]      |
> >  [         X           ]    Datas
> >  [         X           ]      |
> >  [         X           ]      v
> > 
> >                              ___
> >  [ Feature 1 offset    ]      |
> >  [ Feature 1 size      ] Features headers
> >  [ Feature 2 offset    ]      |
> >  [ Feature 2 size      ]      v
> > 
> >  [ Feature 1 content   ]
> >  [ Feature 2 content   ]
> >  -----------------------
> > 
> > We have as many feature's section headers as we have features in use for
> > the current file.
> > 
> > Say Feat 1 and Feat 3 are used by the file, but not Feat 2. Then the
> > feature headers will be like follows:
> > 
> > [ Feature 1 offset    ]      |
> > [ Feature 1 size      ] Features headers
> > [ Feature 3 offset    ]      |
> > [ Feature 3 size      ]      v
> > 
> > There is no hole to cover Feature 2 that is not in use here. We only
> > need to cover the needed headers in order, from the lowest feature bit
> > to the highest.
> > 
> > Currently we have two features: HEADER_TRACE_INFO and HEADER_BUILD_ID.
> > Both have their contents that follow the feature headers. Putting the
> > contents right after the feature headers is not mandatory though.
> > While we keep the feature headers right after the data and in order,
> > their offsets can point everywhere. We have just put the two above
> > feature contents in the end of the file for convenience.
> > 
> > The purpose of this layout change is to have a file format that scales
> > while keeping it simple: having such linear feature headers is less
> > error prone wrt forward/backward compatibility as the content of a
> > feature can be put anywhere, its location can even change by the
> > time, it's fine because its headers will tell where it is. And we know
> > how to find these headers, following the above rules.
> 
> Does do mess up append though... be sure to add some magic for that.


Yep. It seems to work. What happens is that trace information are
re-computed. The same goes for build id I guess.

What happens is that the appended datas override the features sections
and headers, but this is fine because we rewrite them in the end.

But these both need to support appending mode internally (ie: append
the new informtions on top of exisiting ones instead of redo the whole)
and then rewrite the whole at exit like it's done currently.


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] perf tools: Bring linear set of section headers for features
  2009-11-11 16:49     ` Frederic Weisbecker
@ 2009-11-11 17:32       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-11 17:32 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Peter Zijlstra, Ingo Molnar, LKML, Mike Galbraith,
	Paul Mackerras, Hitoshi Mitake

Em Wed, Nov 11, 2009 at 05:49:37PM +0100, Frederic Weisbecker escreveu:
> On Wed, Nov 11, 2009 at 07:20:04AM +0100, Peter Zijlstra wrote:
> > On Wed, 2009-11-11 at 04:51 +0100, Frederic Weisbecker wrote:
> > > Build a set of section headers for features right after the datas.
> > > Each implemented feature will have one of such section header that
> > > provides the offset and the size of the data manipulated by the feature.
> > > 
> > > The trace informations have moved after the data and are recorded
> > > on exit time.
> > > 
> > > The new layout is as follows:
> > > 
> > >  -----------------------
> > >                              ___
> > >  [ magic               ]      |
> > >  [ header size         ]      |
> > >  [ attr size           ]      |
> > >  [ attr content offset ]      |
> > >  [ attr content size   ]      |
> > >  [ data offset         ]  File Headers
> > >  [ data size           ]      |
> > >  [ event_types offset  ]      |
> > >  [ event_types size    ]      |
> > >  [ feature bitmap      ]      v 
> > > 
> > >  [ attr section        ]
> > >  [ events section      ]
> > > 
> > >                              ___
> > >  [         X           ]      |
> > >  [         X           ]      |
> > >  [         X           ]    Datas
> > >  [         X           ]      |
> > >  [         X           ]      v
> > > 
> > >                              ___
> > >  [ Feature 1 offset    ]      |
> > >  [ Feature 1 size      ] Features headers
> > >  [ Feature 2 offset    ]      |
> > >  [ Feature 2 size      ]      v
> > > 
> > >  [ Feature 1 content   ]
> > >  [ Feature 2 content   ]
> > >  -----------------------
> > > 
> > > We have as many feature's section headers as we have features in use for
> > > the current file.
> > > 
> > > Say Feat 1 and Feat 3 are used by the file, but not Feat 2. Then the
> > > feature headers will be like follows:
> > > 
> > > [ Feature 1 offset    ]      |
> > > [ Feature 1 size      ] Features headers
> > > [ Feature 3 offset    ]      |
> > > [ Feature 3 size      ]      v
> > > 
> > > There is no hole to cover Feature 2 that is not in use here. We only
> > > need to cover the needed headers in order, from the lowest feature bit
> > > to the highest.
> > > 
> > > Currently we have two features: HEADER_TRACE_INFO and HEADER_BUILD_ID.
> > > Both have their contents that follow the feature headers. Putting the
> > > contents right after the feature headers is not mandatory though.
> > > While we keep the feature headers right after the data and in order,
> > > their offsets can point everywhere. We have just put the two above
> > > feature contents in the end of the file for convenience.
> > > 
> > > The purpose of this layout change is to have a file format that scales
> > > while keeping it simple: having such linear feature headers is less
> > > error prone wrt forward/backward compatibility as the content of a
> > > feature can be put anywhere, its location can even change by the
> > > time, it's fine because its headers will tell where it is. And we know
> > > how to find these headers, following the above rules.
> > 
> > Does do mess up append though... be sure to add some magic for that.
> 
> Yep. It seems to work. What happens is that trace information are
> re-computed. The same goes for build id I guess.

Well, we would need to get the existing buildids, insert them into the
dsos list, and as we use dsos__findnew DSOs already in the list wouldn't
be added, and at the end we would, as you say, recompute the buildid
table.
 
> What happens is that the appended datas override the features sections
> and headers, but this is fine because we rewrite them in the end.
> 
> But these both need to support appending mode internally (ie: append
> the new informtions on top of exisiting ones instead of redo the whole)
> and then rewrite the whole at exit like it's done currently.

yeah.

- Arnado

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2009-11-11 17:32 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-11  3:51 [PATCH 1/6] perf tools: Synthetize the targeted process Frederic Weisbecker
2009-11-11  3:51 ` [PATCH 2/6] perf tools: Move the build-id storage operations to headers Frederic Weisbecker
2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
2009-11-11  3:51 ` [PATCH 3/6] perf tools: Split up build id saving into fetch and write Frederic Weisbecker
2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
2009-11-11  3:51 ` [PATCH 4/6] perf tools: Read the build-ids from the header layer Frederic Weisbecker
2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
2009-11-11  3:51 ` [PATCH 5/6] perf tools: Use perf_header__set/has_feat whenever possible Frederic Weisbecker
2009-11-11  7:43   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
2009-11-11  3:51 ` [PATCH 6/6] perf tools: Bring linear set of section headers for features Frederic Weisbecker
2009-11-11  6:20   ` Peter Zijlstra
2009-11-11 16:49     ` Frederic Weisbecker
2009-11-11 17:32       ` Arnaldo Carvalho de Melo
2009-11-11  7:44   ` [tip:perf/core] " tip-bot for Frederic Weisbecker
2009-11-11  7:42 ` [tip:perf/core] perf tools: Synthetize the targeted process tip-bot for Frederic Weisbecker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.