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>

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 569a2761b90ac037c3eba06dc181a23c939f5c81..48dbab4b482f700f7894fd5581301547d3da7e02 100644 (file)
--- 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 44a47e13bd673eb362f956b80bd02adbec367d45..3b06f9ca263810988f2acd6c49d30cb077ce5c17 100644 (file)
--- 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 5e1a043aae038154c5c27a1199d753a8f1f019c0..af84e1c0519d72c5d6efd2e3c4c94641bb39c216 100644 (file)
--- 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 4b66b85794109be0248928ba9eacf15ed68d41e2..0c78ffa7bf675f46c9e631d9fa8d51fbc71aded4 100644 (file)
--- 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 c9620ff6496ff67bf36836913fa98154d1ef0726..def7ddc2fd4fbc1b1c57795f4c10c519729b035d 100644 (file)
--- 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 b41b4492b1cc491d7ab941cc3bd709a92b2b6f1e..b9c6e54329713e326d74da9b08164e562e521d03 100644 (file)
--- 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 e9be6ae87a278dd5230d04eeb5c1fe29771016fb..efd1b3c3d4a03cc33f61b4b615af720f0c46fdd4 100644 (file)
--- 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 b6a2a899aa8fb900ef1c86899ab176e8d6b34264..fd4c4500cd15ee6712385bf1b1fdff32f7969fa8 100644 (file)
--- 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 c7753940aea043690ac3d0ed593777c1b143f233..7a4ebeb8b016b4ca01c14f393b3c5fab98567523 100644 (file)
--- 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 54f1ea808db5374a2680654d2d9effdcaa3322a3..6ef65c04ab9a26ee4ecdbf289d2dd9980b5edc25 100644 (file)
--- 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 d2fc46103f834ae314b2d631f2ef265e8b203e0f..459b5e3db267a556ebc8b3b83dbebe4b9b0514ec 100644 (file)
--- 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 0515ce9d3d3eaa7dc34949bb5f109761e3844dc9..ae15f046c405844835d2de82332aae7c39cd84fe 100644 (file)
--- 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 b59abf5aba36eaeb6019ffcf4994b4acf2f9c58c..0f7e544544f569c3246f44e3379bd4453dca119b 100644 (file)
--- 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 ac36f99f14af534952ef95ae87fea15139da4dc3..ffe4b98db8f0883b3071dbc6a63a7cb50301da90 100644 (file)
--- 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);