Re: [PATCH] perf trace: Keep exited threads for summary

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Sep 27, 2024 at 05:19:26PM +0200, Michael Petlan wrote:
> Since 9ffa6c7512ca ("perf machine thread: Remove exited threads by
> default") perf cleans exited threads up, but as said, sometimes they
> are necessary to be kept. The mentioned commit does not cover all the
> cases, we also need the information to construct the summary table in
> perf-trace.
> 
> Before:
>     # perf trace -s true
> 
>      Summary of events:
> 
> After:
>     # perf trace -s -- true
> 
>      Summary of events:
> 
>      true (383382), 64 events, 91.4%
> 
>        syscall            calls  errors  total       min       avg       max       stddev
>                                          (msec)    (msec)    (msec)    (msec)        (%)
>        --------------- --------  ------ -------- --------- --------- ---------     ------
>        mmap                   8      0     0.150     0.013     0.019     0.031     11.90%
>        mprotect               3      0     0.045     0.014     0.015     0.017      6.47%
>        openat                 2      0     0.014     0.006     0.007     0.007      9.73%
>        munmap                 1      0     0.009     0.009     0.009     0.009      0.00%
>        access                 1      1     0.009     0.009     0.009     0.009      0.00%
>        pread64                4      0     0.006     0.001     0.001     0.002      4.53%
>        fstat                  2      0     0.005     0.001     0.002     0.003     37.59%
>        arch_prctl             2      1     0.003     0.001     0.002     0.002     25.91%
>        read                   1      0     0.003     0.003     0.003     0.003      0.00%
>        close                  2      0     0.003     0.001     0.001     0.001      3.86%
>        brk                    1      0     0.002     0.002     0.002     0.002      0.00%
>        rseq                   1      0     0.001     0.001     0.001     0.001      0.00%
>        prlimit64              1      0     0.001     0.001     0.001     0.001      0.00%
>        set_robust_list        1      0     0.001     0.001     0.001     0.001      0.00%
>        set_tid_address        1      0     0.001     0.001     0.001     0.001      0.00%
>        execve                 1      0     0.000     0.000     0.000     0.000      0.00%
> 
> Fixes: 9ffa6c7512ca ("perf machine thread: Remove exited threads by default")
> 
> Reported-by: Veronika Molnarova <vmolnaro@redhat.com>
> Signed-off-by: Michael Petlan <mpetlan@redhat.com>

Looks good

But I'd really like to have system-wide summary mode so that I can see
the syscall overhead altogether.  Of course, this can be a future work.

Thanks,
Namhyung

> ---
>  tools/perf/builtin-trace.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index f6e847529073..1a12ed71c809 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -5449,6 +5449,10 @@ int cmd_trace(int argc, const char **argv)
>  	if (trace.summary_only)
>  		trace.summary = trace.summary_only;
>  
> +	/* Keep exited threads, otherwise information might be lost for summary */
> +	if (trace.summary || trace.summary_only)
> +		symbol_conf.keep_exited_threads = true;
> +
>  	if (output_name != NULL) {
>  		err = trace__open_output(&trace, output_name);
>  		if (err < 0) {
> -- 
> 2.43.5
>