Skip to content

Commit

Permalink
perf session: Fallback to unordered processing if no sample_id_all
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Ian Munsie authored and Arnaldo Carvalho de Melo committed Dec 21, 2010
1 parent 7639dae commit 21ef97f
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 15 deletions.
2 changes: 1 addition & 1 deletion tools/perf/builtin-annotate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion tools/perf/builtin-buildid-list.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions tools/perf/builtin-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion tools/perf/builtin-inject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion tools/perf/builtin-kmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion tools/perf/builtin-lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion tools/perf/builtin-record.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tools/perf/builtin-report.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion tools/perf/builtin-sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion tools/perf/builtin-script.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion tools/perf/builtin-timechart.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tools/perf/builtin-top.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
11 changes: 10 additions & 1 deletion tools/perf/util/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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:
Expand Down
5 changes: 4 additions & 1 deletion tools/perf/util/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 21ef97f

Please sign in to comment.