2010-12-07 12:49:37

by Thomas Gleixner

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

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

Suggested-by: Ian Munsie <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
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);


2010-12-09 03:50:23

by Ian Munsie

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

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

2010-12-09 13:58:41

by Thomas Gleixner

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

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

2010-12-09 17:33:09

by Arnaldo Carvalho de Melo

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

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

2010-12-10 03:36:40

by Ian Munsie

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

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 <[email protected]>
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 <[email protected]>
---
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

2010-12-22 11:29:44

by Ian Munsie

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

Commit-ID: 21ef97f05a7da5bc23b26cb34d6746f83ca9bf20
Gitweb: http://git.kernel.org/tip/21ef97f05a7da5bc23b26cb34d6746f83ca9bf20
Author: Ian Munsie <[email protected]>
AuthorDate: Fri, 10 Dec 2010 14:09:16 +1100
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ian Munsie <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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);