Re: [PATCH 1/2] perf script: Fix brstackinsn for AUXTRACE

[Arnaldo Carvalho de Melo]

Em Wed, Nov 27, 2019 at 11:53:21AM +0200, Adrian Hunter escreveu:
> brstackinsn must be allowed to be set by the user when AUX area data has
> been captured because, in that case, the branch stack might be
> synthesized on the fly. This fixes the following error:
> 
> Before:
> 
>   $ perf record -e '{intel_pt//,cpu/mem_inst_retired.all_loads,aux-sample-size=8192/pp}:u' grep -rqs jhgjhg /boot
>   [ perf record: Woken up 19 times to write data ]
>   [ perf record: Captured and wrote 2.274 MB perf.data ]
>   $ perf script -F +brstackinsn --xed --itrace=i1usl100 | head
>   Display of branch stack assembler requested, but non all-branch filter set
>   Hint: run 'perf record -b ...'
> 
> After:
> 
>   $ perf record -e '{intel_pt//,cpu/mem_inst_retired.all_loads,aux-sample-size=8192/pp}:u' grep -rqs jhgjhg /boot
>   [ perf record: Woken up 19 times to write data ]
>   [ perf record: Captured and wrote 2.274 MB perf.data ]
>   $ perf script -F +brstackinsn --xed --itrace=i1usl100 | head
>             grep 13759 [002]  8091.310257:       1862                                        instructions:uH:      5641d58069eb bmexec+0x86b (/bin/grep)
>         bmexec+2485:
>         00005641d5806b35                        jnz 0x5641d5806bd0              # MISPRED
>         00005641d5806bd0                        movzxb  (%r13,%rdx,1), %eax
>         00005641d5806bd6                        add %rdi, %rax
>         00005641d5806bd9                        movzxb  -0x1(%rax), %edx
>         00005641d5806bdd                        cmp %rax, %r14
>         00005641d5806be0                        jnb 0x5641d58069c0              # MISPRED
>         mismatch of LBR data and executable
>         00005641d58069c0                        movzxb  (%r13,%rdx,1), %edi

Thanks, applied.

- Arnaldo
 
> Reported-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Fixes: 48d02a1d5c13 ("perf script: Add 'brstackinsn' for branch stacks")
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  tools/perf/builtin-script.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 7b2f0922050c..e8db26b9b29e 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -448,7 +448,7 @@ static int perf_evsel__check_attr(struct evsel *evsel,
>  		       "selected. Hence, no address to lookup the source line number.\n");
>  		return -EINVAL;
>  	}
> -	if (PRINT_FIELD(BRSTACKINSN) &&
> +	if (PRINT_FIELD(BRSTACKINSN) && !allow_user_set &&
>  	    !(perf_evlist__combined_branch_type(session->evlist) &
>  	      PERF_SAMPLE_BRANCH_ANY)) {
>  		pr_err("Display of branch stack assembler requested, but non all-branch filter set\n"
> -- 
> 2.17.1

-- 

- Arnaldo