Commit bdf96c4e authored by Ian Rogers's avatar Ian Rogers Committed by Namhyung Kim
Browse files

perf tool_pmu: Use old_count when computing count values for time events 



When running in interval mode every third count of a time event isn't
showing properly:
```
$ perf stat -e duration_time -a -I 1000
     1.001082862      1,002,290,425      duration_time
     2.004264262      1,003,183,516      duration_time
     3.007381401      <not counted>      duration_time
     4.011160141      1,003,705,631      duration_time
     5.014515385      1,003,290,110      duration_time
     6.018539680      <not counted>      duration_time
     7.022065321      1,003,591,720      duration_time
```
The regression came in with a different fix, found through bisection,
commit 68cb1567 ("perf tool_pmu: Fix aggregation on
duration_time"). The issue is caused by the enabled and running time
of the event matching the old_count's and creating a delta of 0, which
is indicative of an error.

Fixes: 68cb1567 ("perf tool_pmu: Fix aggregation on duration_time")
Signed-off-by: default avatarIan Rogers <irogers@google.com>
Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
parent f69d34e8

tools/perf/builtin-stat.c

+13 −3
Original line number Diff line number Diff line
@@ -283,17 +283,27 @@ static int read_single_counter(struct evsel *counter, int cpu_map_idx, int threa 
	if (err && cpu_map_idx == 0 &&

	    (evsel__tool_event(counter) == TOOL_PMU__EVENT_USER_TIME ||

	     evsel__tool_event(counter) == TOOL_PMU__EVENT_SYSTEM_TIME)) {

		u64 val, *start_time;

		struct perf_counts_values *count =

			perf_counts(counter->counts, cpu_map_idx, thread);

		struct perf_counts_values *old_count = NULL;

		u64 val;



		if (counter->prev_raw_counts)

			old_count = perf_counts(counter->prev_raw_counts, cpu_map_idx, thread);



		start_time = xyarray__entry(counter->start_times, cpu_map_idx, thread);

		if (evsel__tool_event(counter) == TOOL_PMU__EVENT_USER_TIME)

			val = ru_stats.ru_utime_usec_stat.mean;

		else

			val = ru_stats.ru_stime_usec_stat.mean;

		count->ena = count->run = *start_time + val;



		count->val = val;

		if (old_count) {

			count->run = old_count->run + 1;

			count->ena = old_count->ena + 1;

		} else {

			count->run++;

			count->ena++;

		}

		return 0;

	}

	return err;

tools/perf/util/tool_pmu.c

+33 −26
Original line number Diff line number Diff line
@@ -446,16 +446,39 @@ bool tool_pmu__read_event(enum tool_pmu_event ev, 
	}

}



static void perf_counts__update(struct perf_counts_values *count,

				const struct perf_counts_values *old_count,

				bool raw, u64 val)

{

	/*

	 * The values of enabled and running must make a ratio of 100%. The

	 * exact values don't matter as long as they are non-zero to avoid

	 * issues with evsel__count_has_error.

	 */

	if (old_count) {

		count->val = raw ? val : old_count->val + val;

		count->run = old_count->run + 1;

		count->ena = old_count->ena + 1;

		count->lost = old_count->lost;

	} else {

		count->val = val;

		count->run++;

		count->ena++;

		count->lost = 0;

	}

}



int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread)

{

	__u64 *start_time, cur_time, delta_start;

	u64 val;

	int fd, err = 0;

	int err = 0;

	struct perf_counts_values *count, *old_count = NULL;

	bool adjust = false;

	enum tool_pmu_event ev = evsel__tool_event(evsel);



	count = perf_counts(evsel->counts, cpu_map_idx, thread);

	if (evsel->prev_raw_counts)

		old_count = perf_counts(evsel->prev_raw_counts, cpu_map_idx, thread);



	switch (ev) {

	case TOOL_PMU__EVENT_HAS_PMEM:
@@ -466,12 +489,11 @@ int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread) 
	case TOOL_PMU__EVENT_NUM_PACKAGES:

	case TOOL_PMU__EVENT_SLOTS:

	case TOOL_PMU__EVENT_SMT_ON:

	case TOOL_PMU__EVENT_SYSTEM_TSC_FREQ:

	case TOOL_PMU__EVENT_CORE_WIDE:

	case TOOL_PMU__EVENT_TARGET_CPU:

		if (evsel->prev_raw_counts)

			old_count = perf_counts(evsel->prev_raw_counts, cpu_map_idx, thread);

		val = 0;

	case TOOL_PMU__EVENT_SYSTEM_TSC_FREQ: {

		u64 val = 0;



		if (cpu_map_idx == 0 && thread == 0) {

			if (!tool_pmu__read_event(ev, evsel,

						  stat_config.system_wide,
@@ -481,16 +503,9 @@ int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread) 
				val = 0;

			}

		}

		if (old_count) {

			count->val = old_count->val + val;

			count->run = old_count->run + 1;

			count->ena = old_count->ena + 1;

		} else {

			count->val = val;

			count->run++;

			count->ena++;

		}

		perf_counts__update(count, old_count, /*raw=*/false, val);

		return 0;

	}

	case TOOL_PMU__EVENT_DURATION_TIME:

		/*

		 * Pretend duration_time is only on the first CPU and thread, or
@@ -506,9 +521,9 @@ int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread) 
	case TOOL_PMU__EVENT_USER_TIME:

	case TOOL_PMU__EVENT_SYSTEM_TIME: {

		bool system = evsel__tool_event(evsel) == TOOL_PMU__EVENT_SYSTEM_TIME;

		int fd = FD(evsel, cpu_map_idx, thread);



		start_time = xyarray__entry(evsel->start_times, cpu_map_idx, thread);

		fd = FD(evsel, cpu_map_idx, thread);

		lseek(fd, SEEK_SET, 0);

		if (evsel->pid_stat) {

			/* The event exists solely on 1 CPU. */
@@ -542,17 +557,9 @@ int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread) 
	if (adjust) {

		__u64 ticks_per_sec = sysconf(_SC_CLK_TCK);



		delta_start *= 1000000000 / ticks_per_sec;

		delta_start *= 1e9 / ticks_per_sec;

	}

	count->val    = delta_start;

	count->lost   = 0;

	/*

	 * The values of enabled and running must make a ratio of 100%. The

	 * exact values don't matter as long as they are non-zero to avoid

	 * issues with evsel__count_has_error.

	 */

	count->ena++;

	count->run++;

	perf_counts__update(count, old_count, /*raw=*/true, delta_start);

	return 0;

}