[PATCH 1/3] perf: Fix timechart header handling

[PATCH 1/3] perf: Fix timechart header handling

[OGAWA Hirofumi]

Hi,

Update "struct trace_entry" to match with current one. And remove
"size" field from it.

If it has "size", it become cause of alignment mismatch of structure
with kernel.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 tools/perf/builtin-timechart.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff -puN tools/perf/builtin-timechart.c~perf-fix-timechart-header-handling tools/perf/builtin-timechart.c
--- linux-2.6/tools/perf/builtin-timechart.c~perf-fix-timechart-header-handling	2009-12-06 19:27:33.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-timechart.c	2009-12-06 19:27:44.000000000 +0900
@@ -302,12 +302,11 @@ process_exit_event(event_t *event)
 }
 
 struct trace_entry {
-	u32			size;
 	unsigned short		type;
 	unsigned char		flags;
 	unsigned char		preempt_count;
 	int			pid;
-	int			tgid;
+	int			lock_depth;
 };
 
 struct power_entry {
@@ -489,6 +488,7 @@ process_sample_event(event_t *event)
 	u64 stamp = 0;
 	u32 cpu = 0;
 	u32 pid = 0;
+	u32 size, *size_ptr;
 	struct trace_entry *te;
 
 	if (sample_type & PERF_SAMPLE_IP)
@@ -518,9 +518,13 @@ process_sample_event(event_t *event)
 	if (sample_type & PERF_SAMPLE_PERIOD)
 		cursor++;
 
-	te = (void *)&event->sample.array[cursor];
+	size_ptr = (void *)&event->sample.array[cursor];
 
-	if (sample_type & PERF_SAMPLE_RAW && te->size > 0) {
+	size = *size_ptr;
+	size_ptr++;
+
+	te = (void *)size_ptr;
+	if (sample_type & PERF_SAMPLE_RAW && size > 0) {
 		char *event_str;
 		struct power_entry *pe;
 
_

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[PATCH 2/3] perf: make common SAMPLE_EVENT parser

[OGAWA Hirofumi]

Currently, sample event data is parsed for each commands, and it is
assuming that the data is not including other data. (E.g. timechart,
trace, etc. can't parse the event if it has PERF_SAMPLE_CALLCHAIN)

So, even if we record the superset data for multiple commands at a
time, commands can't parse. etc.

To fix it, this makes common sample event parser, and use it to parse
sample event correctly. (PERF_SAMPLE_READ is unsupported for now
though, it seems to be not using.)

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 tools/perf/builtin-kmem.c      |   36 +++++++--------------
 tools/perf/builtin-report.c    |   39 ++++++++++-------------
 tools/perf/builtin-sched.c     |   38 +++++++---------------
 tools/perf/builtin-timechart.c |   56 ++++++++-------------------------
 tools/perf/builtin-trace.c     |   48 ++++++++--------------------
 tools/perf/util/event.c        |   67 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/event.h        |   17 +++++++++-
 7 files changed, 155 insertions(+), 146 deletions(-)

diff -puN tools/perf/util/event.h~perf-make-common-sample tools/perf/util/event.h
--- linux-2.6/tools/perf/util/event.h~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/event.h	2009-12-06 19:42:51.000000000 +0900
@@ -56,11 +56,25 @@ struct read_event {
 	u64 id;
 };
 
-struct sample_event{
+struct sample_event {
 	struct perf_event_header        header;
 	u64 array[];
 };
 
+struct sample_data {
+	u64 ip;
+	u32 pid, tid;
+	u64 time;
+	u64 addr;
+	u64 id;
+	u64 stream_id;
+	u32 cpu;
+	u64 period;
+	struct ip_callchain *callchain;
+	u32 raw_size;
+	void *raw_data;
+};
+
 #define BUILD_ID_SIZE 20
 
 struct build_id_event {
@@ -155,5 +169,6 @@ int event__process_task(event_t *self);
 struct addr_location;
 int event__preprocess_sample(const event_t *self, struct addr_location *al,
 			     symbol_filter_t filter);
+int event__parse_sample(event_t *event, u64 type, struct sample_data *data);
 
 #endif /* __PERF_RECORD_H */
diff -puN tools/perf/builtin-report.c~perf-make-common-sample tools/perf/builtin-report.c
--- linux-2.6/tools/perf/builtin-report.c~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-report.c	2009-12-06 19:42:51.000000000 +0900
@@ -605,44 +605,41 @@ static int validate_chain(struct ip_call
 
 static int process_sample_event(event_t *event)
 {
-	u64 ip = event->ip.ip;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
-	struct ip_callchain *chain = NULL;
+	struct sample_data data;
 	int cpumode;
 	struct addr_location al;
-	struct thread *thread = threads__findnew(event->ip.pid);
+	struct thread *thread;
 
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	memset(&data, 0, sizeof(data));
+	data.period = 1;
+
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
 	if (sample_type & PERF_SAMPLE_CALLCHAIN) {
 		unsigned int i;
 
-		chain = (void *)more_data;
-
-		dump_printf("... chain: nr:%Lu\n", chain->nr);
+		dump_printf("... chain: nr:%Lu\n", data.callchain->nr);
 
-		if (validate_chain(chain, event) < 0) {
+		if (validate_chain(data.callchain, event) < 0) {
 			pr_debug("call-chain problem with event, "
 				 "skipping it.\n");
 			return 0;
 		}
 
 		if (dump_trace) {
-			for (i = 0; i < chain->nr; i++)
-				dump_printf("..... %2d: %016Lx\n", i, chain->ips[i]);
+			for (i = 0; i < data.callchain->nr; i++)
+				dump_printf("..... %2d: %016Lx\n",
+					    i, data.callchain->ips[i]);
 		}
 	}
 
+	thread = threads__findnew(data.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			event->header.type);
@@ -657,7 +654,7 @@ static int process_sample_event(event_t 
 	cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
 
 	thread__find_addr_location(thread, cpumode,
-				   MAP__FUNCTION, ip, &al, NULL);
+				   MAP__FUNCTION, data.ip, &al, NULL);
 	/*
 	 * We have to do this here as we may have a dso with no symbol hit that
 	 * has a name longer than the ones with symbols sampled.
@@ -675,12 +672,12 @@ static int process_sample_event(event_t 
 	if (sym_list && al.sym && !strlist__has_entry(sym_list, al.sym->name))
 		return 0;
 
-	if (hist_entry__add(&al, chain, period)) {
+	if (hist_entry__add(&al, data.callchain, data.period)) {
 		pr_debug("problem incrementing symbol count, skipping event\n");
 		return -1;
 	}
 
-	event__stats.total += period;
+	event__stats.total += data.period;
 
 	return 0;
 }
diff -puN tools/perf/util/event.c~perf-make-common-sample tools/perf/util/event.c
--- linux-2.6/tools/perf/util/event.c~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/event.c	2009-12-06 19:42:51.000000000 +0900
@@ -310,3 +310,70 @@ int event__preprocess_sample(const event
 			al->level == 'H' ? "[hypervisor]" : "<not found>");
 	return 0;
 }
+
+int event__parse_sample(event_t *event, u64 type, struct sample_data *data)
+{
+	u64 *array = event->sample.array;
+
+	if (type & PERF_SAMPLE_IP) {
+		data->ip = event->ip.ip;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_TID) {
+		u32 *p = (u32 *)array;
+		data->pid = p[0];
+		data->tid = p[1];
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_TIME) {
+		data->time = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_ADDR) {
+		data->addr = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_ID) {
+		data->id = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_STREAM_ID) {
+		data->stream_id = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_CPU) {
+		u32 *p = (u32 *)array;
+		data->cpu = *p;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_PERIOD) {
+		data->period = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_READ) {
+		pr_debug("PERF_SAMPLE_READ is unsuported for now\n");
+		return -1;
+	}
+
+	if (type & PERF_SAMPLE_CALLCHAIN) {
+		data->callchain = (struct ip_callchain *)array;
+		array += 1 + data->callchain->nr;
+	}
+
+	if (type & PERF_SAMPLE_RAW) {
+		u32 *p = (u32 *)array;
+		data->raw_size = *p;
+		p++;
+		data->raw_data = p;
+	}
+
+	return 0;
+}
diff -puN tools/perf/builtin-annotate.c~perf-make-common-sample tools/perf/builtin-annotate.c
diff -puN tools/perf/builtin-kmem.c~perf-make-common-sample tools/perf/builtin-kmem.c
--- linux-2.6/tools/perf/builtin-kmem.c~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-kmem.c	2009-12-06 19:42:51.000000000 +0900
@@ -320,35 +320,23 @@ process_raw_event(event_t *raw_event __u
 
 static int process_sample_event(event_t *event)
 {
-	u64 ip = event->ip.ip;
-	u64 timestamp = -1;
-	u32 cpu = -1;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
-	struct thread *thread = threads__findnew(event->ip.pid);
-
-	if (sample_type & PERF_SAMPLE_TIME) {
-		timestamp = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	struct sample_data data;
+	struct thread *thread;
 
-	if (sample_type & PERF_SAMPLE_CPU) {
-		cpu = *(u32 *)more_data;
-		more_data += sizeof(u32);
-		more_data += sizeof(u32); /* reserved */
-	}
+	memset(&data, 0, sizeof(data));
+	data.time = -1;
+	data.cpu = -1;
+	data.period = 1;
 
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
+	thread = threads__findnew(event->ip.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			 event->header.type);
@@ -357,7 +345,7 @@ static int process_sample_event(event_t 
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, more_data, cpu, timestamp, thread);
+	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
 
 	return 0;
 }
diff -puN tools/perf/builtin-sched.c~perf-make-common-sample tools/perf/builtin-sched.c
--- linux-2.6/tools/perf/builtin-sched.c~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-sched.c	2009-12-06 19:42:51.000000000 +0900
@@ -1598,40 +1598,26 @@ process_raw_event(event_t *raw_event __u
 
 static int process_sample_event(event_t *event)
 {
+	struct sample_data data;
 	struct thread *thread;
-	u64 ip = event->ip.ip;
-	u64 timestamp = -1;
-	u32 cpu = -1;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
 
 	if (!(sample_type & PERF_SAMPLE_RAW))
 		return 0;
 
-	thread = threads__findnew(event->ip.pid);
+	memset(&data, 0, sizeof(data));
+	data.time = -1;
+	data.cpu = -1;
+	data.period = -1;
 
-	if (sample_type & PERF_SAMPLE_TIME) {
-		timestamp = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
-
-	if (sample_type & PERF_SAMPLE_CPU) {
-		cpu = *(u32 *)more_data;
-		more_data += sizeof(u32);
-		more_data += sizeof(u32); /* reserved */
-	}
-
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
+	thread = threads__findnew(data.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			 event->header.type);
@@ -1640,10 +1626,10 @@ static int process_sample_event(event_t 
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	if (profile_cpu != -1 && profile_cpu != (int) cpu)
+	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, more_data, cpu, timestamp, thread);
+	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
 
 	return 0;
 }
diff -puN tools/perf/builtin-timechart.c~perf-make-common-sample tools/perf/builtin-timechart.c
--- linux-2.6/tools/perf/builtin-timechart.c~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-timechart.c	2009-12-06 19:42:51.000000000 +0900
@@ -483,48 +483,22 @@ static void sched_switch(int cpu, u64 ti
 static int
 process_sample_event(event_t *event)
 {
-	int cursor = 0;
-	u64 addr = 0;
-	u64 stamp = 0;
-	u32 cpu = 0;
-	u32 pid = 0;
-	u32 size, *size_ptr;
+	struct sample_data data;
 	struct trace_entry *te;
 
-	if (sample_type & PERF_SAMPLE_IP)
-		cursor++;
-
-	if (sample_type & PERF_SAMPLE_TID) {
-		pid = event->sample.array[cursor]>>32;
-		cursor++;
-	}
-	if (sample_type & PERF_SAMPLE_TIME) {
-		stamp = event->sample.array[cursor++];
+	memset(&data, 0, sizeof(data));
 
-		if (!first_time || first_time > stamp)
-			first_time = stamp;
-		if (last_time < stamp)
-			last_time = stamp;
+	event__parse_sample(event, sample_type, &data);
 
+	if (sample_type & PERF_SAMPLE_TIME) {
+		if (!first_time || first_time > data.time)
+			first_time = data.time;
+		if (last_time < data.time)
+			last_time = data.time;
 	}
-	if (sample_type & PERF_SAMPLE_ADDR)
-		addr = event->sample.array[cursor++];
-	if (sample_type & PERF_SAMPLE_ID)
-		cursor++;
-	if (sample_type & PERF_SAMPLE_STREAM_ID)
-		cursor++;
-	if (sample_type & PERF_SAMPLE_CPU)
-		cpu = event->sample.array[cursor++] & 0xFFFFFFFF;
-	if (sample_type & PERF_SAMPLE_PERIOD)
-		cursor++;
-
-	size_ptr = (void *)&event->sample.array[cursor];
-
-	size = *size_ptr;
-	size_ptr++;
 
-	te = (void *)size_ptr;
-	if (sample_type & PERF_SAMPLE_RAW && size > 0) {
+	te = (void *)data.raw_data;
+	if (sample_type & PERF_SAMPLE_RAW && data.raw_size > 0) {
 		char *event_str;
 		struct power_entry *pe;
 
@@ -536,19 +510,19 @@ process_sample_event(event_t *event)
 			return 0;
 
 		if (strcmp(event_str, "power:power_start") == 0)
-			c_state_start(cpu, stamp, pe->value);
+			c_state_start(data.cpu, data.time, pe->value);
 
 		if (strcmp(event_str, "power:power_end") == 0)
-			c_state_end(cpu, stamp);
+			c_state_end(data.cpu, data.time);
 
 		if (strcmp(event_str, "power:power_frequency") == 0)
-			p_state_change(cpu, stamp, pe->value);
+			p_state_change(data.cpu, data.time, pe->value);
 
 		if (strcmp(event_str, "sched:sched_wakeup") == 0)
-			sched_wakeup(cpu, stamp, pid, te);
+			sched_wakeup(data.cpu, data.time, data.pid, te);
 
 		if (strcmp(event_str, "sched:sched_switch") == 0)
-			sched_switch(cpu, stamp, te);
+			sched_switch(data.cpu, data.time, te);
 	}
 	return 0;
 }
diff -puN tools/perf/builtin-trace.c~perf-make-common-sample tools/perf/builtin-trace.c
--- linux-2.6/tools/perf/builtin-trace.c~perf-make-common-sample	2009-12-06 19:42:51.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-trace.c	2009-12-06 19:42:51.000000000 +0900
@@ -66,58 +66,40 @@ static u64			sample_type;
 
 static int process_sample_event(event_t *event)
 {
-	u64 ip = event->ip.ip;
-	u64 timestamp = -1;
-	u32 cpu = -1;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
-	struct thread *thread = threads__findnew(event->ip.pid);
-
-	if (sample_type & PERF_SAMPLE_TIME) {
-		timestamp = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	struct sample_data data;
+	struct thread *thread;
 
-	if (sample_type & PERF_SAMPLE_CPU) {
-		cpu = *(u32 *)more_data;
-		more_data += sizeof(u32);
-		more_data += sizeof(u32); /* reserved */
-	}
+	memset(&data, 0, sizeof(data));
+	data.time = -1;
+	data.cpu = -1;
+	data.period = 1;
 
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
+	thread = threads__findnew(event->ip.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			 event->header.type);
 		return -1;
 	}
 
-	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
-
 	if (sample_type & PERF_SAMPLE_RAW) {
-		struct {
-			u32 size;
-			char data[0];
-		} *raw = more_data;
-
 		/*
 		 * FIXME: better resolve from pid from the struct trace_entry
 		 * field, although it should be the same than this perf
 		 * event pid
 		 */
-		scripting_ops->process_event(cpu, raw->data, raw->size,
-					     timestamp, thread->comm);
+		scripting_ops->process_event(data.cpu, data.raw_data,
+					     data.raw_size,
+					     data.time, thread->comm);
 	}
-	event__stats.total += period;
+	event__stats.total += data.period;
 
 	return 0;
 }
_

[PATCH 3/3] perf: misc small fixes

[OGAWA Hirofumi]

[Those was checked by valgrind, but not fixed all errors by lazyness. Sorry]

- util/parse-event.c
	"path" is pointer. It should be sizeof(*path)

- util/header.c
	"len" is aligned to 64. So, it tries to write the out of
	long_name buffer.

	So, this use "zero_buf" to write aligned area.

- util/trace-event-read.c
	"size" is not including nul byte. So, this allocates it, and set '\0'.

- util/trace-event-parse.c
	It needs parens to calc correct size.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 tools/perf/util/header.c            |    9 +++++++--
 tools/perf/util/parse-events.c      |    2 +-
 tools/perf/util/trace-event-parse.c |    2 +-
 tools/perf/util/trace-event-read.c  |    3 ++-
 4 files changed, 11 insertions(+), 5 deletions(-)

diff -puN tools/perf/util/parse-events.c~perf-fix-misc tools/perf/util/parse-events.c
--- linux-2.6/tools/perf/util/parse-events.c~perf-fix-misc	2009-12-06 19:42:53.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/parse-events.c	2009-12-06 19:42:53.000000000 +0900
@@ -197,7 +197,7 @@ struct tracepoint_path *tracepoint_id_to
 			if (id == config) {
 				closedir(evt_dir);
 				closedir(sys_dir);
-				path = zalloc(sizeof(path));
+				path = zalloc(sizeof(*path));
 				path->system = malloc(MAX_EVENT_LENGTH);
 				if (!path->system) {
 					free(path);
diff -puN tools/perf/util/symbol.c~perf-fix-misc tools/perf/util/symbol.c
diff -puN tools/perf/util/header.c~perf-fix-misc tools/perf/util/header.c
--- linux-2.6/tools/perf/util/header.c~perf-fix-misc	2009-12-06 19:42:53.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/header.c	2009-12-06 19:42:53.000000000 +0900
@@ -187,7 +187,9 @@ static int do_write(int fd, const void *
 
 static int __dsos__write_buildid_table(struct list_head *head, int fd)
 {
+#define NAME_ALIGN	64
 	struct dso *pos;
+	static const char zero_buf[NAME_ALIGN];
 
 	list_for_each_entry(pos, head, node) {
 		int err;
@@ -197,14 +199,17 @@ static int __dsos__write_buildid_table(s
 		if (!pos->has_build_id)
 			continue;
 		len = pos->long_name_len + 1;
-		len = ALIGN(len, 64);
+		len = ALIGN(len, NAME_ALIGN);
 		memset(&b, 0, sizeof(b));
 		memcpy(&b.build_id, pos->build_id, sizeof(pos->build_id));
 		b.header.size = sizeof(b) + len;
 		err = do_write(fd, &b, sizeof(b));
 		if (err < 0)
 			return err;
-		err = do_write(fd, pos->long_name, len);
+		err = do_write(fd, pos->long_name, pos->long_name_len + 1);
+		if (err < 0)
+			return err;
+		err = do_write(fd, zero_buf, len - pos->long_name_len + 1);
 		if (err < 0)
 			return err;
 	}
diff -puN tools/perf/util/trace-event-read.c~perf-fix-misc tools/perf/util/trace-event-read.c
--- linux-2.6/tools/perf/util/trace-event-read.c~perf-fix-misc	2009-12-06 19:42:53.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/trace-event-read.c	2009-12-06 19:42:53.000000000 +0900
@@ -145,8 +145,9 @@ static void read_proc_kallsyms(void)
 	if (!size)
 		return;
 
-	buf = malloc_or_die(size);
+	buf = malloc_or_die(size + 1);
 	read_or_die(buf, size);
+	buf[size] = '\0';
 
 	parse_proc_kallsyms(buf, size);
 
diff -puN tools/perf/util/trace-event-parse.c~perf-fix-misc tools/perf/util/trace-event-parse.c
--- linux-2.6/tools/perf/util/trace-event-parse.c~perf-fix-misc	2009-12-06 19:42:53.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/trace-event-parse.c	2009-12-06 19:42:53.000000000 +0900
@@ -177,7 +177,7 @@ void parse_proc_kallsyms(char *file, uns
 		func_count++;
 	}
 
-	func_list = malloc_or_die(sizeof(*func_list) * func_count + 1);
+	func_list = malloc_or_die(sizeof(*func_list) * (func_count + 1));
 
 	i = 0;
 	while (list) {
_

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[tip:perf/urgent] perf tools: Misc small fixes

[tip-bot for OGAWA Hirofumi]

Commit-ID:  7691b1ec2e4a8d4bd88dcf88b29792399ebe1c91
Gitweb:     http://git.kernel.org/tip/7691b1ec2e4a8d4bd88dcf88b29792399ebe1c91
Author:     OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
AuthorDate: Sun, 6 Dec 2009 20:10:49 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 6 Dec 2009 18:15:02 +0100

perf tools: Misc small fixes

- util/header.c
	"len" is aligned to 64. So, it tries to write the out of
	long_name buffer.

	So, this use "zero_buf" to write aligned area.

- util/trace-event-read.c
	"size" is not including nul byte. So, this allocates it, and set '\0'.

- util/trace-event-parse.c
	It needs parens to calc correct size.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <87d42s8iiu.fsf_-_@devron.myhome.or.jp>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/util/header.c            |    9 +++++++--
 tools/perf/util/trace-event-parse.c |    2 +-
 tools/perf/util/trace-event-read.c  |    3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4805e6d..08b6759 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -187,7 +187,9 @@ static int do_write(int fd, const void *buf, size_t size)
 
 static int __dsos__write_buildid_table(struct list_head *head, int fd)
 {
+#define NAME_ALIGN	64
 	struct dso *pos;
+	static const char zero_buf[NAME_ALIGN];
 
 	list_for_each_entry(pos, head, node) {
 		int err;
@@ -197,14 +199,17 @@ static int __dsos__write_buildid_table(struct list_head *head, int fd)
 		if (!pos->has_build_id)
 			continue;
 		len = pos->long_name_len + 1;
-		len = ALIGN(len, 64);
+		len = ALIGN(len, NAME_ALIGN);
 		memset(&b, 0, sizeof(b));
 		memcpy(&b.build_id, pos->build_id, sizeof(pos->build_id));
 		b.header.size = sizeof(b) + len;
 		err = do_write(fd, &b, sizeof(b));
 		if (err < 0)
 			return err;
-		err = do_write(fd, pos->long_name, len);
+		err = do_write(fd, pos->long_name, pos->long_name_len + 1);
+		if (err < 0)
+			return err;
+		err = do_write(fd, zero_buf, len - pos->long_name_len + 1);
 		if (err < 0)
 			return err;
 	}
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 0302405..6ffe9d6 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -177,7 +177,7 @@ void parse_proc_kallsyms(char *file, unsigned int size __unused)
 		func_count++;
 	}
 
-	func_list = malloc_or_die(sizeof(*func_list) * func_count + 1);
+	func_list = malloc_or_die(sizeof(*func_list) * (func_count + 1));
 
 	i = 0;
 	while (list) {
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index 342dfdd..1744422 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -145,8 +145,9 @@ static void read_proc_kallsyms(void)
 	if (!size)
 		return;
 
-	buf = malloc_or_die(size);
+	buf = malloc_or_die(size + 1);
 	read_or_die(buf, size);
+	buf[size] = '\0';
 
 	parse_proc_kallsyms(buf, size);
 

[tip:perf/urgent] perf: Make common SAMPLE_EVENT parser

[tip-bot for OGAWA Hirofumi]

Commit-ID:  180f95e29aa8782c019caa64ede2a28d8ab62564
Gitweb:     http://git.kernel.org/tip/180f95e29aa8782c019caa64ede2a28d8ab62564
Author:     OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
AuthorDate: Sun, 6 Dec 2009 20:08:24 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 6 Dec 2009 18:15:01 +0100

perf: Make common SAMPLE_EVENT parser

Currently, sample event data is parsed for each commands, and it
is assuming that the data is not including other data. (E.g.
timechart, trace, etc. can't parse the event if it has
PERF_SAMPLE_CALLCHAIN)

So, even if we record the superset data for multiple commands at
a time, commands can't parse. etc.

To fix it, this makes common sample event parser, and use it to
parse sample event correctly. (PERF_SAMPLE_READ is unsupported
for now though, it seems to be not using.)

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <87hbs48imv.fsf@devron.myhome.or.jp>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-kmem.c      |   36 +++++++--------------
 tools/perf/builtin-report.c    |   39 +++++++++++------------
 tools/perf/builtin-sched.c     |   38 +++++++---------------
 tools/perf/builtin-timechart.c |   58 +++++++++-------------------------
 tools/perf/builtin-trace.c     |   48 +++++++++-------------------
 tools/perf/util/event.c        |   67 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/event.h        |   17 +++++++++-
 7 files changed, 156 insertions(+), 147 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 047fef7..f218990 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -320,35 +320,23 @@ process_raw_event(event_t *raw_event __used, void *more_data,
 
 static int process_sample_event(event_t *event)
 {
-	u64 ip = event->ip.ip;
-	u64 timestamp = -1;
-	u32 cpu = -1;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
-	struct thread *thread = threads__findnew(event->ip.pid);
-
-	if (sample_type & PERF_SAMPLE_TIME) {
-		timestamp = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	struct sample_data data;
+	struct thread *thread;
 
-	if (sample_type & PERF_SAMPLE_CPU) {
-		cpu = *(u32 *)more_data;
-		more_data += sizeof(u32);
-		more_data += sizeof(u32); /* reserved */
-	}
+	memset(&data, 0, sizeof(data));
+	data.time = -1;
+	data.cpu = -1;
+	data.period = 1;
 
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
+	thread = threads__findnew(event->ip.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			 event->header.type);
@@ -357,7 +345,7 @@ static int process_sample_event(event_t *event)
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, more_data, cpu, timestamp, thread);
+	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
 
 	return 0;
 }
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 383c4ab..2b9eb3a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -605,44 +605,41 @@ static int validate_chain(struct ip_callchain *chain, event_t *event)
 
 static int process_sample_event(event_t *event)
 {
-	u64 ip = event->ip.ip;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
-	struct ip_callchain *chain = NULL;
+	struct sample_data data;
 	int cpumode;
 	struct addr_location al;
-	struct thread *thread = threads__findnew(event->ip.pid);
+	struct thread *thread;
 
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	memset(&data, 0, sizeof(data));
+	data.period = 1;
+
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
 	if (sample_type & PERF_SAMPLE_CALLCHAIN) {
 		unsigned int i;
 
-		chain = (void *)more_data;
-
-		dump_printf("... chain: nr:%Lu\n", chain->nr);
+		dump_printf("... chain: nr:%Lu\n", data.callchain->nr);
 
-		if (validate_chain(chain, event) < 0) {
+		if (validate_chain(data.callchain, event) < 0) {
 			pr_debug("call-chain problem with event, "
 				 "skipping it.\n");
 			return 0;
 		}
 
 		if (dump_trace) {
-			for (i = 0; i < chain->nr; i++)
-				dump_printf("..... %2d: %016Lx\n", i, chain->ips[i]);
+			for (i = 0; i < data.callchain->nr; i++)
+				dump_printf("..... %2d: %016Lx\n",
+					    i, data.callchain->ips[i]);
 		}
 	}
 
+	thread = threads__findnew(data.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			event->header.type);
@@ -657,7 +654,7 @@ static int process_sample_event(event_t *event)
 	cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
 
 	thread__find_addr_location(thread, cpumode,
-				   MAP__FUNCTION, ip, &al, NULL);
+				   MAP__FUNCTION, data.ip, &al, NULL);
 	/*
 	 * We have to do this here as we may have a dso with no symbol hit that
 	 * has a name longer than the ones with symbols sampled.
@@ -675,12 +672,12 @@ static int process_sample_event(event_t *event)
 	if (sym_list && al.sym && !strlist__has_entry(sym_list, al.sym->name))
 		return 0;
 
-	if (hist_entry__add(&al, chain, period)) {
+	if (hist_entry__add(&al, data.callchain, data.period)) {
 		pr_debug("problem incrementing symbol count, skipping event\n");
 		return -1;
 	}
 
-	event__stats.total += period;
+	event__stats.total += data.period;
 
 	return 0;
 }
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 26b782f..45c46c7 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1598,40 +1598,26 @@ process_raw_event(event_t *raw_event __used, void *more_data,
 
 static int process_sample_event(event_t *event)
 {
+	struct sample_data data;
 	struct thread *thread;
-	u64 ip = event->ip.ip;
-	u64 timestamp = -1;
-	u32 cpu = -1;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
 
 	if (!(sample_type & PERF_SAMPLE_RAW))
 		return 0;
 
-	thread = threads__findnew(event->ip.pid);
+	memset(&data, 0, sizeof(data));
+	data.time = -1;
+	data.cpu = -1;
+	data.period = -1;
 
-	if (sample_type & PERF_SAMPLE_TIME) {
-		timestamp = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
-
-	if (sample_type & PERF_SAMPLE_CPU) {
-		cpu = *(u32 *)more_data;
-		more_data += sizeof(u32);
-		more_data += sizeof(u32); /* reserved */
-	}
-
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
+	thread = threads__findnew(data.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			 event->header.type);
@@ -1640,10 +1626,10 @@ static int process_sample_event(event_t *event)
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	if (profile_cpu != -1 && profile_cpu != (int) cpu)
+	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, more_data, cpu, timestamp, thread);
+	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
 
 	return 0;
 }
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index c0f29ed..f472df9 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -483,48 +483,22 @@ static void sched_switch(int cpu, u64 timestamp, struct trace_entry *te)
 static int
 process_sample_event(event_t *event)
 {
-	int cursor = 0;
-	u64 addr = 0;
-	u64 stamp = 0;
-	u32 cpu = 0;
-	u32 pid = 0;
-	u32 size, *size_ptr;
+	struct sample_data data;
 	struct trace_entry *te;
 
-	if (sample_type & PERF_SAMPLE_IP)
-		cursor++;
-
-	if (sample_type & PERF_SAMPLE_TID) {
-		pid = event->sample.array[cursor]>>32;
-		cursor++;
-	}
-	if (sample_type & PERF_SAMPLE_TIME) {
-		stamp = event->sample.array[cursor++];
+	memset(&data, 0, sizeof(data));
 
-		if (!first_time || first_time > stamp)
-			first_time = stamp;
-		if (last_time < stamp)
-			last_time = stamp;
+	event__parse_sample(event, sample_type, &data);
 
+	if (sample_type & PERF_SAMPLE_TIME) {
+		if (!first_time || first_time > data.time)
+			first_time = data.time;
+		if (last_time < data.time)
+			last_time = data.time;
 	}
-	if (sample_type & PERF_SAMPLE_ADDR)
-		addr = event->sample.array[cursor++];
-	if (sample_type & PERF_SAMPLE_ID)
-		cursor++;
-	if (sample_type & PERF_SAMPLE_STREAM_ID)
-		cursor++;
-	if (sample_type & PERF_SAMPLE_CPU)
-		cpu = event->sample.array[cursor++] & 0xFFFFFFFF;
-	if (sample_type & PERF_SAMPLE_PERIOD)
-		cursor++;
-
-	size_ptr = (void *)&event->sample.array[cursor];
-
-	size = *size_ptr;
-	size_ptr++;
 
-	te = (void *)size_ptr;
-	if (sample_type & PERF_SAMPLE_RAW && size > 0) {
+	te = (void *)data.raw_data;
+	if (sample_type & PERF_SAMPLE_RAW && data.raw_size > 0) {
 		char *event_str;
 		struct power_entry *pe;
 
@@ -536,19 +510,19 @@ process_sample_event(event_t *event)
 			return 0;
 
 		if (strcmp(event_str, "power:power_start") == 0)
-			c_state_start(cpu, stamp, pe->value);
+			c_state_start(data.cpu, data.time, pe->value);
 
 		if (strcmp(event_str, "power:power_end") == 0)
-			c_state_end(cpu, stamp);
+			c_state_end(data.cpu, data.time);
 
 		if (strcmp(event_str, "power:power_frequency") == 0)
-			p_state_change(cpu, stamp, pe->value);
+			p_state_change(data.cpu, data.time, pe->value);
 
 		if (strcmp(event_str, "sched:sched_wakeup") == 0)
-			sched_wakeup(cpu, stamp, pid, te);
+			sched_wakeup(data.cpu, data.time, data.pid, te);
 
 		if (strcmp(event_str, "sched:sched_switch") == 0)
-			sched_switch(cpu, stamp, te);
+			sched_switch(data.cpu, data.time, te);
 	}
 	return 0;
 }
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index abb914a..c2fcc34 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -66,58 +66,40 @@ static u64			sample_type;
 
 static int process_sample_event(event_t *event)
 {
-	u64 ip = event->ip.ip;
-	u64 timestamp = -1;
-	u32 cpu = -1;
-	u64 period = 1;
-	void *more_data = event->ip.__more_data;
-	struct thread *thread = threads__findnew(event->ip.pid);
-
-	if (sample_type & PERF_SAMPLE_TIME) {
-		timestamp = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	struct sample_data data;
+	struct thread *thread;
 
-	if (sample_type & PERF_SAMPLE_CPU) {
-		cpu = *(u32 *)more_data;
-		more_data += sizeof(u32);
-		more_data += sizeof(u32); /* reserved */
-	}
+	memset(&data, 0, sizeof(data));
+	data.time = -1;
+	data.cpu = -1;
+	data.period = 1;
 
-	if (sample_type & PERF_SAMPLE_PERIOD) {
-		period = *(u64 *)more_data;
-		more_data += sizeof(u64);
-	}
+	event__parse_sample(event, sample_type, &data);
 
 	dump_printf("(IP, %d): %d/%d: %p period: %Ld\n",
 		event->header.misc,
-		event->ip.pid, event->ip.tid,
-		(void *)(long)ip,
-		(long long)period);
+		data.pid, data.tid,
+		(void *)(long)data.ip,
+		(long long)data.period);
 
+	thread = threads__findnew(event->ip.pid);
 	if (thread == NULL) {
 		pr_debug("problem processing %d event, skipping it.\n",
 			 event->header.type);
 		return -1;
 	}
 
-	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
-
 	if (sample_type & PERF_SAMPLE_RAW) {
-		struct {
-			u32 size;
-			char data[0];
-		} *raw = more_data;
-
 		/*
 		 * FIXME: better resolve from pid from the struct trace_entry
 		 * field, although it should be the same than this perf
 		 * event pid
 		 */
-		scripting_ops->process_event(cpu, raw->data, raw->size,
-					     timestamp, thread->comm);
+		scripting_ops->process_event(data.cpu, data.raw_data,
+					     data.raw_size,
+					     data.time, thread->comm);
 	}
-	event__stats.total += period;
+	event__stats.total += data.period;
 
 	return 0;
 }
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 414b89d..4dcecaf 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -310,3 +310,70 @@ int event__preprocess_sample(const event_t *self, struct addr_location *al,
 			al->level == 'H' ? "[hypervisor]" : "<not found>");
 	return 0;
 }
+
+int event__parse_sample(event_t *event, u64 type, struct sample_data *data)
+{
+	u64 *array = event->sample.array;
+
+	if (type & PERF_SAMPLE_IP) {
+		data->ip = event->ip.ip;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_TID) {
+		u32 *p = (u32 *)array;
+		data->pid = p[0];
+		data->tid = p[1];
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_TIME) {
+		data->time = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_ADDR) {
+		data->addr = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_ID) {
+		data->id = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_STREAM_ID) {
+		data->stream_id = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_CPU) {
+		u32 *p = (u32 *)array;
+		data->cpu = *p;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_PERIOD) {
+		data->period = *array;
+		array++;
+	}
+
+	if (type & PERF_SAMPLE_READ) {
+		pr_debug("PERF_SAMPLE_READ is unsuported for now\n");
+		return -1;
+	}
+
+	if (type & PERF_SAMPLE_CALLCHAIN) {
+		data->callchain = (struct ip_callchain *)array;
+		array += 1 + data->callchain->nr;
+	}
+
+	if (type & PERF_SAMPLE_RAW) {
+		u32 *p = (u32 *)array;
+		data->raw_size = *p;
+		p++;
+		data->raw_data = p;
+	}
+
+	return 0;
+}
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index a4cc810..c7a78ee 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -56,11 +56,25 @@ struct read_event {
 	u64 id;
 };
 
-struct sample_event{
+struct sample_event {
 	struct perf_event_header        header;
 	u64 array[];
 };
 
+struct sample_data {
+	u64 ip;
+	u32 pid, tid;
+	u64 time;
+	u64 addr;
+	u64 id;
+	u64 stream_id;
+	u32 cpu;
+	u64 period;
+	struct ip_callchain *callchain;
+	u32 raw_size;
+	void *raw_data;
+};
+
 #define BUILD_ID_SIZE 20
 
 struct build_id_event {
@@ -155,5 +169,6 @@ int event__process_task(event_t *self);
 struct addr_location;
 int event__preprocess_sample(const event_t *self, struct addr_location *al,
 			     symbol_filter_t filter);
+int event__parse_sample(event_t *event, u64 type, struct sample_data *data);
 
 #endif /* __PERF_RECORD_H */

Re: [PATCH 1/3] perf: Fix timechart header handling

[Ingo Molnar]

* OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote:

> Hi,
> 
> Update "struct trace_entry" to match with current one. And remove
> "size" field from it.
> 
> If it has "size", it become cause of alignment mismatch of structure
> with kernel.
> 
> Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> ---
> 
>  tools/perf/builtin-timechart.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Just curious, have you tested timechart functionality after having done 
this change - does it still work fine for you?

Thanks,

	Ingo

Re: [PATCH 1/3] perf: Fix timechart header handling

[OGAWA Hirofumi]

Ingo Molnar <mingo@elte.hu> writes:

> * OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote:
>
>> Hi,
>> 
>> Update "struct trace_entry" to match with current one. And remove
>> "size" field from it.
>> 
>> If it has "size", it become cause of alignment mismatch of structure
>> with kernel.
>> 
>> Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>> ---
>> 
>>  tools/perf/builtin-timechart.c |   12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> Just curious, have you tested timechart functionality after having done 
> this change - does it still work fine for you?

Yes. Rather, it doesn't work without this. (Without patch, it provide
svg output, but it doesn't provide correct sched state. And this issue
is only 64bit arch.)

E.g. The following has difference (perf timechart doesn't has hole).

In kernel,

struct ftrace_raw_sched_switch {
	struct trace_entry         ent;                  /*     0    12 */
	char                       prev_comm[16];        /*    12    16 */
	pid_t                      prev_pid;             /*    28     4 */
	int                        prev_prio;            /*    32     4 */

	/* XXX 4 bytes hole, try to pack */

	long int                   prev_state;           /*    40     8 */
	char                       next_comm[16];        /*    48    16 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	pid_t                      next_pid;             /*    64     4 */
	int                        next_prio;            /*    68     4 */
	char                       __data[0];            /*    72     0 */

	/* size: 72, cachelines: 2, members: 9 */
	/* sum members: 68, holes: 1, sum holes: 4 */
	/* last cacheline: 8 bytes */
};

In perl timechart, [Note, struct trace_entry is including "u32 size"]

struct sched_switch {
	struct trace_entry         te;                   /*     0    16 */
	char                       prev_comm[16];        /*    16    16 */
	int                        prev_pid;             /*    32     4 */
	int                        prev_prio;            /*    36     4 */
	long int                   prev_state;           /*    40     8 */
	char                       next_comm[16];        /*    48    16 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	int                        next_pid;             /*    64     4 */
	int                        next_prio;            /*    68     4 */

	/* size: 72, cachelines: 2, members: 8 */
	/* last cacheline: 8 bytes */
};


Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH 1/3] perf: Fix timechart header handling

[Ingo Molnar]

* OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> wrote:
> >
> >> Hi,
> >> 
> >> Update "struct trace_entry" to match with current one. And remove
> >> "size" field from it.
> >> 
> >> If it has "size", it become cause of alignment mismatch of structure
> >> with kernel.
> >> 
> >> Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> >> ---
> >> 
> >>  tools/perf/builtin-timechart.c |   12 ++++++++----
> >>  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > Just curious, have you tested timechart functionality after having done 
> > this change - does it still work fine for you?
> 
> Yes. Rather, it doesn't work without this. (Without patch, it provide
> svg output, but it doesn't provide correct sched state. And this issue
> is only 64bit arch.)
> 
> E.g. The following has difference (perf timechart doesn't has hole).
> 
> In kernel,
> 
> struct ftrace_raw_sched_switch {
> 	struct trace_entry         ent;                  /*     0    12 */
> 	char                       prev_comm[16];        /*    12    16 */
> 	pid_t                      prev_pid;             /*    28     4 */
> 	int                        prev_prio;            /*    32     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	long int                   prev_state;           /*    40     8 */
> 	char                       next_comm[16];        /*    48    16 */
> 	/* --- cacheline 1 boundary (64 bytes) --- */
> 	pid_t                      next_pid;             /*    64     4 */
> 	int                        next_prio;            /*    68     4 */
> 	char                       __data[0];            /*    72     0 */
> 
> 	/* size: 72, cachelines: 2, members: 9 */
> 	/* sum members: 68, holes: 1, sum holes: 4 */
> 	/* last cacheline: 8 bytes */
> };
> 
> In perl timechart, [Note, struct trace_entry is including "u32 size"]
> 
> struct sched_switch {
> 	struct trace_entry         te;                   /*     0    16 */
> 	char                       prev_comm[16];        /*    16    16 */
> 	int                        prev_pid;             /*    32     4 */
> 	int                        prev_prio;            /*    36     4 */
> 	long int                   prev_state;           /*    40     8 */
> 	char                       next_comm[16];        /*    48    16 */
> 	/* --- cacheline 1 boundary (64 bytes) --- */
> 	int                        next_pid;             /*    64     4 */
> 	int                        next_prio;            /*    68     4 */
> 
> 	/* size: 72, cachelines: 2, members: 8 */
> 	/* last cacheline: 8 bytes */
> };
> 

ok - thanks for the confirmation!

	Ingo

Re: [PATCH 1/3] perf: Fix timechart header handling

[OGAWA Hirofumi]

Ingo Molnar <mingo@elte.hu> writes:

>> Yes. Rather, it doesn't work without this. (Without patch, it provide
>> svg output, but it doesn't provide correct sched state. And this issue
>> is only 64bit arch.)

[...]

> ok - thanks for the confirmation!

BTW, more correct fix would be using raw_filed_value() or something, so
it would become more robust. But, sorry, I didn't.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[tip:perf/urgent] perf timechart: Fix header handling

[tip-bot for OGAWA Hirofumi]

Commit-ID:  028c515253761084c6594bf9ac9b194b51d87065
Gitweb:     http://git.kernel.org/tip/028c515253761084c6594bf9ac9b194b51d87065
Author:     OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
AuthorDate: Sun, 6 Dec 2009 20:07:29 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 6 Dec 2009 18:15:01 +0100

perf timechart: Fix header handling

Update "struct trace_entry" to match with current one. And
remove "size" field from it.

If it has "size", it become cause of alignment mismatch of
structure with kernel.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Acked-by: Arjan van de Ven <arjan@infradead.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <87ljhg8ioe.fsf@devron.myhome.or.jp>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-timechart.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index cb58b66..c0f29ed 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -302,12 +302,11 @@ process_exit_event(event_t *event)
 }
 
 struct trace_entry {
-	u32			size;
 	unsigned short		type;
 	unsigned char		flags;
 	unsigned char		preempt_count;
 	int			pid;
-	int			tgid;
+	int			lock_depth;
 };
 
 struct power_entry {
@@ -489,6 +488,7 @@ process_sample_event(event_t *event)
 	u64 stamp = 0;
 	u32 cpu = 0;
 	u32 pid = 0;
+	u32 size, *size_ptr;
 	struct trace_entry *te;
 
 	if (sample_type & PERF_SAMPLE_IP)
@@ -518,9 +518,13 @@ process_sample_event(event_t *event)
 	if (sample_type & PERF_SAMPLE_PERIOD)
 		cursor++;
 
-	te = (void *)&event->sample.array[cursor];
+	size_ptr = (void *)&event->sample.array[cursor];
 
-	if (sample_type & PERF_SAMPLE_RAW && te->size > 0) {
+	size = *size_ptr;
+	size_ptr++;
+
+	te = (void *)size_ptr;
+	if (sample_type & PERF_SAMPLE_RAW && size > 0) {
 		char *event_str;
 		struct power_entry *pe;