[patch 0/9] perf: Consolidate the event handling and ordering

[patch 0/9] perf: Consolidate the event handling and ordering

[Thomas Gleixner]

The following series fixes an unbound array access and further
consolidates the event handling in session.c.

It now allows:

   - time ordered dumps
   - break time ordering when not all events provide timestamps

Thanks,

	tglx

[patch 1/9] perf: tools: Prevent unbound event__name array access

[Thomas Gleixner]

[-- Attachment #1: perf-session-fix-event-name-access.patch --]
[-- Type: text/plain, Size: 3172 bytes --]

event__name[] is missing an entry for PERF_RECORD_FINISHED_ROUND, but
we happily access the array from the dump code.

Make event__name[] static and provide an accessor function, fix up all
callers and add the missing string.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/event.c   |   12 +++++++++++-
 tools/perf/util/event.h   |    2 +-
 tools/perf/util/hist.c    |    9 ++++++---
 tools/perf/util/session.c |    2 +-
 4 files changed, 19 insertions(+), 6 deletions(-)

Index: linux-2.6-tip/tools/perf/util/event.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/event.c
+++ linux-2.6-tip/tools/perf/util/event.c
@@ -7,7 +7,7 @@
 #include "strlist.h"
 #include "thread.h"
 
-const char *event__name[] = {
+static const char *event__name[] = {
 	[0]			 = "TOTAL",
 	[PERF_RECORD_MMAP]	 = "MMAP",
 	[PERF_RECORD_LOST]	 = "LOST",
@@ -22,8 +22,18 @@ const char *event__name[] = {
 	[PERF_RECORD_HEADER_EVENT_TYPE]	 = "EVENT_TYPE",
 	[PERF_RECORD_HEADER_TRACING_DATA]	 = "TRACING_DATA",
 	[PERF_RECORD_HEADER_BUILD_ID]	 = "BUILD_ID",
+	[PERF_RECORD_FINISHED_ROUND]	 = "FINISHED_ROUND",
 };
 
+const char *event__get_event_name(unsigned int id)
+{
+	if (id >= ARRAY_SIZE(event__name))
+		return "INVALID";
+	if (!event__name[id])
+		return "UNKNOWN";
+	return event__name[id];
+}
+
 static struct sample_data synth_sample = {
 	.pid	   = -1,
 	.tid	   = -1,
Index: linux-2.6-tip/tools/perf/util/event.h
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/event.h
+++ linux-2.6-tip/tools/perf/util/event.h
@@ -171,6 +171,6 @@ int event__preprocess_sample(const event
 int event__parse_sample(const event_t *event, struct perf_session *session,
 			struct sample_data *sample);
 
-extern const char *event__name[];
+const char *event__get_event_name(unsigned int id);
 
 #endif /* __PERF_RECORD_H */
Index: linux-2.6-tip/tools/perf/util/hist.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/hist.c
+++ linux-2.6-tip/tools/perf/util/hist.c
@@ -1168,10 +1168,13 @@ size_t hists__fprintf_nr_events(struct h
 	size_t ret = 0;
 
 	for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
-		if (!event__name[i])
+		const char *name = event__get_event_name(i);
+
+		if (!strcmp(name, "UNKNOWN"))
 			continue;
-		ret += fprintf(fp, "%10s events: %10d\n",
-			       event__name[i], self->stats.nr_events[i]);
+
+		ret += fprintf(fp, "%16s events: %10d\n", name,
+			       self->stats.nr_events[i]);
 	}
 
 	return ret;
Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -718,7 +718,7 @@ static int perf_session__process_event(s
 	if (event->header.type < PERF_RECORD_HEADER_MAX) {
 		dump_printf("%#Lx [%#x]: PERF_RECORD_%s",
 			    file_offset, event->header.size,
-			    event__name[event->header.type]);
+			    event__get_event_name(event->header.type));
 		hists__inc_nr_events(&session->hists, event->header.type);
 	}
 

[patch 2/9] perf: session: Dont queue events w/o timestamps

[Thomas Gleixner]

[-- Attachment #1: perf-session-do-not-queue-events-w-o-timestamps.patch --]
[-- Type: text/plain, Size: 805 bytes --]

If the event has no timestamp assigned then the parse code sets it to
~0ULL which causes the ordering code to enqueue it at the end.

Process it right away.

Reported-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -603,7 +603,7 @@ static int perf_session_queue_event(stru
 	u64 timestamp = data->time;
 	struct sample_queue *new;
 
-	if (!timestamp)
+	if (!timestamp || timestamp == ~0ULL)
 		return -ETIME;
 
 	if (timestamp < s->ordered_samples.last_flush) {

[patch 3/9] perf: session: Consolidate the dump code

[Thomas Gleixner]

[-- Attachment #1: perf-session-split-out-dump-code.patch --]
[-- Type: text/plain, Size: 4288 bytes --]

The dump code used by perf report -D is scattered all over the
place. Move it to separate functions.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/event.h   |    1 
 tools/perf/util/session.c |   64 ++++++++++++++++++++++++++++------------------
 2 files changed, 40 insertions(+), 25 deletions(-)

Index: linux-2.6-tip/tools/perf/util/event.h
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/event.h
+++ linux-2.6-tip/tools/perf/util/event.h
@@ -85,6 +85,7 @@ struct build_id_event {
 };
 
 enum perf_user_event_type { /* above any possible kernel type */
+	PERF_RECORD_USER_TYPE_START		= 64,
 	PERF_RECORD_HEADER_ATTR			= 64,
 	PERF_RECORD_HEADER_EVENT_TYPE		= 65,
 	PERF_RECORD_HEADER_TRACING_DATA		= 66,
Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -665,6 +665,35 @@ static void perf_session__print_tstamp(s
 		printf("%Lu ", sample->time);
 }
 
+static void dump_event(struct perf_session *session, event_t *event,
+		       u64 file_offset, struct sample_data *sample)
+{
+	if (!dump_trace)
+		return;
+
+	dump_printf("\n%#Lx [%#x]: event: %d\n", file_offset,
+		    event->header.size, event->header.type);
+
+	trace_event(event);
+
+	if (sample)
+		perf_session__print_tstamp(session, event, sample);
+
+	dump_printf("%#Lx [%#x]: PERF_RECORD_%s",
+		    file_offset, event->header.size,
+		    event__get_event_name(event->header.type));
+}
+
+static void dump_sample(struct perf_session *session, event_t *event,
+			struct sample_data *sample)
+{
+	dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld\n", event->header.misc,
+		    sample->pid, sample->tid, sample->ip, sample->period);
+
+	if (session->sample_type & PERF_SAMPLE_CALLCHAIN)
+		callchain__dump(sample);
+}
+
 static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
@@ -703,32 +732,24 @@ static int perf_session__process_event(s
 	struct sample_data sample;
 	int ret;
 
-	trace_event(event);
-
 	if (session->header.needs_swap && event__swap_ops[event->header.type])
 		event__swap_ops[event->header.type](event);
 
-	if (event->header.type >= PERF_RECORD_MMAP &&
-	    event->header.type <= PERF_RECORD_SAMPLE) {
-		event__parse_sample(event, session, &sample);
-		if (dump_trace)
-			perf_session__print_tstamp(session, event, &sample);
-	}
+	if (event->header.type >= PERF_RECORD_HEADER_MAX)
+		return -EINVAL;
+
+	hists__inc_nr_events(&session->hists, event->header.type);
 
-	if (event->header.type < PERF_RECORD_HEADER_MAX) {
-		dump_printf("%#Lx [%#x]: PERF_RECORD_%s",
-			    file_offset, event->header.size,
-			    event__get_event_name(event->header.type));
-		hists__inc_nr_events(&session->hists, event->header.type);
+	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
+		dump_event(session, event, file_offset, NULL);
+	else {
+		event__parse_sample(event, session, &sample);
+		dump_event(session, event, file_offset, &sample);
 	}
 
 	/* These events are processed right away */
 	switch (event->header.type) {
 	case PERF_RECORD_SAMPLE:
-		dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld\n",
-			    event->header.misc,
-			    sample.pid, sample.tid, sample.ip, sample.period);
-
 		if (session->sample_type & PERF_SAMPLE_CALLCHAIN) {
 			if (!ip_callchain__valid(sample.callchain, event)) {
 				pr_debug("call-chain problem with event, "
@@ -738,9 +759,8 @@ static int perf_session__process_event(s
 					sample.period;
 				return 0;
 			}
-
-			callchain__dump(&sample);
 		}
+		dump_sample(session, event, &sample);
 		break;
 
 	case PERF_RECORD_HEADER_ATTR:
@@ -870,9 +890,6 @@ more:
 
 	head += size;
 
-	dump_printf("\n%#Lx [%#x]: event: %d\n",
-		    head, event.header.size, event.header.type);
-
 	if (skip > 0)
 		head += skip;
 
@@ -961,9 +978,6 @@ more:
 
 	size = event->header.size;
 
-	dump_printf("\n%#Lx [%#x]: event: %d\n",
-		    file_pos, event->header.size, event->header.type);
-
 	if (size == 0 ||
 	    perf_session__process_event(session, event, ops, file_pos) < 0) {
 		dump_printf("%#Lx [%#x]: skipping unknown header type: %d\n",

[patch 4/9] perf: session: Store file offset in sample_queue

[Thomas Gleixner]

[-- Attachment #1: perf-session-store-file-offset-in-sample-queue.patch --]
[-- Type: text/plain, Size: 1449 bytes --]

Preparatory patch for ordered output of perf report -D

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -444,6 +444,7 @@ static event__swap_op event__swap_ops[] 
 
 struct sample_queue {
 	u64			timestamp;
+	u64			file_offset;
 	event_t			*event;
 	struct list_head	list;
 };
@@ -596,7 +597,7 @@ static void __queue_event(struct sample_
 #define MAX_SAMPLE_BUFFER	(64 * 1024 / sizeof(struct sample_queue))
 
 static int perf_session_queue_event(struct perf_session *s, event_t *event,
-				    struct sample_data *data)
+				    struct sample_data *data, u64 file_offset)
 {
 	struct ordered_samples *os = &s->ordered_samples;
 	struct list_head *sc = &os->sample_cache;
@@ -628,6 +629,7 @@ static int perf_session_queue_event(stru
 	}
 
 	new->timestamp = timestamp;
+	new->file_offset = file_offset;
 	new->event = event;
 
 	__queue_event(new, s);
@@ -780,7 +782,8 @@ static int perf_session__process_event(s
 	}
 
 	if (ops->ordered_samples) {
-		ret = perf_session_queue_event(session, event, &sample);
+		ret = perf_session_queue_event(session, event, &sample,
+					       file_offset);
 		if (ret != -ETIME)
 			return ret;
 	}

[patch 5/9] perf: session: Add file_offset to event delivery function

[Thomas Gleixner]

[-- Attachment #1: perf-session-add-fileoffset-to-deliver.patch --]
[-- Type: text/plain, Size: 1835 bytes --]

Preparatory patch for ordered output of perf report -D

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -465,7 +465,8 @@ static void perf_session_free_sample_buf
 static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
-				      struct perf_event_ops *ops);
+				      struct perf_event_ops *ops,
+				      u64 file_offset);
 
 static void flush_sample_queue(struct perf_session *s,
 			       struct perf_event_ops *ops)
@@ -485,7 +486,8 @@ static void flush_sample_queue(struct pe
 			break;
 
 		event__parse_sample(iter->event, s, &sample);
-		perf_session_deliver_event(s, iter->event, &sample, ops);
+		perf_session_deliver_event(s, iter->event, &sample, ops,
+					   iter->file_offset);
 
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
@@ -699,7 +701,8 @@ static void dump_sample(struct perf_sess
 static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
-				      struct perf_event_ops *ops)
+				      struct perf_event_ops *ops,
+				      u64 file_offset __used)
 {
 	switch (event->header.type) {
 	case PERF_RECORD_SAMPLE:
@@ -788,7 +791,8 @@ static int perf_session__process_event(s
 			return ret;
 	}
 
-	return perf_session_deliver_event(session, event, &sample, ops);
+	return perf_session_deliver_event(session, event, &sample, ops,
+					  file_offset);
 }
 
 void perf_event_header__bswap(struct perf_event_header *self)

[patch 6/9] perf: session: Move dump code to event delivery path

[Thomas Gleixner]

[-- Attachment #1: perf-session-move-dump-code-to-delivery.patch --]
[-- Type: text/plain, Size: 1455 bytes --]

Preparatory patch for ordered perf report -D

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -702,10 +702,13 @@ static int perf_session_deliver_event(st
 				      event_t *event,
 				      struct sample_data *sample,
 				      struct perf_event_ops *ops,
-				      u64 file_offset __used)
+				      u64 file_offset)
 {
+	dump_event(session, event, file_offset, sample);
+
 	switch (event->header.type) {
 	case PERF_RECORD_SAMPLE:
+		dump_sample(session, event, sample);
 		return ops->sample(event, sample, session);
 	case PERF_RECORD_MMAP:
 		return ops->mmap(event, sample, session);
@@ -747,10 +750,8 @@ static int perf_session__process_event(s
 
 	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
 		dump_event(session, event, file_offset, NULL);
-	else {
+	else
 		event__parse_sample(event, session, &sample);
-		dump_event(session, event, file_offset, &sample);
-	}
 
 	/* These events are processed right away */
 	switch (event->header.type) {
@@ -765,7 +766,6 @@ static int perf_session__process_event(s
 				return 0;
 			}
 		}
-		dump_sample(session, event, &sample);
 		break;
 
 	case PERF_RECORD_HEADER_ATTR:

[patch 7/9] perf: session: Split out sample preprocessing

[Thomas Gleixner]

[-- Attachment #1: perf-session-split-out-sample-preprocessing.patch --]
[-- Type: text/plain, Size: 2353 bytes --]

Simplify the code a bit

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |   40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -732,6 +732,22 @@ static int perf_session_deliver_event(st
 	}
 }
 
+static int preprocess_sample_record(struct perf_session *session,
+				    event_t *event, struct sample_data *sample)
+{
+	if (event->header.type != PERF_RECORD_SAMPLE ||
+	    !(session->sample_type & PERF_SAMPLE_CALLCHAIN))
+		return 0;
+
+	if (!ip_callchain__valid(sample->callchain, event)) {
+		pr_debug("call-chain problem with event, skipping it.\n");
+		++session->hists.stats.nr_invalid_chains;
+		session->hists.stats.total_invalid_chains += sample->period;
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int perf_session__process_event(struct perf_session *session,
 				       event_t *event,
 				       struct perf_event_ops *ops,
@@ -750,24 +766,9 @@ static int perf_session__process_event(s
 
 	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
 		dump_event(session, event, file_offset, NULL);
-	else
-		event__parse_sample(event, session, &sample);
 
 	/* These events are processed right away */
 	switch (event->header.type) {
-	case PERF_RECORD_SAMPLE:
-		if (session->sample_type & PERF_SAMPLE_CALLCHAIN) {
-			if (!ip_callchain__valid(sample.callchain, event)) {
-				pr_debug("call-chain problem with event, "
-					 "skipping it.\n");
-				++session->hists.stats.nr_invalid_chains;
-				session->hists.stats.total_invalid_chains +=
-					sample.period;
-				return 0;
-			}
-		}
-		break;
-
 	case PERF_RECORD_HEADER_ATTR:
 		return ops->attr(event, session);
 	case PERF_RECORD_HEADER_EVENT_TYPE:
@@ -784,6 +785,15 @@ static int perf_session__process_event(s
 		break;
 	}
 
+	/*
+	 * For all kernel events we get the sample data
+	 */
+	event__parse_sample(event, session, &sample);
+
+	/* Preprocess sample records - precheck callchains */
+	if (preprocess_sample_record(session, event, &sample))
+		return 0;
+
 	if (ops->ordered_samples) {
 		ret = perf_session_queue_event(session, event, &sample,
 					       file_offset);

[patch 8/9] perf: session: Split out user event processing

[Thomas Gleixner]

[-- Attachment #1: perf-split-out-user-event-processing.patch --]
[-- Type: text/plain, Size: 2178 bytes --]

Simplify further.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |   42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -748,24 +748,10 @@ static int preprocess_sample_record(stru
 	return 0;
 }
 
-static int perf_session__process_event(struct perf_session *session,
-				       event_t *event,
-				       struct perf_event_ops *ops,
-				       u64 file_offset)
+static int process_user_event(struct perf_session *session, event_t *event,
+			      struct perf_event_ops *ops, u64 file_offset)
 {
-	struct sample_data sample;
-	int ret;
-
-	if (session->header.needs_swap && event__swap_ops[event->header.type])
-		event__swap_ops[event->header.type](event);
-
-	if (event->header.type >= PERF_RECORD_HEADER_MAX)
-		return -EINVAL;
-
-	hists__inc_nr_events(&session->hists, event->header.type);
-
-	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
-		dump_event(session, event, file_offset, NULL);
+	dump_event(session, event, file_offset, NULL);
 
 	/* These events are processed right away */
 	switch (event->header.type) {
@@ -782,8 +768,28 @@ static int perf_session__process_event(s
 	case PERF_RECORD_FINISHED_ROUND:
 		return ops->finished_round(event, session, ops);
 	default:
-		break;
+		return -EINVAL;
 	}
+}
+
+static int perf_session__process_event(struct perf_session *session,
+				       event_t *event,
+				       struct perf_event_ops *ops,
+				       u64 file_offset)
+{
+	struct sample_data sample;
+	int ret;
+
+	if (session->header.needs_swap && event__swap_ops[event->header.type])
+		event__swap_ops[event->header.type](event);
+
+	if (event->header.type >= PERF_RECORD_HEADER_MAX)
+		return -EINVAL;
+
+	hists__inc_nr_events(&session->hists, event->header.type);
+
+	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
+		return process_user_event(session, event, ops, file_offset);
 
 	/*
 	 * For all kernel events we get the sample data

[patch 9/9] pref: session: Break event ordering when timestamps are missing

[Thomas Gleixner]

[-- Attachment #1: pref-session-break-ordering-on-request.patch --]
[-- Type: text/plain, Size: 1929 bytes --]

Allow the session client to specify that event ordering should be
stopped when not all events have time stamps.

Suggested-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/util/session.c |   13 ++++++++++++-
 tools/perf/util/session.h |    1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

Index: linux-2.6-tip/tools/perf/util/session.c
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.c
+++ linux-2.6-tip/tools/perf/util/session.c
@@ -751,12 +751,23 @@ static int preprocess_sample_record(stru
 static int process_user_event(struct perf_session *session, event_t *event,
 			      struct perf_event_ops *ops, u64 file_offset)
 {
+	int ret;
+
 	dump_event(session, event, file_offset, NULL);
 
 	/* These events are processed right away */
 	switch (event->header.type) {
 	case PERF_RECORD_HEADER_ATTR:
-		return ops->attr(event, session);
+		/* This updates session->sample_id_all */
+		ret = ops->attr(event, session);
+		/* Break ordering if sample_id_all is false */
+		if (ops->ordering_requires_timestamps &&
+		    ops->ordered_samples && !session->sample_id_all) {
+			session->ordered_samples.next_flush = ULLONG_MAX;
+			flush_sample_queue(session, ops);
+			ops->ordered_samples = false;
+		}
+		return ret;
 	case PERF_RECORD_HEADER_EVENT_TYPE:
 		return ops->event_type(event, session);
 	case PERF_RECORD_HEADER_TRACING_DATA:
Index: linux-2.6-tip/tools/perf/util/session.h
===================================================================
--- linux-2.6-tip.orig/tools/perf/util/session.h
+++ linux-2.6-tip/tools/perf/util/session.h
@@ -78,6 +78,7 @@ struct perf_event_ops {
 			build_id;
 	event_op2	finished_round;
 	bool		ordered_samples;
+	bool		ordering_requires_timestamps;
 };
 
 struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe);

Re: [patch 0/9] perf: Consolidate the event handling and ordering

[Arnaldo Carvalho de Melo]

Em Tue, Dec 07, 2010 at 12:48:38PM -0000, Thomas Gleixner escreveu:
> The following series fixes an unbound array access and further
> consolidates the event handling in session.c.
> 
> It now allows:
> 
>    - time ordered dumps
>    - break time ordering when not all events provide timestamps

Hi Ian,

	Have you had the chance of reviewing/testing this patch series
so that we know that your needs are satisfied with it?

Thanks,

- Arnaldo

Re: [patch 9/9] pref: session: Break event ordering when timestamps are missing

[Ian Munsie]

Excerpts from Thomas Gleixner's message of Tue Dec 07 12:49:04 UTC 2010:
> Allow the session client to specify that event ordering should be
> stopped when not all events have time stamps.

>  	/* These events are processed right away */
>  	switch (event->header.type) {
>  	case PERF_RECORD_HEADER_ATTR:
> -		return ops->attr(event, session);
> +		/* This updates session->sample_id_all */
> +		ret = ops->attr(event, session);
> +		/* Break ordering if sample_id_all is false */
> +		if (ops->ordering_requires_timestamps &&
> +		    ops->ordered_samples && !session->sample_id_all) {
> +			session->ordered_samples.next_flush = ULLONG_MAX;
> +			flush_sample_queue(session, ops);
> +			ops->ordered_samples = false;
> +		}
> +		return ret;

This fallback still relies on receiving a PERF_RECORD_HEADER_ATTR, which
will only happen if the output is being piped:

$ git grep event__synthesize_attr
builtin-record.c:               err = event__synthesize_attrs(&session->header,
util/header.c:int event__synthesize_attr(struct perf_event_attr *attr, u16 ids, u64 *id,
util/header.c:int event__synthesize_attrs(struct perf_header *self, event__handler_t process,
util/header.c:          err = event__synthesize_attr(&attr->attr, attr->ids, attr->id,
util/header.h:int event__synthesize_attr(struct perf_event_attr *attr, u16 ids, u64 *id,
util/header.h:int event__synthesize_attrs(struct perf_header *self,

$ less builtin-record.c
<SNIP>
	if (pipe_output) {
		err = event__synthesize_attrs(&session->header,
					      process_synthesized_event,
					      session);
<SNIP>

So this will work in this situation (timestamps on samples, but this is
an old kernel so not on other events):

delenn% ./perf record -T -o - ~/tests/cachetest | ./perf report -i -
Initialising array...
Trying approach 1: 0x18fff7eb26fd058
Trying approach 2: 0x18fff7eb26fd058
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.081 MB - (~3549 samples) ]
# Events: 1K cycles
#
# Overhead  Command      Shared Object                   Symbol
# ........  .......  .................  .......................
#
    70.23%  cachetest  cachetest          [.] sumArrayNaive
    28.54%  cachetest  cachetest          [.] sumArrayOptimal
     0.43%  cachetest  [kernel.kallsyms]  [k] insert_work
     0.39%  cachetest  [kernel.kallsyms]  [k] machine_kexec_prepare
     0.35%  cachetest  libc-2.11.2.so     [.] __random
     0.06%  cachetest  [kernel.kallsyms]  [k] print_cfs_rq
     0.01%  cachetest  [kernel.kallsyms]  [k] __register_sysctl_paths


#
# (For a higher level overview, try: perf report --sort comm,dso)
#


But will fail in this case:

delenn% ./perf record -T ~/tests/cachetest                                       ~/linus/tools/perf
Initialising array...
Trying approach 1: 0x18fff7eb26fd058
Trying approach 2: 0x18fff7eb26fd058
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.081 MB perf.data (~3543 samples) ]

delenn% ./perf report|cat                                                        ~/linus/tools/perf
# Events: 1K cycles
#
# Overhead  Command      Shared Object                   Symbol
# ........  .......  .................  .......................
#
    99.47%    :5593  [unknown]          [.] 0xf770cff5
     0.48%    :5593  [kernel.kallsyms]  [k] hpet_next_event
     0.05%    :5593  [libahci]          [k] ahci_scr_read
     0.01%    :5593  [kernel.kallsyms]  [k] flush_signal_handlers
     0.00%    :5593  [kernel.kallsyms]  [k] native_write_msr_safe


#
# (For a higher level overview, try: perf report --sort comm,dso)
#

Since the fall back isn't triggered, not only are COMM and MMAP events
processed first (from patch 2 in this series), but EXIT will as well,
which causes no userspace events to be attributed.

Cheers,
-Ian

Re: [patch 3/9] perf: session: Consolidate the dump code

[Ian Munsie]

Excerpts from Thomas Gleixner's message of Tue Dec 07 12:48:47 UTC 2010:
> The dump code used by perf report -D is scattered all over the
> place. Move it to separate functions.

It's cleaner than my patch, and does the same thing, so:

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [patch 4/9] perf: session: Store file offset in sample_queue

[Ian Munsie]

Excerpts from Thomas Gleixner's message of Tue Dec 07 12:48:50 UTC 2010:
> Subject: [patch 4/9] perf: session: Store file offset in sample_queue

Looks the same as mine, so:

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [patch 5/9] perf: session: Add file_offset to event delivery function

[Ian Munsie]

Excerpts from Thomas Gleixner's message of Tue Dec 07 12:48:53 UTC 2010:
> Subject: [patch 5/9] perf: session: Add file_offset to event delivery function

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

Re: [patch 6/9] perf: session: Move dump code to event delivery path

[Ian Munsie]

Excerpts from Thomas Gleixner's message of Tue Dec 07 12:48:55 UTC 2010:
> Subject: [patch 6/9] perf: session: Move dump code to event delivery path

Acked-by: Ian Munsie <imunsie@au1.ibm.com>

[PATCH v4] perf record,report,annotate,diff: Process events in order

[Ian Munsie]

From: Ian Munsie <imunsie@au1.ibm.com>

This patch changes perf report to ask for the ID info on all events be
default if recording from multiple CPUs.

Perf report, annotate and diff will now process the events in order if
the kernel is able to provide timestamps on all events. This ensures
that events such as COMM and MMAP which are necessary to correctly
interpret samples are processed prior to those samples so that they are
attributed correctly.

Before:
 # perf record ./cachetest
 # perf report

 # Events: 6K cycles
 #
 # Overhead  Command      Shared Object                           Symbol
 # ........  .......  .................  ...............................
 #
     74.11%    :3259  [unknown]          [k] 0x4a6c
      1.50%  cachetest  ld-2.11.2.so       [.] 0x1777c
      1.46%    :3259  [kernel.kallsyms]  [k] .perf_event_mmap_ctx
      1.25%    :3259  [kernel.kallsyms]  [k] restore
      0.74%    :3259  [kernel.kallsyms]  [k] ._raw_spin_lock
      0.71%    :3259  [kernel.kallsyms]  [k] .filemap_fault
      0.66%    :3259  [kernel.kallsyms]  [k] .memset
      0.54%  cachetest  [kernel.kallsyms]  [k] .sha_transform
      0.54%    :3259  [kernel.kallsyms]  [k] .copy_4K_page
      0.54%    :3259  [kernel.kallsyms]  [k] .find_get_page
      0.52%    :3259  [kernel.kallsyms]  [k] .trace_hardirqs_off
      0.50%    :3259  [kernel.kallsyms]  [k] .__do_fault
<SNIP>

After:
 # perf report

 # Events: 6K cycles
 #
 # Overhead  Command      Shared Object                           Symbol
 # ........  .......  .................  ...............................
 #
     44.28%  cachetest  cachetest          [.] sumArrayNaive
     22.53%  cachetest  cachetest          [.] sumArrayOptimal
      6.59%  cachetest  ld-2.11.2.so       [.] 0x1777c
      2.13%  cachetest  [unknown]          [k] 0x340
      1.46%  cachetest  [kernel.kallsyms]  [k] .perf_event_mmap_ctx
      1.25%  cachetest  [kernel.kallsyms]  [k] restore
      0.74%  cachetest  [kernel.kallsyms]  [k] ._raw_spin_lock
      0.71%  cachetest  [kernel.kallsyms]  [k] .filemap_fault
      0.66%  cachetest  [kernel.kallsyms]  [k] .memset
      0.54%  cachetest  [kernel.kallsyms]  [k] .copy_4K_page
      0.54%  cachetest  [kernel.kallsyms]  [k] .find_get_page
      0.54%  cachetest  [kernel.kallsyms]  [k] .sha_transform
      0.52%  cachetest  [kernel.kallsyms]  [k] .trace_hardirqs_off
      0.50%  cachetest  [kernel.kallsyms]  [k] .__do_fault
<SNIP>

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---

Changelog:
v4
- Rebased on Thomas' new userspace perf patches (with report -D reordering)
- Also cause perf annotate and perf diff to process events in order

v3
- Rebased on Thomas' userspace perf patch
- Dropped my userspace changes that Thomas' patch also addressed
- Dropped report -D reordering

v2
- Rebased on Arnaldo's ABI changes

v1
- Original patch with my ABI changes & report -D reordering

 tools/perf/builtin-annotate.c |    2 ++
 tools/perf/builtin-diff.c     |    2 ++
 tools/perf/builtin-record.c   |    5 ++++-
 tools/perf/builtin-report.c   |    2 ++
 4 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 569a276..793db36 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -375,6 +375,8 @@ static struct perf_event_ops event_ops = {
 	.mmap	= event__process_mmap,
 	.comm	= event__process_comm,
 	.fork	= event__process_task,
+	.ordered_samples = true,
+	.ordering_requires_timestamps = true,
 };
 
 static int __cmd_annotate(void)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 5e1a043..d21dc25a 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -61,6 +61,8 @@ static struct perf_event_ops event_ops = {
 	.exit	= event__process_task,
 	.fork	= event__process_task,
 	.lost	= event__process_lost,
+	.ordered_samples = true,
+	.ordering_requires_timestamps = true,
 };
 
 static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 699dd21..310dd21 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -285,7 +285,7 @@ static void create_counter(int counter, int cpu)
 	if (system_wide)
 		attr->sample_type	|= PERF_SAMPLE_CPU;
 
-	if (sample_time)
+	if (sample_time || system_wide || !no_inherit || cpu_list)
 		attr->sample_type	|= PERF_SAMPLE_TIME;
 
 	if (raw_samples) {
@@ -327,6 +327,9 @@ try_again:
 				 * Old kernel, no attr->sample_id_type_all field
 				 */
 				sample_id_all_avail = false;
+				if (!sample_time && !raw_samples)
+					attr->sample_type &= ~PERF_SAMPLE_TIME;
+
 				goto retry_sample_id;
 			}
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 904519f..5f01503 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -244,6 +244,8 @@ static struct perf_event_ops event_ops = {
 	.event_type = event__process_event_type,
 	.tracing_data = event__process_tracing_data,
 	.build_id = event__process_build_id,
+	.ordered_samples = true,
+	.ordering_requires_timestamps = true,
 };
 
 extern volatile int session_done;
-- 
1.7.2.3

Re: [patch 0/9] perf: Consolidate the event handling and ordering

[Ian Munsie]

Excerpts from Arnaldo Carvalho de Melo's message of Thu Dec 09 07:24:52 +1100 2010:
> Hi Ian,
> 
>     Have you had the chance of reviewing/testing this patch series
> so that we know that your needs are satisfied with it?

Hi Arnaldo,

Sorry I didn't get around to reviewing them yesterday. Overall they look
pretty good, other than the fall back issue running them the userspace
tool on an unpatched kernel I mentioned.

I've added my patch to enable the sorting in perf record. This time I
also enabled it in perf annotate and perf diff as I realised they would
also need the ordering to properly attribute the samples. I haven't
added it to any other commands - some already enable ordered_samples, so
we wouldn't want to enable ordering_requires_timestamps on them, and I
don't think any of the remaining commands need it.

Cheers,
-Ian

Re: [patch 9/9] pref: session: Break event ordering when timestamps are missing

[Thomas Gleixner]

On Thu, 9 Dec 2010, Ian Munsie wrote:
> Excerpts from Thomas Gleixner's message of Tue Dec 07 12:49:04 UTC 2010:
> > Allow the session client to specify that event ordering should be
> > stopped when not all events have time stamps.
> 
> >  	/* These events are processed right away */
> >  	switch (event->header.type) {
> >  	case PERF_RECORD_HEADER_ATTR:
> > -		return ops->attr(event, session);
> > +		/* This updates session->sample_id_all */
> > +		ret = ops->attr(event, session);
> > +		/* Break ordering if sample_id_all is false */
> > +		if (ops->ordering_requires_timestamps &&
> > +		    ops->ordered_samples && !session->sample_id_all) {
> > +			session->ordered_samples.next_flush = ULLONG_MAX;
> > +			flush_sample_queue(session, ops);
> > +			ops->ordered_samples = false;
> > +		}
> > +		return ret;
> Since the fall back isn't triggered, not only are COMM and MMAP events
> processed first (from patch 2 in this series), but EXIT will as well,
> which causes no userspace events to be attributed.

So we need to check this in every event processing path? Or do we have
for this kind of processing some other method which allows us to
disable the ordered_samples bit once ?

Thanks,

	tglx

Re: [patch 9/9] pref: session: Break event ordering when timestamps are missing

[Arnaldo Carvalho de Melo]

Em Thu, Dec 09, 2010 at 02:58:10PM +0100, Thomas Gleixner escreveu:
> On Thu, 9 Dec 2010, Ian Munsie wrote:
> > Excerpts from Thomas Gleixner's message of Tue Dec 07 12:49:04 UTC 2010:
> > > Allow the session client to specify that event ordering should be
> > > stopped when not all events have time stamps.
> > 
> > >  	/* These events are processed right away */
> > >  	switch (event->header.type) {
> > >  	case PERF_RECORD_HEADER_ATTR:
> > > -		return ops->attr(event, session);
> > > +		/* This updates session->sample_id_all */
> > > +		ret = ops->attr(event, session);
> > > +		/* Break ordering if sample_id_all is false */
> > > +		if (ops->ordering_requires_timestamps &&
> > > +		    ops->ordered_samples && !session->sample_id_all) {
> > > +			session->ordered_samples.next_flush = ULLONG_MAX;
> > > +			flush_sample_queue(session, ops);
> > > +			ops->ordered_samples = false;
> > > +		}
> > > +		return ret;
> > Since the fall back isn't triggered, not only are COMM and MMAP events
> > processed first (from patch 2 in this series), but EXIT will as well,
> > which causes no userspace events to be attributed.
> 
> So we need to check this in every event processing path? Or do we have
> for this kind of processing some other method which allows us to
> disable the ordered_samples bit once ?

FYI: I merged everything up to 8/9 and will push to Ingo soon, holler
if you disagree.

- Arnaldo

[tip:perf/core] perf event: Prevent unbound event__name array access

[tip-bot for Thomas Gleixner]

Commit-ID:  3835bc00c5b2d8e337a6e9d7b44f47e02760dba3
Gitweb:     http://git.kernel.org/tip/3835bc00c5b2d8e337a6e9d7b44f47e02760dba3
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:42 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 11:15:07 -0200

perf event: Prevent unbound event__name array access

event__name[] is missing an entry for PERF_RECORD_FINISHED_ROUND, but we
happily access the array from the dump code.

Make event__name[] static and provide an accessor function, fix up all
callers and add the missing string.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124550.432593943@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c   |   12 +++++++++++-
 tools/perf/util/event.h   |    2 +-
 tools/perf/util/hist.c    |    9 ++++++---
 tools/perf/util/session.c |    2 +-
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index e4cdc1e..183aedd 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -7,7 +7,7 @@
 #include "strlist.h"
 #include "thread.h"
 
-const char *event__name[] = {
+static const char *event__name[] = {
 	[0]			 = "TOTAL",
 	[PERF_RECORD_MMAP]	 = "MMAP",
 	[PERF_RECORD_LOST]	 = "LOST",
@@ -22,8 +22,18 @@ const char *event__name[] = {
 	[PERF_RECORD_HEADER_EVENT_TYPE]	 = "EVENT_TYPE",
 	[PERF_RECORD_HEADER_TRACING_DATA]	 = "TRACING_DATA",
 	[PERF_RECORD_HEADER_BUILD_ID]	 = "BUILD_ID",
+	[PERF_RECORD_FINISHED_ROUND]	 = "FINISHED_ROUND",
 };
 
+const char *event__get_event_name(unsigned int id)
+{
+	if (id >= ARRAY_SIZE(event__name))
+		return "INVALID";
+	if (!event__name[id])
+		return "UNKNOWN";
+	return event__name[id];
+}
+
 static struct sample_data synth_sample = {
 	.pid	   = -1,
 	.tid	   = -1,
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index a95ab18..4716a8f 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -171,6 +171,6 @@ int event__preprocess_sample(const event_t *self, struct perf_session *session,
 int event__parse_sample(const event_t *event, struct perf_session *session,
 			struct sample_data *sample);
 
-extern const char *event__name[];
+const char *event__get_event_name(unsigned int id);
 
 #endif /* __PERF_RECORD_H */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 2022e87..a3b8416 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1168,10 +1168,13 @@ size_t hists__fprintf_nr_events(struct hists *self, FILE *fp)
 	size_t ret = 0;
 
 	for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
-		if (!event__name[i])
+		const char *name = event__get_event_name(i);
+
+		if (!strcmp(name, "UNKNOWN"))
 			continue;
-		ret += fprintf(fp, "%10s events: %10d\n",
-			       event__name[i], self->stats.nr_events[i]);
+
+		ret += fprintf(fp, "%16s events: %10d\n", name,
+			       self->stats.nr_events[i]);
 	}
 
 	return ret;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 3074d38..b3b145a 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -718,7 +718,7 @@ static int perf_session__process_event(struct perf_session *session,
 	if (event->header.type < PERF_RECORD_HEADER_MAX) {
 		dump_printf("%#Lx [%#x]: PERF_RECORD_%s",
 			    file_offset, event->header.size,
-			    event__name[event->header.type]);
+			    event__get_event_name(event->header.type));
 		hists__inc_nr_events(&session->hists, event->header.type);
 	}
 

[tip:perf/core] perf session: Dont queue events w/o timestamps

[tip-bot for Thomas Gleixner]

Commit-ID:  79a14c1f458d598642bf11f09512c83d33a114e6
Gitweb:     http://git.kernel.org/tip/79a14c1f458d598642bf11f09512c83d33a114e6
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:44 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 11:15:38 -0200

perf session: Dont queue events w/o timestamps

If the event has no timestamp assigned then the parse code sets it to
~0ULL which causes the ordering code to enqueue it at the end.

Process it right away.

Reported-by: Ian Munsie <imunsie@au1.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124550.528788441@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index b3b145a..e5d0ed9 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -603,7 +603,7 @@ static int perf_session_queue_event(struct perf_session *s, event_t *event,
 	u64 timestamp = data->time;
 	struct sample_queue *new;
 
-	if (!timestamp)
+	if (!timestamp || timestamp == ~0ULL)
 		return -ETIME;
 
 	if (timestamp < s->ordered_samples.last_flush) {

[tip:perf/core] perf session: Consolidate the dump code

[tip-bot for Thomas Gleixner]

Commit-ID:  9aefcab0de472ee2b3ab195a6827ddd4b170e3a7
Gitweb:     http://git.kernel.org/tip/9aefcab0de472ee2b3ab195a6827ddd4b170e3a7
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:47 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 11:18:06 -0200

perf session: Consolidate the dump code

The dump code used by perf report -D is scattered all over the place.
Move it to separate functions.

Acked-by: Ian Munsie <imunsie@au1.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124550.625434869@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.h   |    1 +
 tools/perf/util/session.c |   64 +++++++++++++++++++++++++++-----------------
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 4716a8f..2b7e919 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -85,6 +85,7 @@ struct build_id_event {
 };
 
 enum perf_user_event_type { /* above any possible kernel type */
+	PERF_RECORD_USER_TYPE_START		= 64,
 	PERF_RECORD_HEADER_ATTR			= 64,
 	PERF_RECORD_HEADER_EVENT_TYPE		= 65,
 	PERF_RECORD_HEADER_TRACING_DATA		= 66,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index e5d0ed9..12761d5 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -665,6 +665,35 @@ static void perf_session__print_tstamp(struct perf_session *session,
 		printf("%Lu ", sample->time);
 }
 
+static void dump_event(struct perf_session *session, event_t *event,
+		       u64 file_offset, struct sample_data *sample)
+{
+	if (!dump_trace)
+		return;
+
+	dump_printf("\n%#Lx [%#x]: event: %d\n", file_offset,
+		    event->header.size, event->header.type);
+
+	trace_event(event);
+
+	if (sample)
+		perf_session__print_tstamp(session, event, sample);
+
+	dump_printf("%#Lx [%#x]: PERF_RECORD_%s",
+		    file_offset, event->header.size,
+		    event__get_event_name(event->header.type));
+}
+
+static void dump_sample(struct perf_session *session, event_t *event,
+			struct sample_data *sample)
+{
+	dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld\n", event->header.misc,
+		    sample->pid, sample->tid, sample->ip, sample->period);
+
+	if (session->sample_type & PERF_SAMPLE_CALLCHAIN)
+		callchain__dump(sample);
+}
+
 static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
@@ -703,32 +732,24 @@ static int perf_session__process_event(struct perf_session *session,
 	struct sample_data sample;
 	int ret;
 
-	trace_event(event);
-
 	if (session->header.needs_swap && event__swap_ops[event->header.type])
 		event__swap_ops[event->header.type](event);
 
-	if (event->header.type >= PERF_RECORD_MMAP &&
-	    event->header.type <= PERF_RECORD_SAMPLE) {
-		event__parse_sample(event, session, &sample);
-		if (dump_trace)
-			perf_session__print_tstamp(session, event, &sample);
-	}
+	if (event->header.type >= PERF_RECORD_HEADER_MAX)
+		return -EINVAL;
 
-	if (event->header.type < PERF_RECORD_HEADER_MAX) {
-		dump_printf("%#Lx [%#x]: PERF_RECORD_%s",
-			    file_offset, event->header.size,
-			    event__get_event_name(event->header.type));
-		hists__inc_nr_events(&session->hists, event->header.type);
+	hists__inc_nr_events(&session->hists, event->header.type);
+
+	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
+		dump_event(session, event, file_offset, NULL);
+	else {
+		event__parse_sample(event, session, &sample);
+		dump_event(session, event, file_offset, &sample);
 	}
 
 	/* These events are processed right away */
 	switch (event->header.type) {
 	case PERF_RECORD_SAMPLE:
-		dump_printf("(IP, %d): %d/%d: %#Lx period: %Ld\n",
-			    event->header.misc,
-			    sample.pid, sample.tid, sample.ip, sample.period);
-
 		if (session->sample_type & PERF_SAMPLE_CALLCHAIN) {
 			if (!ip_callchain__valid(sample.callchain, event)) {
 				pr_debug("call-chain problem with event, "
@@ -738,9 +759,8 @@ static int perf_session__process_event(struct perf_session *session,
 					sample.period;
 				return 0;
 			}
-
-			callchain__dump(&sample);
 		}
+		dump_sample(session, event, &sample);
 		break;
 
 	case PERF_RECORD_HEADER_ATTR:
@@ -870,9 +890,6 @@ more:
 
 	head += size;
 
-	dump_printf("\n%#Lx [%#x]: event: %d\n",
-		    head, event.header.size, event.header.type);
-
 	if (skip > 0)
 		head += skip;
 
@@ -961,9 +978,6 @@ more:
 
 	size = event->header.size;
 
-	dump_printf("\n%#Lx [%#x]: event: %d\n",
-		    file_pos, event->header.size, event->header.type);
-
 	if (size == 0 ||
 	    perf_session__process_event(session, event, ops, file_pos) < 0) {
 		dump_printf("%#Lx [%#x]: skipping unknown header type: %d\n",

[tip:perf/core] perf session: Store file offset in sample_queue

[tip-bot for Thomas Gleixner]

Commit-ID:  e4c2df132fef60a28b851abc1859a531e64f350c
Gitweb:     http://git.kernel.org/tip/e4c2df132fef60a28b851abc1859a531e64f350c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:50 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 12:09:18 -0200

perf session: Store file offset in sample_queue

Preparatory patch for ordered output of perf report -D.

Acked-by: Ian Munsie <imunsie@au1.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124550.725128545@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 12761d5..a433954 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -444,6 +444,7 @@ static event__swap_op event__swap_ops[] = {
 
 struct sample_queue {
 	u64			timestamp;
+	u64			file_offset;
 	event_t			*event;
 	struct list_head	list;
 };
@@ -596,7 +597,7 @@ static void __queue_event(struct sample_queue *new, struct perf_session *s)
 #define MAX_SAMPLE_BUFFER	(64 * 1024 / sizeof(struct sample_queue))
 
 static int perf_session_queue_event(struct perf_session *s, event_t *event,
-				    struct sample_data *data)
+				    struct sample_data *data, u64 file_offset)
 {
 	struct ordered_samples *os = &s->ordered_samples;
 	struct list_head *sc = &os->sample_cache;
@@ -628,6 +629,7 @@ static int perf_session_queue_event(struct perf_session *s, event_t *event,
 	}
 
 	new->timestamp = timestamp;
+	new->file_offset = file_offset;
 	new->event = event;
 
 	__queue_event(new, s);
@@ -780,7 +782,8 @@ static int perf_session__process_event(struct perf_session *session,
 	}
 
 	if (ops->ordered_samples) {
-		ret = perf_session_queue_event(session, event, &sample);
+		ret = perf_session_queue_event(session, event, &sample,
+					       file_offset);
 		if (ret != -ETIME)
 			return ret;
 	}

[tip:perf/core] perf session: Add file_offset to event delivery function

[tip-bot for Thomas Gleixner]

Commit-ID:  f74725dcf2f6931c26bc65e77e34e693eeb8441c
Gitweb:     http://git.kernel.org/tip/f74725dcf2f6931c26bc65e77e34e693eeb8441c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:53 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 12:09:51 -0200

perf session: Add file_offset to event delivery function

Preparatory patch for ordered output of perf report -D

Acked-by: Ian Munsie <imunsie@au1.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124550.818568607@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index a433954..d43e56c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -465,7 +465,8 @@ static void perf_session_free_sample_buffers(struct perf_session *session)
 static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
-				      struct perf_event_ops *ops);
+				      struct perf_event_ops *ops,
+				      u64 file_offset);
 
 static void flush_sample_queue(struct perf_session *s,
 			       struct perf_event_ops *ops)
@@ -485,7 +486,8 @@ static void flush_sample_queue(struct perf_session *s,
 			break;
 
 		event__parse_sample(iter->event, s, &sample);
-		perf_session_deliver_event(s, iter->event, &sample, ops);
+		perf_session_deliver_event(s, iter->event, &sample, ops,
+					   iter->file_offset);
 
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
@@ -699,7 +701,8 @@ static void dump_sample(struct perf_session *session, event_t *event,
 static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
-				      struct perf_event_ops *ops)
+				      struct perf_event_ops *ops,
+				      u64 file_offset __used)
 {
 	switch (event->header.type) {
 	case PERF_RECORD_SAMPLE:
@@ -788,7 +791,8 @@ static int perf_session__process_event(struct perf_session *session,
 			return ret;
 	}
 
-	return perf_session_deliver_event(session, event, &sample, ops);
+	return perf_session_deliver_event(session, event, &sample, ops,
+					  file_offset);
 }
 
 void perf_event_header__bswap(struct perf_event_header *self)

[tip:perf/core] perf session: Move dump code to event delivery path

[tip-bot for Thomas Gleixner]

Commit-ID:  532e7269c01098f0be6e08113c6947ec6ed11bfa
Gitweb:     http://git.kernel.org/tip/532e7269c01098f0be6e08113c6947ec6ed11bfa
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:55 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 12:10:53 -0200

perf session: Move dump code to event delivery path

Preparatory patch for ordered perf report -D

Acked-by: Ian Munsie <imunsie@au1.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124550.918655066@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d43e56c..7c5cc12 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -702,10 +702,13 @@ static int perf_session_deliver_event(struct perf_session *session,
 				      event_t *event,
 				      struct sample_data *sample,
 				      struct perf_event_ops *ops,
-				      u64 file_offset __used)
+				      u64 file_offset)
 {
+	dump_event(session, event, file_offset, sample);
+
 	switch (event->header.type) {
 	case PERF_RECORD_SAMPLE:
+		dump_sample(session, event, sample);
 		return ops->sample(event, sample, session);
 	case PERF_RECORD_MMAP:
 		return ops->mmap(event, sample, session);
@@ -747,10 +750,8 @@ static int perf_session__process_event(struct perf_session *session,
 
 	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
 		dump_event(session, event, file_offset, NULL);
-	else {
+	else
 		event__parse_sample(event, session, &sample);
-		dump_event(session, event, file_offset, &sample);
-	}
 
 	/* These events are processed right away */
 	switch (event->header.type) {
@@ -765,7 +766,6 @@ static int perf_session__process_event(struct perf_session *session,
 				return 0;
 			}
 		}
-		dump_sample(session, event, &sample);
 		break;
 
 	case PERF_RECORD_HEADER_ATTR:

[tip:perf/core] perf session: Split out sample preprocessing

[tip-bot for Thomas Gleixner]

Commit-ID:  3dfc2c0aee789843d18f6e4675658e6879465a56
Gitweb:     http://git.kernel.org/tip/3dfc2c0aee789843d18f6e4675658e6879465a56
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:48:58 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 12:13:13 -0200

perf session: Split out sample preprocessing

Simplify the code a bit.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124551.014649793@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   40 +++++++++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7c5cc12..a765b27 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -732,6 +732,22 @@ static int perf_session_deliver_event(struct perf_session *session,
 	}
 }
 
+static int perf_session__preprocess_sample(struct perf_session *session,
+					   event_t *event, struct sample_data *sample)
+{
+	if (event->header.type != PERF_RECORD_SAMPLE ||
+	    !(session->sample_type & PERF_SAMPLE_CALLCHAIN))
+		return 0;
+
+	if (!ip_callchain__valid(sample->callchain, event)) {
+		pr_debug("call-chain problem with event, skipping it.\n");
+		++session->hists.stats.nr_invalid_chains;
+		session->hists.stats.total_invalid_chains += sample->period;
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int perf_session__process_event(struct perf_session *session,
 				       event_t *event,
 				       struct perf_event_ops *ops,
@@ -750,24 +766,9 @@ static int perf_session__process_event(struct perf_session *session,
 
 	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
 		dump_event(session, event, file_offset, NULL);
-	else
-		event__parse_sample(event, session, &sample);
 
 	/* These events are processed right away */
 	switch (event->header.type) {
-	case PERF_RECORD_SAMPLE:
-		if (session->sample_type & PERF_SAMPLE_CALLCHAIN) {
-			if (!ip_callchain__valid(sample.callchain, event)) {
-				pr_debug("call-chain problem with event, "
-					 "skipping it.\n");
-				++session->hists.stats.nr_invalid_chains;
-				session->hists.stats.total_invalid_chains +=
-					sample.period;
-				return 0;
-			}
-		}
-		break;
-
 	case PERF_RECORD_HEADER_ATTR:
 		return ops->attr(event, session);
 	case PERF_RECORD_HEADER_EVENT_TYPE:
@@ -784,6 +785,15 @@ static int perf_session__process_event(struct perf_session *session,
 		break;
 	}
 
+	/*
+	 * For all kernel events we get the sample data
+	 */
+	event__parse_sample(event, session, &sample);
+
+	/* Preprocess sample records - precheck callchains */
+	if (perf_session__preprocess_sample(session, event, &sample))
+		return 0;
+
 	if (ops->ordered_samples) {
 		ret = perf_session_queue_event(session, event, &sample,
 					       file_offset);

[tip:perf/core] perf session: Split out user event processing

[tip-bot for Thomas Gleixner]

Commit-ID:  ba74f0640d963ccc914ac533cb0ba133ee07bcf2
Gitweb:     http://git.kernel.org/tip/ba74f0640d963ccc914ac533cb0ba133ee07bcf2
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 7 Dec 2010 12:49:01 +0000
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 9 Dec 2010 12:15:43 -0200

perf session: Split out user event processing

Simplify further.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20101207124551.110956235@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   42 ++++++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index a765b27..69760cd 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -748,24 +748,10 @@ static int perf_session__preprocess_sample(struct perf_session *session,
 	return 0;
 }
 
-static int perf_session__process_event(struct perf_session *session,
-				       event_t *event,
-				       struct perf_event_ops *ops,
-				       u64 file_offset)
+static int perf_session__process_user_event(struct perf_session *session, event_t *event,
+					    struct perf_event_ops *ops, u64 file_offset)
 {
-	struct sample_data sample;
-	int ret;
-
-	if (session->header.needs_swap && event__swap_ops[event->header.type])
-		event__swap_ops[event->header.type](event);
-
-	if (event->header.type >= PERF_RECORD_HEADER_MAX)
-		return -EINVAL;
-
-	hists__inc_nr_events(&session->hists, event->header.type);
-
-	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
-		dump_event(session, event, file_offset, NULL);
+	dump_event(session, event, file_offset, NULL);
 
 	/* These events are processed right away */
 	switch (event->header.type) {
@@ -782,8 +768,28 @@ static int perf_session__process_event(struct perf_session *session,
 	case PERF_RECORD_FINISHED_ROUND:
 		return ops->finished_round(event, session, ops);
 	default:
-		break;
+		return -EINVAL;
 	}
+}
+
+static int perf_session__process_event(struct perf_session *session,
+				       event_t *event,
+				       struct perf_event_ops *ops,
+				       u64 file_offset)
+{
+	struct sample_data sample;
+	int ret;
+
+	if (session->header.needs_swap && event__swap_ops[event->header.type])
+		event__swap_ops[event->header.type](event);
+
+	if (event->header.type >= PERF_RECORD_HEADER_MAX)
+		return -EINVAL;
+
+	hists__inc_nr_events(&session->hists, event->header.type);
+
+	if (event->header.type >= PERF_RECORD_USER_TYPE_START)
+		return perf_session__process_user_event(session, event, ops, file_offset);
 
 	/*
 	 * For all kernel events we get the sample data

Re: [patch 9/9] pref: session: Break event ordering when timestamps are missing

[Ian Munsie]

Excerpts from Thomas Gleixner's message of Fri Dec 10 00:58:10 +1100 2010:
> On Thu, 9 Dec 2010, Ian Munsie wrote:
> > Excerpts from Thomas Gleixner's message of Tue Dec 07 12:49:04 UTC 2010:
> > > Allow the session client to specify that event ordering should be
> > > stopped when not all events have time stamps.
> > 
> > >      /* These events are processed right away */
> > >      switch (event->header.type) {
> > >      case PERF_RECORD_HEADER_ATTR:
> > > -        return ops->attr(event, session);
> > > +        /* This updates session->sample_id_all */
> > > +        ret = ops->attr(event, session);
> > > +        /* Break ordering if sample_id_all is false */
> > > +        if (ops->ordering_requires_timestamps &&
> > > +            ops->ordered_samples && !session->sample_id_all) {
> > > +            session->ordered_samples.next_flush = ULLONG_MAX;
> > > +            flush_sample_queue(session, ops);
> > > +            ops->ordered_samples = false;
> > > +        }
> > > +        return ret;
> > Since the fall back isn't triggered, not only are COMM and MMAP events
> > processed first (from patch 2 in this series), but EXIT will as well,
> > which causes no userspace events to be attributed.
> 
> So we need to check this in every event processing path? Or do we have
> for this kind of processing some other method which allows us to
> disable the ordered_samples bit once ?

Any reason we couldn't put it in the perf_session__new function? It
means passing the event_ops to it as well, but I can't see any reason
why not to:


>From 711157f13a9adeab87a83eb8e0d9bc67cf1e2be8 Mon Sep 17 00:00:00 2001
From: Ian Munsie <imunsie@au1.ibm.com>
Date: Fri, 10 Dec 2010 14:09:16 +1100
Subject: [PATCH] perf session: Fallback to unordered processing if no sample_id_all

If we are running the new perf on an old kernel without support for
sample_id_all, we should fall back to the old unordered processing of
events. If we didn't than we would *always* process events without
timestamps out of order, whether or not we hit a reordering race. In
other words, instead of there being a chance of not attributing samples
correctly, we would guarantee that samples would not be attributed.

While processing all events without timestamps before events with
timestamps may seem like an intuitive solution, it falls down as
PERF_RECORD_EXIT events would also be processed before any samples.
Even with a workaround for that case, samples before/after an exec would
not be attributed correctly.

This patch allows commands to indicate whether they need to fall back to
unordered processing, so that commands that do not care about timestamps
on every event will not be affected. If we do fallback, this will print
out a warning if report -D was invoked.

This patch adds the test in perf_session__new so that we only need to
test once per session. Commands that do not use an event_ops (such as
record and top) can simply pass NULL in it's place.

Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
---
 tools/perf/builtin-annotate.c     |    2 +-
 tools/perf/builtin-buildid-list.c |    3 ++-
 tools/perf/builtin-diff.c         |    4 ++--
 tools/perf/builtin-inject.c       |    2 +-
 tools/perf/builtin-kmem.c         |    3 ++-
 tools/perf/builtin-lock.c         |    2 +-
 tools/perf/builtin-record.c       |    2 +-
 tools/perf/builtin-report.c       |    2 +-
 tools/perf/builtin-sched.c        |    3 ++-
 tools/perf/builtin-script.c       |    2 +-
 tools/perf/builtin-timechart.c    |    3 ++-
 tools/perf/builtin-top.c          |    2 +-
 tools/perf/util/session.c         |   11 ++++++++++-
 tools/perf/util/session.h         |    5 ++++-
 14 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 793db36..c056cdc 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -384,7 +384,7 @@ static int __cmd_annotate(void)
 	int ret;
 	struct perf_session *session;
 
-	session = perf_session__new(input_name, O_RDONLY, force, false);
+	session = perf_session__new(input_name, O_RDONLY, force, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 44a47e1..3b06f9c 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -39,7 +39,8 @@ static int __cmd_buildid_list(void)
 	int err = -1;
 	struct perf_session *session;
 
-	session = perf_session__new(input_name, O_RDONLY, force, false);
+	session = perf_session__new(input_name, O_RDONLY, force, false,
+				    &build_id__mark_dso_hit_ops);
 	if (session == NULL)
 		return -1;
 
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index d21dc25a..97846dc 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -144,8 +144,8 @@ static int __cmd_diff(void)
 	int ret, i;
 	struct perf_session *session[2];
 
-	session[0] = perf_session__new(input_old, O_RDONLY, force, false);
-	session[1] = perf_session__new(input_new, O_RDONLY, force, false);
+	session[0] = perf_session__new(input_old, O_RDONLY, force, false, &event_ops);
+	session[1] = perf_session__new(input_new, O_RDONLY, force, false, &event_ops);
 	if (session[0] == NULL || session[1] == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 4b66b85..0c78ffa 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -196,7 +196,7 @@ static int __cmd_inject(void)
 		inject_ops.tracing_data	= event__repipe_tracing_data;
 	}
 
-	session = perf_session__new(input_name, O_RDONLY, false, true);
+	session = perf_session__new(input_name, O_RDONLY, false, true, &inject_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index c9620ff..def7ddc 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -481,7 +481,8 @@ static void sort_result(void)
 static int __cmd_kmem(void)
 {
 	int err = -EINVAL;
-	struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index b41b449..b9c6e54 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -858,7 +858,7 @@ static struct perf_event_ops eops = {
 
 static int read_events(void)
 {
-	session = perf_session__new(input_name, O_RDONLY, 0, false);
+	session = perf_session__new(input_name, O_RDONLY, 0, false, &eops);
 	if (!session)
 		die("Initializing perf session failed\n");
 
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 310dd21..184c444 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -574,7 +574,7 @@ static int __cmd_record(int argc, const char **argv)
 	}
 
 	session = perf_session__new(output_name, O_WRONLY,
-				    write_mode == WRITE_FORCE, false);
+				    write_mode == WRITE_FORCE, false, NULL);
 	if (session == NULL) {
 		pr_err("Not enough memory for reading perf file header\n");
 		return -1;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index ccaea63..4af7ce6 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -310,7 +310,7 @@ static int __cmd_report(void)
 
 	signal(SIGINT, sig_handler);
 
-	session = perf_session__new(input_name, O_RDONLY, force, false);
+	session = perf_session__new(input_name, O_RDONLY, force, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index c775394..7a4ebeb 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1643,7 +1643,8 @@ static struct perf_event_ops event_ops = {
 static int read_events(void)
 {
 	int err = -EINVAL;
-	struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 54f1ea8..6ef65c0 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -779,7 +779,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 	if (!script_name)
 		setup_pager();
 
-	session = perf_session__new(input_name, O_RDONLY, 0, false);
+	session = perf_session__new(input_name, O_RDONLY, 0, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index d2fc461..459b5e3 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -937,7 +937,8 @@ static struct perf_event_ops event_ops = {
 
 static int __cmd_timechart(void)
 {
-	struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &event_ops);
 	int ret = -EINVAL;
 
 	if (session == NULL)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0515ce9..ae15f04 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1272,7 +1272,7 @@ static int __cmd_top(void)
 	 * FIXME: perf_session__new should allow passing a O_MMAP, so that all this
 	 * mmap reading, etc is encapsulated in it. Use O_WRONLY for now.
 	 */
-	struct perf_session *session = perf_session__new(NULL, O_WRONLY, false, false);
+	struct perf_session *session = perf_session__new(NULL, O_WRONLY, false, false, NULL);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index b59abf5..0f7e544 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -125,7 +125,9 @@ static void perf_session__destroy_kernel_maps(struct perf_session *self)
 	machines__destroy_guest_kernel_maps(&self->machines);
 }
 
-struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe)
+struct perf_session *perf_session__new(const char *filename, int mode,
+				       bool force, bool repipe,
+				       struct perf_event_ops *ops)
 {
 	size_t len = filename ? strlen(filename) + 1 : 0;
 	struct perf_session *self = zalloc(sizeof(*self) + len);
@@ -170,6 +172,13 @@ struct perf_session *perf_session__new(const char *filename, int mode, bool forc
 	}
 
 	perf_session__update_sample_type(self);
+
+	if (ops && ops->ordering_requires_timestamps &&
+	    ops->ordered_samples && !self->sample_id_all) {
+		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
+		ops->ordered_samples = false;
+	}
+
 out:
 	return self;
 out_free:
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index ac36f99..ffe4b98 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -78,9 +78,12 @@ struct perf_event_ops {
 			build_id;
 	event_op2	finished_round;
 	bool		ordered_samples;
+	bool		ordering_requires_timestamps;
 };
 
-struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe);
+struct perf_session *perf_session__new(const char *filename, int mode,
+				       bool force, bool repipe,
+				       struct perf_event_ops *ops);
 void perf_session__delete(struct perf_session *self);
 
 void perf_event_header__bswap(struct perf_event_header *self);
-- 
1.7.2.3

[tip:perf/core] perf session: Fallback to unordered processing if no sample_id_all

[tip-bot for Ian Munsie]

Commit-ID:  21ef97f05a7da5bc23b26cb34d6746f83ca9bf20
Gitweb:     http://git.kernel.org/tip/21ef97f05a7da5bc23b26cb34d6746f83ca9bf20
Author:     Ian Munsie <imunsie@au1.ibm.com>
AuthorDate: Fri, 10 Dec 2010 14:09:16 +1100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 20:17:51 -0200

perf session: Fallback to unordered processing if no sample_id_all

If we are running the new perf on an old kernel without support for
sample_id_all, we should fall back to the old unordered processing of
events. If we didn't than we would *always* process events without
timestamps out of order, whether or not we hit a reordering race. In
other words, instead of there being a chance of not attributing samples
correctly, we would guarantee that samples would not be attributed.

While processing all events without timestamps before events with
timestamps may seem like an intuitive solution, it falls down as
PERF_RECORD_EXIT events would also be processed before any samples.
Even with a workaround for that case, samples before/after an exec would
not be attributed correctly.

This patch allows commands to indicate whether they need to fall back to
unordered processing, so that commands that do not care about timestamps
on every event will not be affected. If we do fallback, this will print
out a warning if report -D was invoked.

This patch adds the test in perf_session__new so that we only need to
test once per session. Commands that do not use an event_ops (such as
record and top) can simply pass NULL in it's place.

Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <1291951882-sup-6069@au1.ibm.com>
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c     |    2 +-
 tools/perf/builtin-buildid-list.c |    3 ++-
 tools/perf/builtin-diff.c         |    4 ++--
 tools/perf/builtin-inject.c       |    2 +-
 tools/perf/builtin-kmem.c         |    3 ++-
 tools/perf/builtin-lock.c         |    2 +-
 tools/perf/builtin-record.c       |    2 +-
 tools/perf/builtin-report.c       |    2 +-
 tools/perf/builtin-sched.c        |    3 ++-
 tools/perf/builtin-script.c       |    2 +-
 tools/perf/builtin-timechart.c    |    3 ++-
 tools/perf/builtin-top.c          |    2 +-
 tools/perf/util/session.c         |   11 ++++++++++-
 tools/perf/util/session.h         |    5 ++++-
 14 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 569a276..48dbab4 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -382,7 +382,7 @@ static int __cmd_annotate(void)
 	int ret;
 	struct perf_session *session;
 
-	session = perf_session__new(input_name, O_RDONLY, force, false);
+	session = perf_session__new(input_name, O_RDONLY, force, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 44a47e1..3b06f9c 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -39,7 +39,8 @@ static int __cmd_buildid_list(void)
 	int err = -1;
 	struct perf_session *session;
 
-	session = perf_session__new(input_name, O_RDONLY, force, false);
+	session = perf_session__new(input_name, O_RDONLY, force, false,
+				    &build_id__mark_dso_hit_ops);
 	if (session == NULL)
 		return -1;
 
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 5e1a043..af84e1c 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -142,8 +142,8 @@ static int __cmd_diff(void)
 	int ret, i;
 	struct perf_session *session[2];
 
-	session[0] = perf_session__new(input_old, O_RDONLY, force, false);
-	session[1] = perf_session__new(input_new, O_RDONLY, force, false);
+	session[0] = perf_session__new(input_old, O_RDONLY, force, false, &event_ops);
+	session[1] = perf_session__new(input_new, O_RDONLY, force, false, &event_ops);
 	if (session[0] == NULL || session[1] == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 4b66b85..0c78ffa 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -196,7 +196,7 @@ static int __cmd_inject(void)
 		inject_ops.tracing_data	= event__repipe_tracing_data;
 	}
 
-	session = perf_session__new(input_name, O_RDONLY, false, true);
+	session = perf_session__new(input_name, O_RDONLY, false, true, &inject_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index c9620ff..def7ddc 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -481,7 +481,8 @@ static void sort_result(void)
 static int __cmd_kmem(void)
 {
 	int err = -EINVAL;
-	struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index b41b449..b9c6e54 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -858,7 +858,7 @@ static struct perf_event_ops eops = {
 
 static int read_events(void)
 {
-	session = perf_session__new(input_name, O_RDONLY, 0, false);
+	session = perf_session__new(input_name, O_RDONLY, 0, false, &eops);
 	if (!session)
 		die("Initializing perf session failed\n");
 
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e9be6ae..efd1b3c 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -572,7 +572,7 @@ static int __cmd_record(int argc, const char **argv)
 	}
 
 	session = perf_session__new(output_name, O_WRONLY,
-				    write_mode == WRITE_FORCE, false);
+				    write_mode == WRITE_FORCE, false, NULL);
 	if (session == NULL) {
 		pr_err("Not enough memory for reading perf file header\n");
 		return -1;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index b6a2a89..fd4c450 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -308,7 +308,7 @@ static int __cmd_report(void)
 
 	signal(SIGINT, sig_handler);
 
-	session = perf_session__new(input_name, O_RDONLY, force, false);
+	session = perf_session__new(input_name, O_RDONLY, force, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index c775394..7a4ebeb 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1643,7 +1643,8 @@ static struct perf_event_ops event_ops = {
 static int read_events(void)
 {
 	int err = -EINVAL;
-	struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 54f1ea8..6ef65c0 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -779,7 +779,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 	if (!script_name)
 		setup_pager();
 
-	session = perf_session__new(input_name, O_RDONLY, 0, false);
+	session = perf_session__new(input_name, O_RDONLY, 0, false, &event_ops);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index d2fc461..459b5e3 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -937,7 +937,8 @@ static struct perf_event_ops event_ops = {
 
 static int __cmd_timechart(void)
 {
-	struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
+	struct perf_session *session = perf_session__new(input_name, O_RDONLY,
+							 0, false, &event_ops);
 	int ret = -EINVAL;
 
 	if (session == NULL)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0515ce9..ae15f04 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1272,7 +1272,7 @@ static int __cmd_top(void)
 	 * FIXME: perf_session__new should allow passing a O_MMAP, so that all this
 	 * mmap reading, etc is encapsulated in it. Use O_WRONLY for now.
 	 */
-	struct perf_session *session = perf_session__new(NULL, O_WRONLY, false, false);
+	struct perf_session *session = perf_session__new(NULL, O_WRONLY, false, false, NULL);
 	if (session == NULL)
 		return -ENOMEM;
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index b59abf5..0f7e544 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -125,7 +125,9 @@ static void perf_session__destroy_kernel_maps(struct perf_session *self)
 	machines__destroy_guest_kernel_maps(&self->machines);
 }
 
-struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe)
+struct perf_session *perf_session__new(const char *filename, int mode,
+				       bool force, bool repipe,
+				       struct perf_event_ops *ops)
 {
 	size_t len = filename ? strlen(filename) + 1 : 0;
 	struct perf_session *self = zalloc(sizeof(*self) + len);
@@ -170,6 +172,13 @@ struct perf_session *perf_session__new(const char *filename, int mode, bool forc
 	}
 
 	perf_session__update_sample_type(self);
+
+	if (ops && ops->ordering_requires_timestamps &&
+	    ops->ordered_samples && !self->sample_id_all) {
+		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
+		ops->ordered_samples = false;
+	}
+
 out:
 	return self;
 out_free:
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index ac36f99..ffe4b98 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -78,9 +78,12 @@ struct perf_event_ops {
 			build_id;
 	event_op2	finished_round;
 	bool		ordered_samples;
+	bool		ordering_requires_timestamps;
 };
 
-struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe);
+struct perf_session *perf_session__new(const char *filename, int mode,
+				       bool force, bool repipe,
+				       struct perf_event_ops *ops);
 void perf_session__delete(struct perf_session *self);
 
 void perf_event_header__bswap(struct perf_event_header *self);

[tip:perf/core] perf record,report,annotate,diff: Process events in order

[tip-bot for Ian Munsie]

Commit-ID:  eac23d1c384b55e4bbb89ea9e5a6bb77fb4d1140
Gitweb:     http://git.kernel.org/tip/eac23d1c384b55e4bbb89ea9e5a6bb77fb4d1140
Author:     Ian Munsie <imunsie@au1.ibm.com>
AuthorDate: Thu, 9 Dec 2010 16:33:53 +1100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 20:17:51 -0200

perf record,report,annotate,diff: Process events in order

This patch changes perf report to ask for the ID info on all events be
default if recording from multiple CPUs.

Perf report, annotate and diff will now process the events in order if
the kernel is able to provide timestamps on all events. This ensures
that events such as COMM and MMAP which are necessary to correctly
interpret samples are processed prior to those samples so that they are
attributed correctly.

Before:
 # perf record ./cachetest
 # perf report

 # Events: 6K cycles
 #
 # Overhead  Command      Shared Object                           Symbol
 # ........  .......  .................  ...............................
 #
     74.11%    :3259  [unknown]          [k] 0x4a6c
      1.50%  cachetest  ld-2.11.2.so       [.] 0x1777c
      1.46%    :3259  [kernel.kallsyms]  [k] .perf_event_mmap_ctx
      1.25%    :3259  [kernel.kallsyms]  [k] restore
      0.74%    :3259  [kernel.kallsyms]  [k] ._raw_spin_lock
      0.71%    :3259  [kernel.kallsyms]  [k] .filemap_fault
      0.66%    :3259  [kernel.kallsyms]  [k] .memset
      0.54%  cachetest  [kernel.kallsyms]  [k] .sha_transform
      0.54%    :3259  [kernel.kallsyms]  [k] .copy_4K_page
      0.54%    :3259  [kernel.kallsyms]  [k] .find_get_page
      0.52%    :3259  [kernel.kallsyms]  [k] .trace_hardirqs_off
      0.50%    :3259  [kernel.kallsyms]  [k] .__do_fault
<SNIP>

After:
 # perf report

 # Events: 6K cycles
 #
 # Overhead  Command      Shared Object                           Symbol
 # ........  .......  .................  ...............................
 #
     44.28%  cachetest  cachetest          [.] sumArrayNaive
     22.53%  cachetest  cachetest          [.] sumArrayOptimal
      6.59%  cachetest  ld-2.11.2.so       [.] 0x1777c
      2.13%  cachetest  [unknown]          [k] 0x340
      1.46%  cachetest  [kernel.kallsyms]  [k] .perf_event_mmap_ctx
      1.25%  cachetest  [kernel.kallsyms]  [k] restore
      0.74%  cachetest  [kernel.kallsyms]  [k] ._raw_spin_lock
      0.71%  cachetest  [kernel.kallsyms]  [k] .filemap_fault
      0.66%  cachetest  [kernel.kallsyms]  [k] .memset
      0.54%  cachetest  [kernel.kallsyms]  [k] .copy_4K_page
      0.54%  cachetest  [kernel.kallsyms]  [k] .find_get_page
      0.54%  cachetest  [kernel.kallsyms]  [k] .sha_transform
      0.52%  cachetest  [kernel.kallsyms]  [k] .trace_hardirqs_off
      0.50%  cachetest  [kernel.kallsyms]  [k] .__do_fault
<SNIP>

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <1291872833-839-1-git-send-email-imunsie@au1.ibm.com>
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c |    2 ++
 tools/perf/builtin-diff.c     |    2 ++
 tools/perf/builtin-record.c   |    5 ++++-
 tools/perf/builtin-report.c   |    2 ++
 4 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 48dbab4..c056cdc 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -375,6 +375,8 @@ static struct perf_event_ops event_ops = {
 	.mmap	= event__process_mmap,
 	.comm	= event__process_comm,
 	.fork	= event__process_task,
+	.ordered_samples = true,
+	.ordering_requires_timestamps = true,
 };
 
 static int __cmd_annotate(void)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index af84e1c..97846dc 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -61,6 +61,8 @@ static struct perf_event_ops event_ops = {
 	.exit	= event__process_task,
 	.fork	= event__process_task,
 	.lost	= event__process_lost,
+	.ordered_samples = true,
+	.ordering_requires_timestamps = true,
 };
 
 static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index efd1b3c..5149e3d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -285,7 +285,7 @@ static void create_counter(int counter, int cpu)
 	if (system_wide)
 		attr->sample_type	|= PERF_SAMPLE_CPU;
 
-	if (sample_time)
+	if (sample_time || system_wide || !no_inherit || cpu_list)
 		attr->sample_type	|= PERF_SAMPLE_TIME;
 
 	if (raw_samples) {
@@ -327,6 +327,9 @@ try_again:
 				 * Old kernel, no attr->sample_id_type_all field
 				 */
 				sample_id_all_avail = false;
+				if (!sample_time && !raw_samples)
+					attr->sample_type &= ~PERF_SAMPLE_TIME;
+
 				goto retry_sample_id;
 			}
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index fd4c450..4af7ce6 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -244,6 +244,8 @@ static struct perf_event_ops event_ops = {
 	.event_type = event__process_event_type,
 	.tracing_data = event__process_tracing_data,
 	.build_id = event__process_build_id,
+	.ordered_samples = true,
+	.ordering_requires_timestamps = true,
 };
 
 extern volatile int session_done;