linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
To: Stephane Eranian <eranian@google.com>
Cc: Michael Neuling <mikey@neuling.org>,
	"ak@linux.intel.com" <ak@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Michael Ellerman <michael@ellerman.id.au>,
	Linux PPC dev <linuxppc-dev@ozlabs.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
	Ingo Molnar <mingo@kernel.org>
Subject: Re: [V6 00/11] perf: New conditional branch filter
Date: Mon, 02 Jun 2014 21:34:38 +0530	[thread overview]
Message-ID: <538CA096.20709@linux.vnet.ibm.com> (raw)
In-Reply-To: <CABPqkBSmuMth4KmdLw7Nigf5uQREQJ+xL=42xV9c8FjR2VPO5g@mail.gmail.com>

On 06/02/2014 06:29 PM, Stephane Eranian wrote:
> On Wed, May 28, 2014 at 10:04 AM, Anshuman Khandual
> <khandual@linux.vnet.ibm.com> wrote:
>> On 05/27/2014 05:39 PM, Stephane Eranian wrote:
>>> I have been looking at those patches and ran some tests.
>>> And I found a few issues so far.
>>>
>>> I am running:
>>> $ perf record -j any_ret -e cycles:u test_program
>>> $ perf report -D
>>>
>>> Most entries are okay and match the filter, however some do not make sense:
>>>
>>> 3642586996762 0x15d0 [0x108]: PERF_RECORD_SAMPLE(IP, 2): 17921/17921:
>>> 0x10001170 period: 613678 addr: 0
>>> .... branch stack: nr:9
>>> .....  0: 00000000100011cc -> 0000000010000e38
>>> .....  1: 0000000010001150 -> 00000000100011bc
>>> .....  2: 0000000010001208 -> 0000000010000e38
>>> .....  3: 0000000010001160 -> 00000000100011f8
>>> .....  4: 00000000100011cc -> 0000000010000e38
>>> .....  5: 0000000010001150 -> 00000000100011bc
>>> .....  6: 0000000010001208 -> 0000000010000e38
>>> .....  7: 0000000010001160 -> 00000000100011f8
>>> .....  8: 0000000000000000 -> 0000000010001160
>>> ^^^^^^
>>> Entry 8 does not make sense, unless 0x0 is a valid return branch
>>> instruction address.
>>> If an address is invalid, the whole entry needs to be eliminated. It
>>> is okay to have
>>> less than the max number of entries supported by HW.
>>
>> Hey Stephane,
>>
>> Okay. The same behaviour is also reflected in the test results what I have
>> shared in the patchset. Here is that section.
>>
>> (3) perf record -j any_ret -e branch-misses:u ./cprog
>>
>> # Overhead  Command  Source Shared Object          Source Symbol  Target Shared Object          Target Symbol
>> # ........  .......  ....................  .....................  ....................  .....................
>> #
>>     15.61%    cprog  [unknown]             [.] 00000000           cprog                 [.] sw_3_1
>>      6.28%    cprog  cprog                 [.] symbol2            cprog                 [.] hw_1_2
>>      6.28%    cprog  cprog                 [.] ctr_addr           cprog                 [.] sw_4_1
>>      6.26%    cprog  cprog                 [.] success_3_1_3      cprog                 [.] sw_3_1
>>      6.24%    cprog  cprog                 [.] symbol1            cprog                 [.] hw_1_1
>>      6.24%    cprog  cprog                 [.] sw_4_2             cprog                 [.] callme
>>      6.21%    cprog  [unknown]             [.] 00000000           cprog                 [.] callme
>>      6.19%    cprog  cprog                 [.] lr_addr            cprog                 [.] sw_4_2
>>      3.16%    cprog  cprog                 [.] hw_1_2             cprog                 [.] callme
>>      3.15%    cprog  cprog                 [.] success_3_1_1      cprog                 [.] sw_3_1
>>      3.15%    cprog  cprog                 [.] sw_4_1             cprog                 [.] callme
>>      3.14%    cprog  cprog                 [.] callme             cprog                 [.] main
>>      3.13%    cprog  cprog                 [.] hw_1_1             cprog                 [.] callme
>>
>> So a lot of samples above have 0x0 as the "from" address. This originates from the code
>> section here inside the function "power_pmu_bhrb_read", where we hit two back to back
> 
> Could you explain the back-to-back case a bit more here?
> Back-to-back returns to me means something like:
> 
> int foo()
> {
>   ...
>    return bar();
> }
> 
> int bar()
> {
>   return 0;
> }
> 
> Not counting the leaf optimization here, bar return to foo which
> immediately returns: 2 back-2-back returns.
> Is that the case you're talking about here?
> 

No. Filtering of return branches has been implemented in SW only. So PMU as such does not capture
return only branches. It captures all the branches what it encounters. During the capture process
PMU might *record* two back to back "target addresses" (without capturing the from address for the
first one) for which we are unable to figure out the "from address". This leaves us with one branch
record where we have the target address not from address and so we make it zero. With the current
logic all branch records with "from address" as zero get filtered through and become the part of the
final set. I was not too sure how to deal with these cases.

>> target addresses. So we zero out the from address for the first target address and re-read
>> the second address over again. So thats how we get zero as the from address. This is how the
>> HW capture the samples. I was reluctant to drop these samples but I agree that these kind of
>> samples can be dropped if we need to.
>>
> I think we need to make it as simple as possible for tools, i.e.,
> avoid having to decode the
> disassembly to figure out what happened. Here address 0 is not exploitable.

Thats right. Dropping the branch record where we have only the target address not the from address
might just solve this problem.

  reply	other threads:[~2014-06-02 16:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-05  9:09 [V6 00/11] perf: New conditional branch filter Anshuman Khandual
2014-05-05  9:09 ` [V6 01/11] perf: Add PERF_SAMPLE_BRANCH_COND Anshuman Khandual
2014-05-05  9:09 ` [V6 02/11] perf, tool: Conditional branch filter 'cond' added to perf record Anshuman Khandual
2014-05-05  9:09 ` [V6 03/11] x86, perf: Add conditional branch filtering support Anshuman Khandual
2014-05-05  9:09 ` [V6 04/11] perf, documentation: Description for conditional branch filter Anshuman Khandual
2014-05-05  9:09 ` [V6 05/11] powerpc, perf: Re-arrange BHRB processing Anshuman Khandual
2014-05-05  9:09 ` [V6 06/11] powerpc, perf: Re-arrange PMU based branch filter processing in POWER8 Anshuman Khandual
2014-05-05  9:09 ` [V6 07/11] powerpc, perf: Change the name of HW PMU branch filter tracking variable Anshuman Khandual
2014-05-05  9:09 ` [V6 08/11] powerpc, lib: Add new branch analysis support functions Anshuman Khandual
2014-05-05  9:09 ` [V6 09/11] powerpc, perf: Enable SW filtering in branch stack sampling framework Anshuman Khandual
2014-05-05  9:09 ` [V6 10/11] power8, perf: Adapt BHRB PMU configuration to work with SW filters Anshuman Khandual
2014-05-05  9:09 ` [V6 11/11] powerpc, perf: Enable privilege mode SW branch filters Anshuman Khandual
2014-05-27 12:09 ` [V6 00/11] perf: New conditional branch filter Stephane Eranian
2014-05-28  8:04   ` Anshuman Khandual
2014-06-02 12:59     ` Stephane Eranian
2014-06-02 16:04       ` Anshuman Khandual [this message]
2014-06-02 16:25         ` Stephane Eranian
2014-06-02 22:52       ` Michael Neuling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=538CA096.20709@linux.vnet.ibm.com \
    --to=khandual@linux.vnet.ibm.com \
    --cc=acme@ghostprotocols.net \
    --cc=ak@linux.intel.com \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=michael@ellerman.id.au \
    --cc=mikey@neuling.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=sukadev@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).