> On 27 Apr 2026, at 11:26 AM, Namhyung Kim <[email protected]> wrote:
>
> On Sun, Apr 26, 2026 at 03:09:30PM +0530, Athira Rajeev wrote:
>> 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.
>>
>> Signed-off-by: Athira Rajeev <[email protected]>
>
> Acked-by: Namhyung Kim <[email protected]>
>
> Thanks,
> Namhyung
Hi,
Can we please have this pulled in, if the patch looks fine ?
Thanks
Athira
>
>> ---
>> tools/perf/builtin-sched.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
>> index 555247568e7a..ee1b89a6af50 100644
>> --- a/tools/perf/builtin-sched.c
>> +++ b/tools/perf/builtin-sched.c
>> @@ -2197,11 +2197,6 @@ static void timehist_print_sample(struct perf_sched
>> *sched,
>> printf(" ");
>> }
>>
>> - if (!thread__comm_set(thread)) {
>> - const char *prev_comm = evsel__strval(evsel, sample,
>> "prev_comm");
>> - thread__set_comm(thread, prev_comm, sample->time);
>> - }
>> -
>> printf(" %-*s ", comm_width, timehist_get_commstr(thread));
>>
>> if (sched->show_prio)
>> @@ -2890,6 +2885,15 @@ static int timehist_sched_change_event(const struct
>> perf_tool *tool,
>> itr->last_thread = NULL;
>> }
>>
>> + /*
>> + * 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 = evsel__strval(evsel, sample, "prev_comm");
>> + thread__set_comm(thread, prev_comm, sample->time);
>> + }
>> +
>> if (!sched->summary_only)
>> timehist_print_sample(sched, evsel, sample, &al, thread, t, state);
>> }
>> --
>> 2.47.3
