Re: [PATCH] tools/perf/sched: Update process names of processes in zombie state for both -s and -S options

[Namhyung Kim <namhyung@kernel.org>]

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 <atrajeev@linux.ibm.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  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
>