aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNamhyung Kim <namhyung@kernel.org>2022-12-12 15:08:20 -0800
committerArnaldo Carvalho de Melo <acme@redhat.com>2022-12-14 15:28:19 -0300
commitc587e77e100fa40eb6af10e00497c67acf493f33 (patch)
treeb282536e3eef1c81e5486c381a16705b612d4eea
parent5f8f95673f68914419e8657b3263736264b40e16 (diff)
downloadnios2-c587e77e100fa40eb6af10e00497c67acf493f33.tar.gz
perf stat: Do not delay the workload with --delay
The -D/--delay option is to delay the measure after the program starts. But the current code goes to sleep before starting the program so the program is delayed too. This is not the intention, let's fix it. Before: $ time sudo ./perf stat -a -e cycles -D 3000 sleep 4 Events disabled Events enabled Performance counter stats for 'system wide': 4,326,949,337 cycles 4.007494118 seconds time elapsed real 0m7.474s user 0m0.356s sys 0m0.120s It ran the workload for 4 seconds and gave the 3 second delay. So it should skip the first 3 second and measure the last 1 second only. But as you can see, it delays 3 seconds and ran the workload after that for 4 seconds. So the total time (real) was 7 seconds. After: $ time sudo ./perf stat -a -e cycles -D 3000 sleep 4 Events disabled Events enabled Performance counter stats for 'system wide': 1,063,551,013 cycles 1.002769510 seconds time elapsed real 0m4.484s user 0m0.385s sys 0m0.086s The bug was introduced when it changed enablement of system-wide events with a command line workload. But it should've considered the initial delay case. The code was reworked since then (in bb8bc52e7578) so I'm afraid it won't be applied cleanly. Fixes: d0a0a511493d2695 ("perf stat: Fix forked applications enablement of counters") Reported-by: Kevin Nomura <nomurak@google.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> Tested-by: Thomas Richter <tmricht@linux.ibm.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Ian Rogers <irogers@google.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Sumanth Korikkar <sumanthk@linux.ibm.com> Link: https://lore.kernel.org/r/20221212230820.901382-1-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
-rw-r--r--tools/perf/builtin-stat.c33
1 files changed, 17 insertions, 16 deletions
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2808f2aa9a825..bf640abc9c41f 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -539,26 +539,14 @@ static int enable_counters(void)
return err;
}
- if (stat_config.initial_delay < 0) {
- pr_info(EVLIST_DISABLED_MSG);
- return 0;
- }
-
- if (stat_config.initial_delay > 0) {
- pr_info(EVLIST_DISABLED_MSG);
- usleep(stat_config.initial_delay * USEC_PER_MSEC);
- }
-
/*
* We need to enable counters only if:
* - we don't have tracee (attaching to task or cpu)
* - we have initial delay configured
*/
- if (!target__none(&target) || stat_config.initial_delay) {
+ if (!target__none(&target)) {
if (!all_counters_use_bpf)
evlist__enable(evsel_list);
- if (stat_config.initial_delay > 0)
- pr_info(EVLIST_ENABLED_MSG);
}
return 0;
}
@@ -926,14 +914,27 @@ try_again_reset:
return err;
}
- err = enable_counters();
- if (err)
- return -1;
+ if (stat_config.initial_delay) {
+ pr_info(EVLIST_DISABLED_MSG);
+ } else {
+ err = enable_counters();
+ if (err)
+ return -1;
+ }
/* Exec the command, if any */
if (forks)
evlist__start_workload(evsel_list);
+ if (stat_config.initial_delay > 0) {
+ usleep(stat_config.initial_delay * USEC_PER_MSEC);
+ err = enable_counters();
+ if (err)
+ return -1;
+
+ pr_info(EVLIST_ENABLED_MSG);
+ }
+
t0 = rdclock();
clock_gettime(CLOCK_MONOTONIC, &ref_time);