perf script: Minimize "not reaching sample" for '-F +brstackinsn'

In some situations 'perf script -F +brstackinsn' sees a lot of "not
reaching sample" messages.

This happens when the last LBR block before the sample contains a branch
that is not in the LBR, and the instruction dumping stops.

  $ perf record -b  emacs -Q --batch '()'
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.396 MB perf.data (443 samples) ]
  $ perf script -F +brstackinsn
  ...
          00007f0ab2d171a4        insn: 41 0f 94 c0
          00007f0ab2d171a8        insn: 83 fa 01
          00007f0ab2d171ab        insn: 74 d3                     # PRED 6 cycles [313] 1.00 IPC
          00007f0ab2d17180        insn: 45 84 c0
          00007f0ab2d17183        insn: 74 28
          ... not reaching sample ...

  $ perf script -F +brstackinsn | grep -c reach
  136
  $

This is a problem for further analysis that wants to see the full code
upto the sample.

There are two common cases where the message is bogus:

- The LBR only logs taken branches, but the branch might be a
  conditional branch that is not taken (that is the most common case
  actually)

- The LBR sampling uses a filter ignoring some branches, but the perf
  script check checks for all branches.

This patch fixes these two conditions, by only checking for conditional
branches, as well as checking the perf_event_attr's branch filter
attributes.

For the test case above it fixes all the messages:

  $ ./perf script -F +brstackinsn | grep -c reach
  0

Note that there are still conditions when the message is hit --
sometimes there can be a unconditional branch that misses the LBR update
before the sample -- but they are much more rare now.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Link: https://lore.kernel.org/r/20240229161828.386397-1-ak@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 206b084265555ebc5e22cb1424ff54fa96a27fd3..dbe792b52c5cbc82c10de5f1a66c0754997e747e 100644 (file)
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1428,7 +1428,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
         * Due to pipeline delays the LBRs might be missing a branch
         * or two, which can result in very large or negative blocks
         * between final branch and sample. When this happens just
-        * continue walking after the last TO until we hit a branch.
+        * continue walking after the last TO.
         */
        start = entries[0].to;
        end = sample->ip;
@@ -1463,7 +1463,9 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
                printed += fprintf(fp, "\n");
                if (ilen == 0)
                        break;
-               if (arch_is_branch(buffer + off, len - off, x.is64bit) && start + off != sample->ip) {
+               if ((attr->branch_sample_type == 0 || attr->branch_sample_type & PERF_SAMPLE_BRANCH_ANY)
+                               && arch_is_uncond_branch(buffer + off, len - off, x.is64bit)
+                               && start + off != sample->ip) {
                        /*
                         * Hit a missing branch. Just stop.
                         */

diff --git a/tools/perf/util/dump-insn.c b/tools/perf/util/dump-insn.c
index 2bd8585db93c30df83d2d0740a596a56a2a1f8f7..c1cc0ade48d05e5ee51ebe9c050705717937cba5 100644 (file)
--- a/tools/perf/util/dump-insn.c
+++ b/tools/perf/util/dump-insn.c
@@ -15,7 +15,7 @@ const char *dump_insn(struct perf_insn *x __maybe_unused,
 }
 
 __weak
-int arch_is_branch(const unsigned char *buf __maybe_unused,
+int arch_is_uncond_branch(const unsigned char *buf __maybe_unused,
                   size_t len __maybe_unused,
                   int x86_64 __maybe_unused)
 {

diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h
index 4a7797dd6d0924ebd125d16dcc33a629177fe459..20d4d7bb5275856ea766d91a06ea2c511df25b7c 100644 (file)
--- a/tools/perf/util/dump-insn.h
+++ b/tools/perf/util/dump-insn.h
@@ -21,6 +21,6 @@ struct perf_insn {
 
 const char *dump_insn(struct perf_insn *x, u64 ip,
                      u8 *inbuf, int inlen, int *lenp);
-int arch_is_branch(const unsigned char *buf, size_t len, int x86_64);
+int arch_is_uncond_branch(const unsigned char *buf, size_t len, int x86_64);
 
 #endif

diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
index 4407130d91f8ab979301a3170a371ea3cd89534e..47cf35799a4dfc38625e78df9ace2a30049ae2c9 100644 (file)
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -209,12 +209,13 @@ int intel_pt_get_insn(const unsigned char *buf, size_t len, int x86_64,
        return 0;
 }
 
-int arch_is_branch(const unsigned char *buf, size_t len, int x86_64)
+int arch_is_uncond_branch(const unsigned char *buf, size_t len, int x86_64)
 {
        struct intel_pt_insn in;
        if (intel_pt_get_insn(buf, len, x86_64, &in) < 0)
                return -1;
-       return in.branch != INTEL_PT_BR_NO_BRANCH;
+       return in.branch == INTEL_PT_BR_UNCONDITIONAL ||
+              in.branch == INTEL_PT_BR_INDIRECT;
 }
 
 const char *dump_insn(struct perf_insn *x, uint64_t ip __maybe_unused,