Skip to content

Commit c587e77

Browse files
namhyungacmel
authored andcommitted
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 bb8bc52) so I'm afraid it won't be applied cleanly. Fixes: d0a0a51 ("perf stat: Fix forked applications enablement of counters") Reported-by: Kevin Nomura <[email protected]> Signed-off-by: Namhyung Kim <[email protected]> Tested-by: Thomas Richter <[email protected]> Cc: Adrian Hunter <[email protected]> Cc: Ian Rogers <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Sumanth Korikkar <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 5f8f956 commit c587e77

File tree

1 file changed

+17
-16
lines changed

1 file changed

+17
-16
lines changed

tools/perf/builtin-stat.c

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -539,26 +539,14 @@ static int enable_counters(void)
539539
return err;
540540
}
541541

542-
if (stat_config.initial_delay < 0) {
543-
pr_info(EVLIST_DISABLED_MSG);
544-
return 0;
545-
}
546-
547-
if (stat_config.initial_delay > 0) {
548-
pr_info(EVLIST_DISABLED_MSG);
549-
usleep(stat_config.initial_delay * USEC_PER_MSEC);
550-
}
551-
552542
/*
553543
* We need to enable counters only if:
554544
* - we don't have tracee (attaching to task or cpu)
555545
* - we have initial delay configured
556546
*/
557-
if (!target__none(&target) || stat_config.initial_delay) {
547+
if (!target__none(&target)) {
558548
if (!all_counters_use_bpf)
559549
evlist__enable(evsel_list);
560-
if (stat_config.initial_delay > 0)
561-
pr_info(EVLIST_ENABLED_MSG);
562550
}
563551
return 0;
564552
}
@@ -926,14 +914,27 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
926914
return err;
927915
}
928916

929-
err = enable_counters();
930-
if (err)
931-
return -1;
917+
if (stat_config.initial_delay) {
918+
pr_info(EVLIST_DISABLED_MSG);
919+
} else {
920+
err = enable_counters();
921+
if (err)
922+
return -1;
923+
}
932924

933925
/* Exec the command, if any */
934926
if (forks)
935927
evlist__start_workload(evsel_list);
936928

929+
if (stat_config.initial_delay > 0) {
930+
usleep(stat_config.initial_delay * USEC_PER_MSEC);
931+
err = enable_counters();
932+
if (err)
933+
return -1;
934+
935+
pr_info(EVLIST_ENABLED_MSG);
936+
}
937+
937938
t0 = rdclock();
938939
clock_gettime(CLOCK_MONOTONIC, &ref_time);
939940

0 commit comments

Comments
 (0)