| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 4 Apr 2017 23:52:53 +0800
From: "Jin, Yao" <yao.jin@...ux.intel.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Jiri Olsa <jolsa@...nel.org>, linux-kernel@...r.kernel.org,
ak@...ux.intel.com, kan.liang@...el.com, yao.jin@...el.com,
Peter Zijlstra <peterz@...radead.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH v1 1/5] perf/core: Define the common branch type
classification
On 4/4/2017 10:18 PM, Arnaldo Carvalho de Melo wrote:
> Adding the perf kernel maintainers to the CC list.
>
> Em Fri, Mar 31, 2017 at 11:18:38PM +0800, Jin Yao escreveu:
>> It is often useful to know the branch types while analyzing branch
>> data. For example, a call is very different from a conditional branch.
>>
>> Currently we have to look it up in binary while the binary may later
>> not be available and even the binary is available but user has to take
>> some time. It is very useful for user to check it directly in perf
>> report.
>>
>> Perf already has support for disassembling the branch instruction
>> to get the branch type. The branch type is defined in lbr.c.
>>
>> 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.
>>
>> Since the disassembling of branch instruction needs some overhead,
>> a new PERF_SAMPLE_BRANCH_TYPE_SAVE is introduced to indicate if it
>> needs to disassemble the branch instruction and record the branch
>> type.
>>
>> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
>> ---
>> include/uapi/linux/perf_event.h | 24 +++++++++++++++++++++++-
>> tools/include/uapi/linux/perf_event.h | 24 +++++++++++++++++++++++-
>> 2 files changed, 46 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>> index d09a9cd..4d731fd 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -174,6 +174,8 @@ enum perf_branch_sample_type_shift {
>> PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT = 14, /* no flags */
>> PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT = 15, /* no cycles */
>>
>> + PERF_SAMPLE_BRANCH_TYPE_SAVE_SHIFT = 16, /* save branch type */
>> +
>> PERF_SAMPLE_BRANCH_MAX_SHIFT /* non-ABI */
>> };
>>
>> @@ -198,9 +200,27 @@ enum perf_branch_sample_type {
>> PERF_SAMPLE_BRANCH_NO_FLAGS = 1U << PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT,
>> PERF_SAMPLE_BRANCH_NO_CYCLES = 1U << PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT,
>>
>> + PERF_SAMPLE_BRANCH_TYPE_SAVE =
>> + 1U << PERF_SAMPLE_BRANCH_TYPE_SAVE_SHIFT,
>> +
>> PERF_SAMPLE_BRANCH_MAX = 1U << PERF_SAMPLE_BRANCH_MAX_SHIFT,
>> };
>>
>> +/*
>> + * Common flow change classification
>> + */
>> +enum {
>> + PERF_BR_NONE = 0, /* unknown */
>> + PERF_BR_JCC_FWD = 1 << 1, /* conditional forward jump */
>> + PERF_BR_JCC_BWD = 1 << 2, /* conditional backward jump */
>> + PERF_BR_JMP = 1 << 3, /* jump */
>> + PERF_BR_IND_JMP = 1 << 4, /* indirect jump */
>> + PERF_BR_CALL = 1 << 5, /* call */
>> + PERF_BR_IND_CALL = 1 << 6, /* indirect call */
>> + PERF_BR_RET = 1 << 7, /* return */
>> + PERF_BR_FAR_BRANCH = 1 << 8, /* SYSCALL,SYSRET,IRQ,... */
> Humm, wouldn't be better to have those in separate buckets? I.e.
>
> PERF_BR_SYSCALL
> PERF_BR_SYSRET,
> PERF_BR_IRQ
>
> etc?
There are also other types. I.e. abort, ..... I use FAR_BRANCH is
because I just want to differentiate between basic branch types and
others. (others may be too much and platform specific).
>
> And why a bitmask? /me reads a bit more... couldn't find a reason for
> this:
>
> + type:9, /* branch type */
>
> Do you have a reason to use 9 bits? Why not just:
>
> enum {
> PERF_BR_NONE = 0, /* unknown */
> PERF_BR_JCC_FWD = 1, /* conditional forward jump */
> PERF_BR_JCC_BWD = 2, /* conditional backward jump */
> PERF_BR_JMP = 3, /* jump */
> PERF_BR_IND_JMP = 4, /* indirect jump */
> PERF_BR_CALL = 5, /* call */
> PERF_BR_IND_CALL = 6, /* indirect call */
> PERF_BR_RET = 7, /* return */
> PERF_BR_FAR_BRANCH = 8, /* SYSCALL,SYSRET,IRQ,... */
>
> And then use, say, 4 or 5 bits for that type field?
>
> I must be missing something trivial ;-\
>
> - Arnaldo
You are right. I made things more complicated. Yes, the definitions
should be clear and simple. I will redefine them in v2.
Thanks
Jin Yao
>
>> +};
>> +
>> #define PERF_SAMPLE_BRANCH_PLM_ALL \
>> (PERF_SAMPLE_BRANCH_USER|\
>> PERF_SAMPLE_BRANCH_KERNEL|\
>> @@ -999,6 +1019,7 @@ union perf_mem_data_src {
>> * in_tx: running in a hardware transaction
>> * abort: aborting a hardware transaction
>> * cycles: cycles from last branch (or 0 if not supported)
>> + * type: branch type
>> */
>> struct perf_branch_entry {
>> __u64 from;
>> @@ -1008,7 +1029,8 @@ struct perf_branch_entry {
>> in_tx:1, /* in transaction */
>> abort:1, /* transaction abort */
>> cycles:16, /* cycle count to last branch */
>> - reserved:44;
>> + type:9, /* branch type */
>> + reserved:35;
>> };
>>
>> #endif /* _UAPI_LINUX_PERF_EVENT_H */
>> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
>> index d09a9cd..4d731fd 100644
>> --- a/tools/include/uapi/linux/perf_event.h
>> +++ b/tools/include/uapi/linux/perf_event.h
>> @@ -174,6 +174,8 @@ enum perf_branch_sample_type_shift {
>> PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT = 14, /* no flags */
>> PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT = 15, /* no cycles */
>>
>> + PERF_SAMPLE_BRANCH_TYPE_SAVE_SHIFT = 16, /* save branch type */
>> +
>> PERF_SAMPLE_BRANCH_MAX_SHIFT /* non-ABI */
>> };
>>
>> @@ -198,9 +200,27 @@ enum perf_branch_sample_type {
>> PERF_SAMPLE_BRANCH_NO_FLAGS = 1U << PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT,
>> PERF_SAMPLE_BRANCH_NO_CYCLES = 1U << PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT,
>>
>> + PERF_SAMPLE_BRANCH_TYPE_SAVE =
>> + 1U << PERF_SAMPLE_BRANCH_TYPE_SAVE_SHIFT,
>> +
>> PERF_SAMPLE_BRANCH_MAX = 1U << PERF_SAMPLE_BRANCH_MAX_SHIFT,
>> };
>>
>> +/*
>> + * Common flow change classification
>> + */
>> +enum {
>> + PERF_BR_NONE = 0, /* unknown */
>> + PERF_BR_JCC_FWD = 1 << 1, /* conditional forward jump */
>> + PERF_BR_JCC_BWD = 1 << 2, /* conditional backward jump */
>> + PERF_BR_JMP = 1 << 3, /* jump */
>> + PERF_BR_IND_JMP = 1 << 4, /* indirect jump */
>> + PERF_BR_CALL = 1 << 5, /* call */
>> + PERF_BR_IND_CALL = 1 << 6, /* indirect call */
>> + PERF_BR_RET = 1 << 7, /* return */
>> + PERF_BR_FAR_BRANCH = 1 << 8, /* SYSCALL,SYSRET,IRQ,... */
>> +};
>> +
>> #define PERF_SAMPLE_BRANCH_PLM_ALL \
>> (PERF_SAMPLE_BRANCH_USER|\
>> PERF_SAMPLE_BRANCH_KERNEL|\
>> @@ -999,6 +1019,7 @@ union perf_mem_data_src {
>> * in_tx: running in a hardware transaction
>> * abort: aborting a hardware transaction
>> * cycles: cycles from last branch (or 0 if not supported)
>> + * type: branch type
>> */
>> struct perf_branch_entry {
>> __u64 from;
>> @@ -1008,7 +1029,8 @@ struct perf_branch_entry {
>> in_tx:1, /* in transaction */
>> abort:1, /* transaction abort */
>> cycles:16, /* cycle count to last branch */
>> - reserved:44;
>> + type:9, /* branch type */
>> + reserved:35;
>> };
>>
>> #endif /* _UAPI_LINUX_PERF_EVENT_H */
>> --
>> 2.7.4
Powered by blists - more mailing lists