[PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Andi Kleen]

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>

--

v2: Adjust comment (Adrian Hunter)
---
 tools/perf/builtin-script.c                              | 6 ++++--
 tools/perf/util/dump-insn.c                              | 2 +-
 tools/perf/util/dump-insn.h                              | 2 +-
 tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c | 5 +++--
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 37088cc0ff1b..b97f810ad00e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1343,7 +1343,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;
@@ -1378,7 +1378,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 2bd8585db93c..c1cc0ade48d0 100644
--- 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 650125061530..a5de239679d7 100644
--- a/tools/perf/util/dump-insn.h
+++ b/tools/perf/util/dump-insn.h
@@ -20,6 +20,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 c5d57027ec23..292027a984a9 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -200,12 +200,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,
-- 
2.43.0

Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Adrian Hunter]

On 29/02/24 18:18, Andi Kleen wrote:
> 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>
> 
> --
> 
> v2: Adjust comment (Adrian Hunter)
> ---
>  tools/perf/builtin-script.c                              | 6 ++++--
>  tools/perf/util/dump-insn.c                              | 2 +-
>  tools/perf/util/dump-insn.h                              | 2 +-
>  tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c | 5 +++--
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 37088cc0ff1b..b97f810ad00e 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1343,7 +1343,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;
> @@ -1378,7 +1378,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) {

I still think this condition could use some more explanation,
say:
		/*
		 * Warn if it should be possible to walk to the sample but an
		 * unconditional branch is encountered. Conditional branches are
		 * assumed to be non-taken, since non-taken branches would not
		 * be in the branch stack.
		 */

> +		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 2bd8585db93c..c1cc0ade48d0 100644
> --- 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 650125061530..a5de239679d7 100644
> --- a/tools/perf/util/dump-insn.h
> +++ b/tools/perf/util/dump-insn.h
> @@ -20,6 +20,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 c5d57027ec23..292027a984a9 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
> @@ -200,12 +200,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,

[PING] Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Andi Kleen]

Looks like this bug fix was never applied. Ping!

On Thu, Feb 29, 2024 at 08:18:28AM -0800, Andi Kleen wrote:
> 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>
> 
> --
> 
> v2: Adjust comment (Adrian Hunter)
> ---
>  tools/perf/builtin-script.c                              | 6 ++++--
>  tools/perf/util/dump-insn.c                              | 2 +-
>  tools/perf/util/dump-insn.h                              | 2 +-
>  tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c | 5 +++--
>  4 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 37088cc0ff1b..b97f810ad00e 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1343,7 +1343,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;
> @@ -1378,7 +1378,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 2bd8585db93c..c1cc0ade48d0 100644
> --- 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 650125061530..a5de239679d7 100644
> --- a/tools/perf/util/dump-insn.h
> +++ b/tools/perf/util/dump-insn.h
> @@ -20,6 +20,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 c5d57027ec23..292027a984a9 100644
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
> @@ -200,12 +200,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,
> -- 
> 2.43.0
> 

Re: [PING] Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Arnaldo Carvalho de Melo]

On Wed, May 08, 2024 at 01:32:01PM -0700, Andi Kleen wrote:
> Looks like this bug fix was never applied. Ping!

There was a comment by Adrian, about adding a comment with some
explanation, may I add that to your patch while applying it?

Adrian,

With that in may I have your Acked-by or Reviewed-by?

- Arnaldo
 
> On Thu, Feb 29, 2024 at 08:18:28AM -0800, Andi Kleen wrote:
> > 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>
> > 
> > --
> > 
> > v2: Adjust comment (Adrian Hunter)
> > ---
> >  tools/perf/builtin-script.c                              | 6 ++++--
> >  tools/perf/util/dump-insn.c                              | 2 +-
> >  tools/perf/util/dump-insn.h                              | 2 +-
> >  tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c | 5 +++--
> >  4 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 37088cc0ff1b..b97f810ad00e 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -1343,7 +1343,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;
> > @@ -1378,7 +1378,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 2bd8585db93c..c1cc0ade48d0 100644
> > --- 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 650125061530..a5de239679d7 100644
> > --- a/tools/perf/util/dump-insn.h
> > +++ b/tools/perf/util/dump-insn.h
> > @@ -20,6 +20,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 c5d57027ec23..292027a984a9 100644
> > --- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
> > +++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
> > @@ -200,12 +200,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,
> > -- 
> > 2.43.0
> > 

Re: [PING] Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Andi Kleen]

On Fri, May 10, 2024 at 11:47:26AM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, May 08, 2024 at 01:32:01PM -0700, Andi Kleen wrote:
> > Looks like this bug fix was never applied. Ping!
> 
> There was a comment by Adrian, about adding a comment with some
> explanation, may I add that to your patch while applying it?

Sure feel free to add anything.

-Andi

Re: [PING] Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Adrian Hunter]

On 10/05/24 17:47, Arnaldo Carvalho de Melo wrote:
> On Wed, May 08, 2024 at 01:32:01PM -0700, Andi Kleen wrote:
>> Looks like this bug fix was never applied. Ping!
> 
> There was a comment by Adrian, about adding a comment with some
> explanation, may I add that to your patch while applying it?
> 
> Adrian,
> 
> With that in may I have your Acked-by or Reviewed-by?

Yup:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

Re: [PING] Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Andi Kleen]

On Fri, May 10, 2024 at 11:47:26AM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, May 08, 2024 at 01:32:01PM -0700, Andi Kleen wrote:
> > Looks like this bug fix was never applied. Ping!
> 
> There was a comment by Adrian, about adding a comment with some
> explanation, may I add that to your patch while applying it?

Yes.

> 
> Adrian,
> 
> With that in may I have your Acked-by or Reviewed-by?

It looks like the patch was never applied. Thanks for Kan to reminding
me.

Please apply it.


-Andi

Re: [PING] Re: [PATCH v2] perf, script: Minimize "not reaching sample" for brstackinsn

[Arnaldo Carvalho de Melo]

On Fri, Aug 30, 2024 at 08:51:43AM -0700, Andi Kleen wrote:
> On Fri, May 10, 2024 at 11:47:26AM -0300, Arnaldo Carvalho de Melo wrote:
> > On Wed, May 08, 2024 at 01:32:01PM -0700, Andi Kleen wrote:
> > > Looks like this bug fix was never applied. Ping!
> > 
> > There was a comment by Adrian, about adding a comment with some
> > explanation, may I add that to your patch while applying it?
> 
> Yes.
> 
> > 
> > Adrian,
> > 
> > With that in may I have your Acked-by or Reviewed-by?
> 
> It looks like the patch was never applied. Thanks for Kan to reminding
> me.
> 
> Please apply it.

I'll check.

- Arnaldo