Commit 3df33eff authored by Mark Rutland's avatar Mark Rutland Committed by Arnaldo Carvalho de Melo

perf stat: Avoid skew when reading events

When we don't have a tracee (i.e. we're attaching to a task or CPU),
counters can still be running after our workload finishes, and can still
be running as we read their values. As we read events one-by-one, there
can be arbitrary skew between values of events, even within a group.
This means that ratios within an event group are not reliable.

This skew can be seen if measuring a group of identical events, e.g:

  # perf stat -a -C0 -e '{cycles,cycles}' sleep 1

To avoid this, we must stop groups from counting before we read the
values of any constituent events. This patch adds and makes use of a new
disable_counters() helper, which disables group leaders (and thus each
group as a whole). This mirrors the use of enable_counters() for
starting event groups in the absence of a tracee.

Closing a group leader splits the group, and without a disabled group
leader the newly split events will begin counting. Thus to ensure counts
are reliable we must defer closing group leaders until all counts have
been read. To do so this patch removes the event closing logic from the
read_counters() helper, explicitly closes the events using
perf_evlist__close(), which also aids legibility.
Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1470747869-3567-1-git-send-email-mark.rutland@arm.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent cb3f3378
...@@ -331,7 +331,7 @@ static int read_counter(struct perf_evsel *counter) ...@@ -331,7 +331,7 @@ static int read_counter(struct perf_evsel *counter)
return 0; return 0;
} }
static void read_counters(bool close_counters) static void read_counters(void)
{ {
struct perf_evsel *counter; struct perf_evsel *counter;
...@@ -341,11 +341,6 @@ static void read_counters(bool close_counters) ...@@ -341,11 +341,6 @@ static void read_counters(bool close_counters)
if (perf_stat_process_counter(&stat_config, counter)) if (perf_stat_process_counter(&stat_config, counter))
pr_warning("failed to process counter %s\n", counter->name); pr_warning("failed to process counter %s\n", counter->name);
if (close_counters) {
perf_evsel__close_fd(counter, perf_evsel__nr_cpus(counter),
thread_map__nr(evsel_list->threads));
}
} }
} }
...@@ -353,7 +348,7 @@ static void process_interval(void) ...@@ -353,7 +348,7 @@ static void process_interval(void)
{ {
struct timespec ts, rs; struct timespec ts, rs;
read_counters(false); read_counters();
clock_gettime(CLOCK_MONOTONIC, &ts); clock_gettime(CLOCK_MONOTONIC, &ts);
diff_timespec(&rs, &ts, &ref_time); diff_timespec(&rs, &ts, &ref_time);
...@@ -380,6 +375,17 @@ static void enable_counters(void) ...@@ -380,6 +375,17 @@ static void enable_counters(void)
perf_evlist__enable(evsel_list); perf_evlist__enable(evsel_list);
} }
static void disable_counters(void)
{
/*
* If we don't have tracee (attaching to task or cpu), counters may
* still be running. To get accurate group ratios, we must stop groups
* from counting before reading their constituent counters.
*/
if (!target__none(&target))
perf_evlist__disable(evsel_list);
}
static volatile int workload_exec_errno; static volatile int workload_exec_errno;
/* /*
...@@ -657,11 +663,20 @@ static int __run_perf_stat(int argc, const char **argv) ...@@ -657,11 +663,20 @@ static int __run_perf_stat(int argc, const char **argv)
} }
} }
disable_counters();
t1 = rdclock(); t1 = rdclock();
update_stats(&walltime_nsecs_stats, t1 - t0); update_stats(&walltime_nsecs_stats, t1 - t0);
read_counters(true); /*
* Closing a group leader splits the group, and as we only disable
* group leaders, results in remaining events becoming enabled. To
* avoid arbitrary skew, we must read all counters before closing any
* group leaders.
*/
read_counters();
perf_evlist__close(evsel_list);
return WEXITSTATUS(status); return WEXITSTATUS(status);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment