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

[Veronika Molnarova <vmolnaro@redhat.com>]

On 9/27/24 17:19, 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>

The -S and -s option works after this patch, but --errno-summary ends with
segfault for some reason.

Before:
	# perf trace --errno-summary -- true


 	Summary of events:

After:
	# perf trace --errno-summary -- true

	 Summary of events:
	
	 true (11386), 60 events, 90.9%
	
   	   syscall            calls  errors  total       min       avg       max       stddev
        	                             (msec)    (msec)    (msec)    (msec)        (%)
 	   --------------- --------  ------ -------- --------- --------- ---------     ------
   	   mmap                   8      0     0.044     0.003     0.006     0.010     15.17%
   	   mprotect               3      0     0.014     0.004     0.005     0.005      4.56%
   	   openat                 2      0     0.009     0.004     0.004     0.005      5.14%
   	   munmap                 1      0     0.007     0.007     0.007     0.007      0.00%
  	   access                 1      1     0.004     0.004     0.004     0.004      0.00%
	perf: Segmentation fault
	Obtained 12 stack frames.
	./perf() [0x5ae123]
	./perf() [0x5ae1cb]
	/lib64/libc.so.6(+0x3dbb0) [0x7f633845fbb0]
	./perf() [0x56e2fb]
	./perf() [0x4850b3]
	./perf() [0x48ee89]
	./perf() [0x493042]
	./perf() [0x49334b]
	./perf() [0x40ee8c]
	/lib64/libc.so.6(+0x27b8a) [0x7f6338449b8a]
	/lib64/libc.so.6(__libc_start_main+0x8b) [0x7f6338449c4b]
	./perf() [0x40f4b5]
	Segmentation fault

Will try to check what is causing this issue.

Thanks,
Veronika

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