From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753861AbdGJLqe (ORCPT ); Mon, 10 Jul 2017 07:46:34 -0400 Received: from mga04.intel.com ([192.55.52.120]:38946 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753556AbdGJLqW (ORCPT ); Mon, 10 Jul 2017 07:46:22 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,340,1496127600"; d="scan'208";a="877187467" Subject: Re: [PATCH v6 1/7] perf/core: Define the common branch type classification To: Michael Ellerman , acme@kernel.org, jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com Cc: ak@linux.intel.com, kan.liang@intel.com, linuxppc-dev@lists.ozlabs.org, Linux-kernel@vger.kernel.org, yao.jin@intel.com References: <1492690075-17243-1-git-send-email-yao.jin@linux.intel.com> <1492690075-17243-2-git-send-email-yao.jin@linux.intel.com> <87r2xoj08g.fsf@concordia.ellerman.id.au> <820424b8-d7b3-56cc-2b97-ec570d44ec25@linux.intel.com> <87h8ykvayi.fsf@concordia.ellerman.id.au> From: "Jin, Yao" Message-ID: Date: Mon, 10 Jul 2017 19:46:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <87h8ykvayi.fsf@concordia.ellerman.id.au> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Michael, Please let me summarize for the new branch type definitions. 1. We all agree these definitions: + PERF_BR_COND = 1, /* conditional */ + PERF_BR_UNCOND = 2, /* unconditional */ + PERF_BR_IND = 3, /* indirect */ + PERF_BR_CALL = 4, /* call */ + PERF_BR_IND_CALL = 5, /* indirect call */ + PERF_BR_RET = 6, /* return */ + PERF_BR_SYSCALL = 7, /* syscall */ + PERF_BR_SYSRET = 8, /* syscall return */ + PERF_BR_IRET = 11, /* return from interrupt */ 2. I wish to keep following definitions for x86. + PERF_BR_IRQ = 9, /* hw interrupt/trap/fault */ + PERF_BR_INT = 10, /* sw interrupt */ PERF_BR_INT is triggered by instruction "int" . PERF_BR_IRQ is triggered by interrupts, traps, faults (the ring 0,3 transition). 3. I can drop PERF_BR_FAR_BRANCH 4. I'd like to add following types for powerpc. PERF_BR_COND_CALL /* Conditional call */ PERF_BR_COND_RET /* Condition return */ If you agree these new definitions, I will prepare the new patch. Thanks Jin Yao On 7/10/2017 6:32 PM, Michael Ellerman wrote: > "Jin, Yao" writes: >> On 7/10/2017 2:05 PM, Michael Ellerman wrote: >>> Jin Yao writes: >>> >>>> It is often useful to know the branch types while analyzing branch >>>> data. For example, a call is very different from a conditional branch. >>>> > ... >>>> To keep consistent on kernel and userspace and make the classification >>>> more common, the patch adds the common branch type classification >>>> in perf_event.h. >>> Most of the code and doc uses "branch" but then a few these are called >>> "jump". Can we just stick with "branch"? >>> >>>> PERF_BR_NONE : unknown >>>> PERF_BR_JCC : conditional jump >>>> PERF_BR_JMP : jump >>>> PERF_BR_IND_JMP : indirect jump >>> eg: >>> >>> PERF_BR_COND : conditional branch >>> PERF_BR_UNCOND : unconditional branch >>> PERF_BR_IND : indirect branch >> Call and jump are all branches. If we want to figure out which one is >> jump and which one is call, we need the detail branch type definitions. > Yeah I'm not saying we don't need the different types, I'm saying I'd > rather we just called them "branch" not "jump". Just because "jump" can > mean different things on different arches. > >> For example, if we only say "PERF_BR_IND", we could not know if it's an >> indirect jump or indirect call. > Yes we can, PERF_BR_IND is an indirect branch, which is not a call, > because if it was a call then it would be PERF_BR_IND_CALL. > >>>> PERF_BR_CALL : call >>>> PERF_BR_IND_CALL : indirect call >>>> PERF_BR_RET : return >>>> PERF_BR_SYSCALL : syscall >>>> PERF_BR_SYSRET : syscall return >>>> PERF_BR_IRQ : hw interrupt/trap/fault >>>> PERF_BR_INT : sw interrupt >>> I'm not sure what that means, I'm guessing on x86 it means someone >>> executed "int" ? >> PERF_BR_IRQ is for hw interrupt and PERF_BR_INT is for sw interrupt. > OK, but I still don't know what that means :) > > What's an example of an instruction that is PERF_BR_IRQ and PERF_BR_INT ? > >> PERF_BR_CALL/PERF_BR_IND_CALL and PERF_BR_RET are for function call >> (direct call and indirect call) and return. > Yep makes sense. > >> PERF_BR_SYSCALL/PERF_BR_SYSRET are for syscall and syscall return. > Yep OK. > >>> Is that sufficiently useful to use up a bit? I think we only have 3 >>> free? >> Do you means 3 bits? Each bit stands for one branch type? I guess what >> you mean is: >> >> PERF_BR_COND : conditional branch >> PERF_BR_UNCOND : unconditional branch >> PERF_BR_IND : indirect branch >> >> But 3 branch types are not enough for us. > What I meant was you're using 4 bits for the type, so you have 16 > possible values, and you've defined 13 of them. Meaning there are only 3 > types free. > > So we should try to only define branch types that are really useful, and > keep some free for future use. > > Maybe PERF_BR_INT is really common on x86 and so it's important to count > it, but like I said above I don't know what it is. > >>>> PERF_BR_IRET : return from interrupt >>>> PERF_BR_FAR_BRANCH: not generic far branch type >>> What is a "not generic far branch" ? >>> >>> I don't know what that would mean on powerpc for example. >> It's reserved for future using I think. > OK so let's not put it in the Linux API until it's defined? > >>> I think the only thing we have on powerpc that's commonly used and that >>> isn't covered above is branches that decrement a loop counter and then >>> branch based on the result. > ... >> Sorry, I'm not familiar with powerpc arch. Or could you add the branch >> type which powerpc needs? > These are good: > > + PERF_BR_COND = 1, /* conditional */ > + PERF_BR_UNCOND = 2, /* unconditional */ > + PERF_BR_IND = 3, /* indirect */ > + PERF_BR_CALL = 4, /* call */ > + PERF_BR_IND_CALL = 5, /* indirect call */ > + PERF_BR_RET = 6, /* return */ > > These we wouldn't use currently, but make sense: > > + PERF_BR_SYSCALL = 7, /* syscall */ > + PERF_BR_SYSRET = 8, /* syscall return */ > + PERF_BR_IRET = 11, /* return from interrupt */ > > These I'm not so sure about, I don't really know what they would map to > for us: > > + PERF_BR_IRQ = 9, /* hw interrupt/trap/fault */ > + PERF_BR_INT = 10, /* sw interrupt */ > > And sounds like this should be dropped for now: > > + PERF_BR_FAR_BRANCH = 12, /* not generic far branch type */ > > The branch types you haven't covered which might be useful for us are: > > PERF_BR_COND_CALL /* Conditional call */ > PERF_BR_COND_RET /* Condition return */ > > > cheers