Commit 21ef97f05a7da5bc23b26cb34d6746f83ca9bf20

Authored by Ian Munsie
Committed by Arnaldo Carvalho de Melo
1 parent 7639dae0ca

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>

Showing 14 changed files with 31 additions and 15 deletions Side-by-side Diff

tools/perf/builtin-annotate.c
... ... @@ -382,7 +382,7 @@
382 382 int ret;
383 383 struct perf_session *session;
384 384  
385   - session = perf_session__new(input_name, O_RDONLY, force, false);
  385 + session = perf_session__new(input_name, O_RDONLY, force, false, &event_ops);
386 386 if (session == NULL)
387 387 return -ENOMEM;
388 388  
tools/perf/builtin-buildid-list.c
... ... @@ -39,7 +39,8 @@
39 39 int err = -1;
40 40 struct perf_session *session;
41 41  
42   - session = perf_session__new(input_name, O_RDONLY, force, false);
  42 + session = perf_session__new(input_name, O_RDONLY, force, false,
  43 + &build_id__mark_dso_hit_ops);
43 44 if (session == NULL)
44 45 return -1;
45 46  
tools/perf/builtin-diff.c
... ... @@ -142,8 +142,8 @@
142 142 int ret, i;
143 143 struct perf_session *session[2];
144 144  
145   - session[0] = perf_session__new(input_old, O_RDONLY, force, false);
146   - session[1] = perf_session__new(input_new, O_RDONLY, force, false);
  145 + session[0] = perf_session__new(input_old, O_RDONLY, force, false, &event_ops);
  146 + session[1] = perf_session__new(input_new, O_RDONLY, force, false, &event_ops);
147 147 if (session[0] == NULL || session[1] == NULL)
148 148 return -ENOMEM;
149 149  
tools/perf/builtin-inject.c
... ... @@ -196,7 +196,7 @@
196 196 inject_ops.tracing_data = event__repipe_tracing_data;
197 197 }
198 198  
199   - session = perf_session__new(input_name, O_RDONLY, false, true);
  199 + session = perf_session__new(input_name, O_RDONLY, false, true, &inject_ops);
200 200 if (session == NULL)
201 201 return -ENOMEM;
202 202  
tools/perf/builtin-kmem.c
... ... @@ -481,7 +481,8 @@
481 481 static int __cmd_kmem(void)
482 482 {
483 483 int err = -EINVAL;
484   - struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
  484 + struct perf_session *session = perf_session__new(input_name, O_RDONLY,
  485 + 0, false, &event_ops);
485 486 if (session == NULL)
486 487 return -ENOMEM;
487 488  
tools/perf/builtin-lock.c
... ... @@ -858,7 +858,7 @@
858 858  
859 859 static int read_events(void)
860 860 {
861   - session = perf_session__new(input_name, O_RDONLY, 0, false);
  861 + session = perf_session__new(input_name, O_RDONLY, 0, false, &eops);
862 862 if (!session)
863 863 die("Initializing perf session failed\n");
864 864  
tools/perf/builtin-record.c
... ... @@ -572,7 +572,7 @@
572 572 }
573 573  
574 574 session = perf_session__new(output_name, O_WRONLY,
575   - write_mode == WRITE_FORCE, false);
  575 + write_mode == WRITE_FORCE, false, NULL);
576 576 if (session == NULL) {
577 577 pr_err("Not enough memory for reading perf file header\n");
578 578 return -1;
tools/perf/builtin-report.c
... ... @@ -308,7 +308,7 @@
308 308  
309 309 signal(SIGINT, sig_handler);
310 310  
311   - session = perf_session__new(input_name, O_RDONLY, force, false);
  311 + session = perf_session__new(input_name, O_RDONLY, force, false, &event_ops);
312 312 if (session == NULL)
313 313 return -ENOMEM;
314 314  
tools/perf/builtin-sched.c
... ... @@ -1643,7 +1643,8 @@
1643 1643 static int read_events(void)
1644 1644 {
1645 1645 int err = -EINVAL;
1646   - struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
  1646 + struct perf_session *session = perf_session__new(input_name, O_RDONLY,
  1647 + 0, false, &event_ops);
1647 1648 if (session == NULL)
1648 1649 return -ENOMEM;
1649 1650  
tools/perf/builtin-script.c
... ... @@ -779,7 +779,7 @@
779 779 if (!script_name)
780 780 setup_pager();
781 781  
782   - session = perf_session__new(input_name, O_RDONLY, 0, false);
  782 + session = perf_session__new(input_name, O_RDONLY, 0, false, &event_ops);
783 783 if (session == NULL)
784 784 return -ENOMEM;
785 785  
tools/perf/builtin-timechart.c
... ... @@ -937,7 +937,8 @@
937 937  
938 938 static int __cmd_timechart(void)
939 939 {
940   - struct perf_session *session = perf_session__new(input_name, O_RDONLY, 0, false);
  940 + struct perf_session *session = perf_session__new(input_name, O_RDONLY,
  941 + 0, false, &event_ops);
941 942 int ret = -EINVAL;
942 943  
943 944 if (session == NULL)
tools/perf/builtin-top.c
... ... @@ -1272,7 +1272,7 @@
1272 1272 * FIXME: perf_session__new should allow passing a O_MMAP, so that all this
1273 1273 * mmap reading, etc is encapsulated in it. Use O_WRONLY for now.
1274 1274 */
1275   - struct perf_session *session = perf_session__new(NULL, O_WRONLY, false, false);
  1275 + struct perf_session *session = perf_session__new(NULL, O_WRONLY, false, false, NULL);
1276 1276 if (session == NULL)
1277 1277 return -ENOMEM;
1278 1278  
tools/perf/util/session.c
... ... @@ -125,7 +125,9 @@
125 125 machines__destroy_guest_kernel_maps(&self->machines);
126 126 }
127 127  
128   -struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe)
  128 +struct perf_session *perf_session__new(const char *filename, int mode,
  129 + bool force, bool repipe,
  130 + struct perf_event_ops *ops)
129 131 {
130 132 size_t len = filename ? strlen(filename) + 1 : 0;
131 133 struct perf_session *self = zalloc(sizeof(*self) + len);
... ... @@ -170,6 +172,13 @@
170 172 }
171 173  
172 174 perf_session__update_sample_type(self);
  175 +
  176 + if (ops && ops->ordering_requires_timestamps &&
  177 + ops->ordered_samples && !self->sample_id_all) {
  178 + dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
  179 + ops->ordered_samples = false;
  180 + }
  181 +
173 182 out:
174 183 return self;
175 184 out_free:
tools/perf/util/session.h
... ... @@ -78,9 +78,12 @@
78 78 build_id;
79 79 event_op2 finished_round;
80 80 bool ordered_samples;
  81 + bool ordering_requires_timestamps;
81 82 };
82 83  
83   -struct perf_session *perf_session__new(const char *filename, int mode, bool force, bool repipe);
  84 +struct perf_session *perf_session__new(const char *filename, int mode,
  85 + bool force, bool repipe,
  86 + struct perf_event_ops *ops);
84 87 void perf_session__delete(struct perf_session *self);
85 88  
86 89 void perf_event_header__bswap(struct perf_event_header *self);