This is a note to let you know that I've just added the patch titled
perf stat: Do not delay the workload with --delay
to the 5.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
perf-stat-do-not-delay-the-workload-with-delay.patch
and it can be found in the queue-5.15 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
commit 7c2203bc0e3e87975bb1eb1111ad6ecd6177db8d
Author: Namhyung Kim <namhyung@xxxxxxxxxx>
Date: Mon Dec 12 15:08:20 2022 -0800
perf stat: Do not delay the workload with --delay
[ Upstream commit c587e77e100fa40eb6af10e00497c67acf493f33 ]
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@xxxxxxxxxx>
Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
Tested-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Cc: Ian Rogers <irogers@xxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Sumanth Korikkar <sumanthk@xxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/20221212230820.901382-1-namhyung@xxxxxxxxxx
Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 2602c750779d..aad65c95c371 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -558,26 +558,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;
}
@@ -953,14 +941,27 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
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);
