In redhat perftool testsuite, observed fail for this test:
-- [ FAIL ] -- perf_sched :: test_timehist :: --with-summary (output regexp
parsing)
This led to analysis of "perf sched timehist" summary options.
# perf sched record -a -o ./perf.data -- sleep 0.1
This will record using perf sched record
perf sched timeliest has two options "-s" and "-S"
# perf sched -i ./perf.data timehist -S
-S : Captures summary also at the end
# perf sched -i ./perf.data timehist -s
-s : Captures only summary
The test saves -s result which has only summary and compares with
summary which comes at the end from -S . Since there is a difference
in these two, test fails.
Checking the behaviour change in -S and -s results, difference is:
rcu_sched[16] 2 4 0.013 0.001
0.003 0.006 33.23 0
migration/11[73] 2 1 0.006 0.006
0.006 0.006 0.00 0
migration/3[33] 2 1 0.006 0.006
0.006 0.006 0.00 0
- :216753[216753] -1 1 0.041 0.041
0.041 0.041 0.00 0
+ sleep[216753] -1 1 0.041 0.041
0.041 0.041 0.00 0
migration/8[58] 2 1 0.005 0.005
0.005 0.005 0.00 0
NetworkManager[811] 1 2 0.089 0.028
0.044 0.060 36.06 0
migration/13[83] 2 1 0.005 0.005
0.005 0.005 0.00 0
Here 216753 is pid for sleep which is a zombie process. This is
happening in latest kernel due to an update in "-S" result.
In -S, the process name appears in the results "sleep[216753]",
where as in the -s, only pid is present in the summary result
":216753[216753]".
After commit 39f473f6d0b2 ("perf sched timehist: decode process names
of processes in zombie state")
for -S option, if process name is using pid, it uses different way to
set it. So that we get the process name and not just Pid.
This change went in only for timehist_print_sample() function.
Add this improvement in generic place so that even -s option (which
captures summary) also will have meaningful information.
Acked-by: Namhyung Kim <[email protected]>
Tested-by: Venkat Rao Bagalkote <[email protected]>
Signed-off-by: Athira Rajeev <[email protected]>
---
Changelog
v1 -> v2:
- Rebased on top of latest perf-tools-next
- Since there is no change in logic, carried Acked-by from
Namhyung Kim and Tested-by from Venkat Rao Bagalkote
tools/perf/builtin-sched.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index b7ccdc6a985d..5d3761a27e67 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2259,12 +2259,6 @@ static void timehist_print_sample(struct perf_sched
*sched,
printf(" ");
}
- if (!thread__comm_set(thread)) {
- const char *prev_comm = perf_sample__strval(sample,
"prev_comm");
-
- thread__set_comm(thread, prev_comm, sample->time);
- }
-
printf(" %-*s ", comm_width, timehist_get_commstr(thread));
if (sched->show_prio)
@@ -2965,6 +2959,16 @@ static int timehist_sched_change_event(const struct
perf_tool *tool,
thread__zput(itr->last_thread);
}
+ /*
+ * If the process name is not set for the thread, use
"prev_comm"
+ * to set it. Otherwise the sched summary will have just pid
information
+ */
+ if (!thread__comm_set(thread)) {
+ const char *prev_comm = perf_sample__strval(sample,
"prev_comm");
+
+ thread__set_comm(thread, prev_comm, sample->time);
+ }
+
if (!sched->summary_only)
timehist_print_sample(sched, sample, &al, thread, t,
state);
}
--
2.52.0
