perf report: Show branch type

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 x86 branch type.

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.

PERF_BR_NONE : unknown
PERF_BR_JCC : conditional jump
PERF_BR_JMP : jump
PERF_BR_IND_JMP : indirect jump
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
PERF_BR_IRET : return from interrupt
PERF_BR_FAR_BRANCH: not generic far branch type

The patch also adds a new field type (4 bits) in perf_branch_entry to record the branch type.

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.

Change log
----------

v6: Not changed.

v5: Not changed. The v5 patch series just change the userspace.

v4: Comparing to previous version, the major changes are:

1. Remove the PERF_BR_JCC_FWD/PERF_BR_JCC_BWD, they will be computed later in userspace.

2. Remove the "cross" field in perf_branch_entry. The cross page computing will be done later in userspace.

Signed-off-by Jin Yao <[email protected]>

---
 include/uapi/linux/perf_event.h       | 29 ++++++++++++++++++++++++++++-
 tools/include/uapi/linux/perf_event.h | 29 ++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 2 deletions(-)

include/uapi/linux/perf_event.h +28 -1

index d09a9cd..69af012 100644
@@ -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,32 @@ 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		= 1,	/* conditional jump */
+	PERF_BR_JMP		= 2,	/* jump */
+	PERF_BR_IND_JMP		= 3,	/* indirect jump */
+	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_IRQ		= 9,	/* hw interrupt/trap/fault */
+	PERF_BR_INT		= 10,	/* sw interrupt */
+	PERF_BR_IRET		= 11,	/* return from interrupt */
+	PERF_BR_FAR_BRANCH	= 12,	/* not generic far branch type */
+	PERF_BR_MAX,
+};
+
 #define PERF_SAMPLE_BRANCH_PLM_ALL \
 	(PERF_SAMPLE_BRANCH_USER|\
 	 PERF_SAMPLE_BRANCH_KERNEL|\

@@ -999,6 +1024,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 +1034,8 @@ struct perf_branch_entry {
 		in_tx:1,    /* in transaction */
 		abort:1,    /* transaction abort */
 		cycles:16,  /* cycle count to last branch */
-		reserved:44;
+		type:4,     /* branch type */
+		reserved:40;
 };
 
 #endif /* _UAPI_LINUX_PERF_EVENT_H */

tools/include/uapi/linux/perf_event.h +28 -1

index d09a9cd..69af012 100644
@@ -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,32 @@ 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		= 1,	/* conditional jump */
+	PERF_BR_JMP		= 2,	/* jump */
+	PERF_BR_IND_JMP		= 3,	/* indirect jump */
+	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_IRQ		= 9,	/* hw interrupt/trap/fault */
+	PERF_BR_INT		= 10,	/* sw interrupt */
+	PERF_BR_IRET		= 11,	/* return from interrupt */
+	PERF_BR_FAR_BRANCH	= 12,	/* not generic far branch type */
+	PERF_BR_MAX,
+};
+
 #define PERF_SAMPLE_BRANCH_PLM_ALL \
 	(PERF_SAMPLE_BRANCH_USER|\
 	 PERF_SAMPLE_BRANCH_KERNEL|\

@@ -999,6 +1024,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 +1034,8 @@ struct perf_branch_entry {
 		in_tx:1,    /* in transaction */
 		abort:1,    /* transaction abort */
 		cycles:16,  /* cycle count to last branch */
-		reserved:44;
+		type:4,     /* branch type */
+		reserved:40;
 };
 
 #endif /* _UAPI_LINUX_PERF_EVENT_H */

Signature · 1 line

2.7.4

PPC folks, maddy, does this work for you guys?

Quoted 189 lines

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 x86 branch type.

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.

PERF_BR_NONE : unknown
PERF_BR_JCC : conditional jump
PERF_BR_JMP : jump
PERF_BR_IND_JMP : indirect jump
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
PERF_BR_IRET : return from interrupt
PERF_BR_FAR_BRANCH: not generic far branch type

The patch also adds a new field type (4 bits) in perf_branch_entry to record the branch type.

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.

Change log
----------

v6: Not changed.

v5: Not changed. The v5 patch series just change the userspace.

v4: Comparing to previous version, the major changes are:

1. Remove the PERF_BR_JCC_FWD/PERF_BR_JCC_BWD, they will be computed later in userspace.

2. Remove the "cross" field in perf_branch_entry. The cross page computing will be done later in userspace.

Signed-off-by Jin Yao <[email protected]>

---
 include/uapi/linux/perf_event.h       | 29 ++++++++++++++++++++++++++++-
 tools/include/uapi/linux/perf_event.h | 29 ++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 2 deletions(-)

include/uapi/linux/perf_event.h +28 -1

index d09a9cd..69af012 100644
@@ -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,32 @@ 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		= 1,	/* conditional jump */
+	PERF_BR_JMP		= 2,	/* jump */
+	PERF_BR_IND_JMP		= 3,	/* indirect jump */
+	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_IRQ		= 9,	/* hw interrupt/trap/fault */
+	PERF_BR_INT		= 10,	/* sw interrupt */
+	PERF_BR_IRET		= 11,	/* return from interrupt */
+	PERF_BR_FAR_BRANCH	= 12,	/* not generic far branch type */
+	PERF_BR_MAX,
+};
+
 #define PERF_SAMPLE_BRANCH_PLM_ALL \
 	(PERF_SAMPLE_BRANCH_USER|\
 	 PERF_SAMPLE_BRANCH_KERNEL|\

@@ -999,6 +1024,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 +1034,8 @@ struct perf_branch_entry {
 		in_tx:1,    /* in transaction */
 		abort:1,    /* transaction abort */
 		cycles:16,  /* cycle count to last branch */
-		reserved:44;
+		type:4,     /* branch type */
+		reserved:40;
 };
 
 #endif /* _UAPI_LINUX_PERF_EVENT_H */

tools/include/uapi/linux/perf_event.h +28 -1

index d09a9cd..69af012 100644
@@ -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,32 @@ 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		= 1,	/* conditional jump */
+	PERF_BR_JMP		= 2,	/* jump */
+	PERF_BR_IND_JMP		= 3,	/* indirect jump */
+	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_IRQ		= 9,	/* hw interrupt/trap/fault */
+	PERF_BR_INT		= 10,	/* sw interrupt */
+	PERF_BR_IRET		= 11,	/* return from interrupt */
+	PERF_BR_FAR_BRANCH	= 12,	/* not generic far branch type */
+	PERF_BR_MAX,
+};
+
 #define PERF_SAMPLE_BRANCH_PLM_ALL \
 	(PERF_SAMPLE_BRANCH_USER|\
 	 PERF_SAMPLE_BRANCH_KERNEL|\

@@ -999,6 +1024,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 +1034,8 @@ struct perf_branch_entry {
 		in_tx:1,    /* in transaction */
 		abort:1,    /* transaction abort */
 		cycles:16,  /* cycle count to last branch */
-		reserved:44;
+		type:4,     /* branch type */
+		reserved:40;
 };
 
 #endif /* _UAPI_LINUX_PERF_EVENT_H */

Signature · 1 line

2.7.4

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 x86 branch type.

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.

PERF_BR_NONE : unknown
PERF_BR_JCC : conditional jump
PERF_BR_JMP : jump
PERF_BR_IND_JMP : indirect jump

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

PERF_BR_IRET : return from interrupt
PERF_BR_FAR_BRANCH: not generic far branch type

Hi Jin Yao,

Sorry I haven't commented until now, but it got lost in the flood of patches.

Just a few nit-picks below ...

Quoted 8 lines · Jin Yao

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.

Quoted 6 lines

Perf already has support for disassembling the branch instruction to get the x86 branch type.

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"?

Quoted 4 lines

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

Quoted 7 lines

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" ?

Is that sufficiently useful to use up a bit? I think we only have 3 free?

Quoted 2 lines

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.

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.

It might be nice if we could separate those out from other conditional branches. Whether it's worth using a bit for I'm not sure. Do other arches have something similar?

Those branches do tend to be "backward conditional", so that may be sufficient. But backward conditional also includes if bodies that have been moved out of line and then branch back to the main body of the function.

cheers

Quoted 5 lines · Michael Ellerman

Quoted 3 lines · Jin Yao

It is often useful to know the branch types while analyzing branch data. For example, a call is very different from a conditional branch.

Quoted 11 lines

Quoted 3 lines

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"?

Quoted 4 lines

PERF_BR_NONE : unknown
PERF_BR_JCC : conditional jump
PERF_BR_JMP : jump
PERF_BR_IND_JMP : indirect jump

Quoted 5 lines

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.

For example, if we only say "PERF_BR_IND", we could not know if it's an indirect jump or indirect call.

Quoted 9 lines

Quoted 7 lines

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.

PERF_BR_CALL/PERF_BR_IND_CALL and PERF_BR_RET are for function call (direct call and indirect call) and return.

PERF_BR_SYSCALL/PERF_BR_SYSRET are for syscall and syscall return.

Quoted 2 lines

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.

Quoted 5 lines

Quoted 2 lines

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.

Quoted 3 lines

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?

Quoted 6 lines · Yao

Quoted 5 lines · Michael Ellerman

Quoted 3 lines · Jin Yao

It is often useful to know the branch types while analyzing branch data. For example, a call is very different from a conditional branch.

...

Quoted 10 lines

Quoted 10 lines

Quoted 3 lines

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"?

Quoted 4 lines

PERF_BR_NONE : unknown
PERF_BR_JCC : conditional jump
PERF_BR_JMP : jump
PERF_BR_IND_JMP : indirect jump

Quoted 7 lines

Quoted 5 lines

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.

Quoted 2 lines

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.

Quoted 10 lines

Quoted 9 lines

Quoted 7 lines

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 ?

Quoted 2 lines

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.

Quoted 1 line

PERF_BR_SYSCALL/PERF_BR_SYSRET are for syscall and syscall return.

Yep OK.

Quoted 10 lines

Quoted 2 lines

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.

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

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?

Quoted 3 lines

Quoted 3 lines

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.

...

Quoted 2 lines

Sorry, I'm not familiar with powerpc arch. Or could you add the branch type which powerpc needs?

These are good:

include/uapi/linux/perf_event.h +6 -0

+	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:

include/uapi/linux/perf_event.h +3 -0

+	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:

(quoted diff) +2 -0

+	PERF_BR_IRQ		= 9,	/* hw interrupt/trap/fault */
+	PERF_BR_INT		= 10,	/* sw interrupt */

And sounds like this should be dropped for now:

(quoted diff) +1 -0

+	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

2. I wish to keep following definitions for x86.

(quoted diff) +2 -0

+	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).

4. I'd like to add following types for powerpc.

PERF_BR_COND_CALL /* Conditional call */
PERF_BR_COND_RET /* Condition return */

Hi!

include/uapi/linux/perf_event.h enum perf_branch_sample_type { · line 224

Quoted 11 lines · Yao

1. We all agree these definitions:

include/uapi/linux/perf_event.h +9 -0

+	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 */

Do we? It does not map very well to PowerPC branch types.

Quoted 8 lines

2. I wish to keep following definitions for x86.

(quoted diff) +2 -0

+	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).

So your "PERF_BR_INT" is a system call? And PERF_BR_IRQ is not an interrupt request (as its name suggests), not what we call an "external interrupt" either; instead it is every interrupt that is not a system call?

It also does not follow the lines of "software caused interrupt" vs. the rest.

Quoted 4 lines

4. I'd like to add following types for powerpc.

PERF_BR_COND_CALL /* Conditional call */
PERF_BR_COND_RET /* Condition return */

Almost all PowerPC branches have a "conditional" version (only "syscall" and "sysret/iret" do not -- and those last two are the same, just like PERF_BR_INT seems to be the same as PERF_BR_SYSCALL).

So how should those PERF_BR_* be used? It cannot be used in an architecture-neutral interface the way you define it now.

Segher

Quoted 3 lines

PERF_BR_INT is triggered by instruction "int" . PERF_BR_IRQ is triggered by interrupts, traps, faults (the ring 0,3 transition).

So your "PERF_BR_INT" is a system call?

And PERF_BR_IRQ is not an interrupt request (as its name suggests), not what we call an "external interrupt" either; instead it is every interrupt that is not a system call?

Quoted 4 lines · Segher Boessenkool

Quoted 3 lines

PERF_BR_INT is triggered by instruction "int" . PERF_BR_IRQ is triggered by interrupts, traps, faults (the ring 0,3 transition).

So your "PERF_BR_INT" is a system call?

The "INT" thing has indeed been used as system call mechanism (typically INT 80). But these days we have special purpose syscall instructions.

It could maybe be compared to the PPC "Unconditional TRAP with immediate" where you use the immediate value as an index into a handler vector.

Quoted 3 lines

And PERF_BR_IRQ is not an interrupt request (as its name suggests), not what we call an "external interrupt" either; instead it is every interrupt that is not a system call?

It is actual interrupts, but also faults, traps and all the other exceptions not caused by "INT" I think.

Quoted 13 lines · Peter Zijlstra

Quoted 4 lines · Segher Boessenkool

Quoted 3 lines

PERF_BR_INT is triggered by instruction "int" . PERF_BR_IRQ is triggered by interrupts, traps, faults (the ring 0,3 transition).

So your "PERF_BR_INT" is a system call?

The "INT" thing has indeed been used as system call mechanism (typically INT 80). But these days we have special purpose syscall instructions.

It could maybe be compared to the PPC "Unconditional TRAP with immediate" where you use the immediate value as an index into a handler vector.

Quoted 6 lines

Quoted 3 lines

And PERF_BR_IRQ is not an interrupt request (as its name suggests), not what we call an "external interrupt" either; instead it is every interrupt that is not a system call?

It is actual interrupts, but also faults, traps and all the other exceptions not caused by "INT" I think.

Yes. It's interrupt, traps, faults. If from is in the user space and to is in the kernel, it indicates the ring3 -> ring0 transition.

If the from instruction is not syscall or other ring transition instruction, it should be interrupt, traps and faults. That's how we get the PERF_BR_IRQ on x86.

Anyway, maybe we just use a minimum but the most common set of branch types now, it could be a good start and acceptable on all architectures.

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 */

Quoted 30 lines · Yao

Quoted 13 lines · Peter Zijlstra

Quoted 4 lines · Segher Boessenkool

Quoted 3 lines

PERF_BR_INT is triggered by instruction "int" . PERF_BR_IRQ is triggered by interrupts, traps, faults (the ring 0,3 transition).

So your "PERF_BR_INT" is a system call?

The "INT" thing has indeed been used as system call mechanism (typically INT 80). But these days we have special purpose syscall instructions.

It could maybe be compared to the PPC "Unconditional TRAP with immediate" where you use the immediate value as an index into a handler vector.

Quoted 6 lines

Quoted 3 lines

And PERF_BR_IRQ is not an interrupt request (as its name suggests), not what we call an "external interrupt" either; instead it is every interrupt that is not a system call?

It is actual interrupts, but also faults, traps and all the other exceptions not caused by "INT" I think.

Yes. It's interrupt, traps, faults. If from is in the user space and to is in the kernel, it indicates the ring3 -> ring0 transition.

If the from instruction is not syscall or other ring transition instruction, it should be interrupt, traps and faults. That's how we get the PERF_BR_IRQ on x86.

Anyway, maybe we just use a minimum but the most common set of branch types now, it could be a good start and acceptable on all architectures.

Quoted 6 lines

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 */

That would be fine by me, if you're sick of talking about it and just want to get it merged :)

I think you could expand it a bit, this list would cover the vast bulk of branch types for us:

PERF_BR_COND /* Conditional */
PERF_BR_UNCOND /* Unconditional */
PERF_BR_IND /* Indirect */
PERF_BR_CALL /* Function call */
PERF_BR_IND_CALL /* Indirect function call */
PERF_BR_RET /* Function return */
PERF_BR_SYSCALL /* Syscall */
PERF_BR_SYSRET /* Syscall return */
PERF_BR_COND_CALL /* Conditional function call */
PERF_BR_COND_RET /* Conditional function return */

cheers

Quoted 4 lines · Segher Boessenkool

Quoted 3 lines

PERF_BR_INT is triggered by instruction "int" . PERF_BR_IRQ is triggered by interrupts, traps, faults (the ring 0,3 transition).

Quoted 1 line

So your "PERF_BR_INT" is a system call?

The "INT" thing has indeed been used as system call mechanism (typically INT 80). But these days we have special purpose syscall instructions.

It could maybe be compared to the PPC "Unconditional TRAP with immediate" where you use the immediate value as an index into a handler vector.

Quoted 3 lines

And PERF_BR_IRQ is not an interrupt request (as its name suggests), not what we call an "external interrupt" either; instead it is every interrupt that is not a system call?

It is actual interrupts, but also faults, traps and all the other exceptions not caused by "INT" I think.

Hi!

Quoted 2 lines · Yao

1. We all agree these definitions:

include/uapi/linux/perf_event.h enum perf_branch_sample_type { · line 224

Quoted 9 lines

include/uapi/linux/perf_event.h +9 -0

+	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 */

Do we? It does not map very well to PowerPC branch types.

arch/x86/events/intel/lbr.c +17 -0

+#define X86_BR_TYPE_MAP_MAX	16
+
+static int
+common_branch_type(int type)
+{
+	int i, mask;
+	const int branch_map[X86_BR_TYPE_MAP_MAX] = {
+		PERF_BR_CALL,		/* X86_BR_CALL */
+		PERF_BR_RET,		/* X86_BR_RET */
+		PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
+		PERF_BR_SYSRET,		/* X86_BR_SYSRET */
+		PERF_BR_INT,		/* X86_BR_INT */
+		PERF_BR_IRET,		/* X86_BR_IRET */
+		PERF_BR_JCC,		/* X86_BR_JCC */
+		PERF_BR_JMP,		/* X86_BR_JMP */
+		PERF_BR_IRQ,		/* X86_BR_IRQ */
+		PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */

arch/x86/events/intel/lbr.c +7 -0

+
+		type >>= 1;
+	}
+
+	return PERF_BR_NONE;
+}
+
 /*

On Thu, Apr 20, 2017 at 08:07:50PM +0800, Jin Yao wrote:

SNIP

Quoted 18 lines

arch/x86/events/intel/lbr.c +17 -0

+#define X86_BR_TYPE_MAP_MAX	16
+
+static int
+common_branch_type(int type)
+{
+	int i, mask;
+	const int branch_map[X86_BR_TYPE_MAP_MAX] = {
+		PERF_BR_CALL,		/* X86_BR_CALL */
+		PERF_BR_RET,		/* X86_BR_RET */
+		PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
+		PERF_BR_SYSRET,		/* X86_BR_SYSRET */
+		PERF_BR_INT,		/* X86_BR_INT */
+		PERF_BR_IRET,		/* X86_BR_IRET */
+		PERF_BR_JCC,		/* X86_BR_JCC */
+		PERF_BR_JMP,		/* X86_BR_JMP */
+		PERF_BR_IRQ,		/* X86_BR_IRQ */
+		PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 964

Quoted 10 lines

arch/x86/events/intel/lbr.c +10 -0

+		PERF_BR_NONE,		/* X86_BR_ABORT */
+		PERF_BR_NONE,		/* X86_BR_IN_TX */
+		PERF_BR_NONE,		/* X86_BR_NO_TX */
+		PERF_BR_CALL,		/* X86_BR_ZERO_CALL */
+		PERF_BR_NONE,		/* X86_BR_CALL_STACK */
+		PERF_BR_IND_JMP,	/* X86_BR_IND_JMP */
+	};
+
+	type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+	mask = ~(~0 << 1);

is that a fancy way to get 1 into the mask? what do I miss?

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 968

Quoted 4 lines

arch/x86/events/intel/lbr.c +4 -0

+
+	for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+		if (type & mask)
+			return branch_map[i];

I wonder some bit search would be faster in here, but maybe not big deal

jirka

Quoted 9 lines

arch/x86/events/intel/lbr.c +7 -0

+
+		type >>= 1;
+	}
+
+	return PERF_BR_NONE;
+}
+
  /*

Quoted 39 lines · Jiri Olsa

On Thu, Apr 20, 2017 at 08:07:50PM +0800, Jin Yao wrote:

SNIP

Quoted 17 lines

+#define X86_BR_TYPE_MAP_MAX 16

arch/x86/events/intel/lbr.c +16 -0

+
+static int
+common_branch_type(int type)
+{
+    int i, mask;
+    const int branch_map[X86_BR_TYPE_MAP_MAX] = {
+        PERF_BR_CALL,        /* X86_BR_CALL */
+        PERF_BR_RET,        /* X86_BR_RET */
+        PERF_BR_SYSCALL,    /* X86_BR_SYSCALL */
+        PERF_BR_SYSRET,        /* X86_BR_SYSRET */
+        PERF_BR_INT,        /* X86_BR_INT */
+        PERF_BR_IRET,        /* X86_BR_IRET */
+        PERF_BR_JCC,        /* X86_BR_JCC */
+        PERF_BR_JMP,        /* X86_BR_JMP */
+        PERF_BR_IRQ,        /* X86_BR_IRQ */
+        PERF_BR_IND_CALL,    /* X86_BR_IND_CALL */

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 964

Quoted 10 lines

arch/x86/events/intel/lbr.c +10 -0

+        PERF_BR_NONE,        /* X86_BR_ABORT */
+        PERF_BR_NONE,        /* X86_BR_IN_TX */
+        PERF_BR_NONE,        /* X86_BR_NO_TX */
+        PERF_BR_CALL,        /* X86_BR_ZERO_CALL */
+        PERF_BR_NONE,        /* X86_BR_CALL_STACK */
+        PERF_BR_IND_JMP,    /* X86_BR_IND_JMP */
+    };
+
+    type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+    mask = ~(~0 << 1);

is that a fancy way to get 1 into the mask? what do I miss?

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 968

Quoted 4 lines

arch/x86/events/intel/lbr.c +4 -0

+
+    for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+        if (type & mask)
+            return branch_map[i];

I wonder some bit search would be faster in here, but maybe not big deal

Quoted 1 line

jirka

I just think the branch_map[] doesn't contain many entries (16 entries here), so maybe checking 1 bit one time should be acceptable. I just want to keep the code simple.

But if the number of entries is more (e.g. 64), maybe it'd better check 2 or 4 bits one time.

Thanks
Jin Yao

Quoted 24 lines · Jiri Olsa

On Thu, Apr 20, 2017 at 08:07:50PM +0800, Jin Yao wrote:

SNIP

Quoted 20 lines

arch/x86/events/intel/lbr.c +20 -0

+#define X86_BR_TYPE_MAP_MAX	16
+
+static int
+common_branch_type(int type)
+{
+	int i, mask;
+	const int branch_map[X86_BR_TYPE_MAP_MAX] = {
+		PERF_BR_CALL,		/* X86_BR_CALL */
+		PERF_BR_RET,		/* X86_BR_RET */
+		PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
+		PERF_BR_SYSRET,		/* X86_BR_SYSRET */
+		PERF_BR_INT,		/* X86_BR_INT */
+		PERF_BR_IRET,		/* X86_BR_IRET */
+		PERF_BR_JCC,		/* X86_BR_JCC */
+		PERF_BR_JMP,		/* X86_BR_JMP */
+		PERF_BR_IRQ,		/* X86_BR_IRQ */
+		PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
+		PERF_BR_NONE,		/* X86_BR_ABORT */
+		PERF_BR_NONE,		/* X86_BR_IN_TX */
+		PERF_BR_NONE,		/* X86_BR_NO_TX */

Quoted 8 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 964

Quoted 7 lines

arch/x86/events/intel/lbr.c +7 -0

+		PERF_BR_CALL,		/* X86_BR_ZERO_CALL */
+		PERF_BR_NONE,		/* X86_BR_CALL_STACK */
+		PERF_BR_IND_JMP,	/* X86_BR_IND_JMP */
+	};
+
+	type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+	mask = ~(~0 << 1);

is that a fancy way to get 1 into the mask? what do I miss?

Quoted 7 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 968

Quoted 5 lines

arch/x86/events/intel/lbr.c +4 -0

+
+	for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+		if (type & mask)
+			return branch_map[i];

I wonder some bit search would be faster in here, but maybe not big deal

Quoted 1 line

jirka

I just think the branch_map[] doesn't contain many entries (16 entries here), so maybe checking 1 bit one time should be acceptable. I just want to keep the code simple.

But if the number of entries is more (e.g. 64), maybe it'd better check 2 or 4 bits one time.

Quoted 26 lines · Yao

Quoted 24 lines · Jiri Olsa

On Thu, Apr 20, 2017 at 08:07:50PM +0800, Jin Yao wrote:

SNIP

Quoted 20 lines

arch/x86/events/intel/lbr.c +20 -0

+#define X86_BR_TYPE_MAP_MAX	16
+
+static int
+common_branch_type(int type)
+{
+	int i, mask;
+	const int branch_map[X86_BR_TYPE_MAP_MAX] = {
+		PERF_BR_CALL,		/* X86_BR_CALL */
+		PERF_BR_RET,		/* X86_BR_RET */
+		PERF_BR_SYSCALL,	/* X86_BR_SYSCALL */
+		PERF_BR_SYSRET,		/* X86_BR_SYSRET */
+		PERF_BR_INT,		/* X86_BR_INT */
+		PERF_BR_IRET,		/* X86_BR_IRET */
+		PERF_BR_JCC,		/* X86_BR_JCC */
+		PERF_BR_JMP,		/* X86_BR_JMP */
+		PERF_BR_IRQ,		/* X86_BR_IRQ */
+		PERF_BR_IND_CALL,	/* X86_BR_IND_CALL */
+		PERF_BR_NONE,		/* X86_BR_ABORT */
+		PERF_BR_NONE,		/* X86_BR_IN_TX */
+		PERF_BR_NONE,		/* X86_BR_NO_TX */

Quoted 8 lines

Quoted 8 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 964

Quoted 7 lines

arch/x86/events/intel/lbr.c +7 -0

+		PERF_BR_CALL,		/* X86_BR_ZERO_CALL */
+		PERF_BR_NONE,		/* X86_BR_CALL_STACK */
+		PERF_BR_IND_JMP,	/* X86_BR_IND_JMP */
+	};
+
+	type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+	mask = ~(~0 << 1);

is that a fancy way to get 1 into the mask? what do I miss?

you did not comment on this one

Quoted 11 lines

Quoted 7 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 968

Quoted 4 lines

arch/x86/events/intel/lbr.c +4 -0

+
+	for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+		if (type & mask)
+			return branch_map[i];

I wonder some bit search would be faster in here, but maybe not big deal

jirka

I just think the branch_map[] doesn't contain many entries (16 entries here), so maybe checking 1 bit one time should be acceptable. I just want to keep the code simple.

Quoted 2 lines

But if the number of entries is more (e.g. 64), maybe it'd better check 2 or 4 bits one time.

ook

jirka

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 964

Quoted 3 lines

arch/x86/events/intel/lbr.c +3 -0

+
+	type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+	mask = ~(~0 << 1);

is that a fancy way to get 1 into the mask? what do I miss?

you did not comment on this one

Sorry, I misunderstood that this comment and the next comment had the same meaning.

In the previous version, I used the switch/case to convert from X86_BR to PERF_BR. I got a comment from community that it'd better use a lookup table for conversion.

Since each bit in type represents a X86_BR type so I use a mask (0x1) to filter the bit. Yes, it looks I can also directly set 0x1 to mask.

I write the code "mask = ~(~0 << 1)" according to my coding habits. If you think I should change the code to "mask = 0x1", that's OK :)

Quoted 12 lines

Quoted 12 lines

Quoted 8 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 968

Quoted 5 lines

arch/x86/events/intel/lbr.c +4 -0

+
+	for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+		if (type & mask)
+			return branch_map[i];

I wonder some bit search would be faster in here, but maybe not big deal

jirka

I just think the branch_map[] doesn't contain many entries (16 entries here), so maybe checking 1 bit one time should be acceptable. I just want to keep the code simple.

Quoted 5 lines

Quoted 2 lines

But if the number of entries is more (e.g. 64), maybe it'd better check 2 or 4 bits one time.

ook

jirka

Sorry, what's the meaning of ook? Does it mean "OK"?

On Tue, May 09, 2017 at 07:57:11PM +0800, Jin, Yao wrote:

SNIP

Quoted 12 lines

Quoted 5 lines

Quoted 4 lines

Quoted 4 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 964

Quoted 3 lines

arch/x86/events/intel/lbr.c +3 -0

+
+	type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+	mask = ~(~0 << 1);

is that a fancy way to get 1 into the mask? what do I miss?

you did not comment on this one

Sorry, I misunderstood that this comment and the next comment had the same meaning.

In the previous version, I used the switch/case to convert from X86_BR to PERF_BR. I got a comment from community that it'd better use a lookup table for conversion.

Quoted 5 lines

Since each bit in type represents a X86_BR type so I use a mask (0x1) to filter the bit. Yes, it looks I can also directly set 0x1 to mask.

I write the code "mask = ~(~0 << 1)" according to my coding habits. If you think I should change the code to "mask = 0x1", that's OK :)

im ok with that.. was just wondering for the reason
I guess compiler will make it single constant assignment anyway

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Quoted 7 lines

arch/x86/events/intel/lbr.c static int branch_type(unsigned long from, unsigned long to, int abort) · line 968

Quoted 4 lines

arch/x86/events/intel/lbr.c +4 -0

+
+	for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+		if (type & mask)
+			return branch_map[i];

I wonder some bit search would be faster in here, but maybe not big deal

jirka

I just think the branch_map[] doesn't contain many entries (16 entries here), so maybe checking 1 bit one time should be acceptable. I just want to keep the code simple.

But if the number of entries is more (e.g. 64), maybe it'd better check 2 or 4 bits one time.

Quoted 4 lines

Quoted 3 lines

ook

jirka

Sorry, what's the meaning of ook? Does it mean "OK"?

just means ok ;-)

thanks,
jirka

v6:
Update according to the review comments from Jiri Olsa <[email protected]>. Major modifications are:

1. Move that multiline conditional code inside {} brackets.

2. Move branch_type_stat_display() from builtin-report.c to branch.c. Move branch_type_str() from callchain.c to branch.c.

3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations

Quoted 10 lines · Jin Yao

v6:
Update according to the review comments from Jiri Olsa <[email protected]>. Major modifications are:

1. Move that multiline conditional code inside {} brackets.

2. Move branch_type_stat_display() from builtin-report.c to branch.c. Move branch_type_str() from callchain.c to branch.c.

Quoted 2 lines

3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations

for the tools part

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Quoted 4 lines · Jin Yao

v6:
Update according to the review comments from Jiri Olsa <[email protected]>. Major modifications are:

Quoted 8 lines

1. Move that multiline conditional code inside {} brackets.

2. Move branch_type_stat_display() from builtin-report.c to branch.c. Move branch_type_str() from callchain.c to branch.c.

3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations

for the tools part

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Hi maintainers,

Is this patch series (v6) OK for merging?

Thanks

Jin Yao

Quoted 19 lines · Jiri Olsa

Quoted 4 lines · Jin Yao

v6:
Update according to the review comments from Jiri Olsa <[email protected]>. Major modifications are:

Quoted 8 lines

1. Move that multiline conditional code inside {} brackets.

2. Move branch_type_stat_display() from builtin-report.c to branch.c. Move branch_type_str() from callchain.c to branch.c.

3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations

for the tools part

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Hi maintainers,

Is this patch series OK or anything I should update?

Thanks

Jin Yao

Quoted 30 lines · Yao

Hi maintainers,

Is this patch series (v6) OK for merging?

Thanks

Jin Yao

Quoted 19 lines · Jiri Olsa

Quoted 4 lines · Jin Yao

v6:
Update according to the review comments from Jiri Olsa <[email protected]>. Major modifications are:

Quoted 8 lines

1. Move that multiline conditional code inside {} brackets.

2. Move branch_type_stat_display() from builtin-report.c to branch.c. Move branch_type_str() from callchain.c to branch.c.

3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations

for the tools part

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

v6:
Update according to the review comments from Jiri Olsa <[email protected]>. Major modifications are:

1. Move that multiline conditional code inside {} brackets.

2. Move branch_type_stat_display() from builtin-report.c to branch.c. Move branch_type_str() from callchain.c to branch.c.

3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations

v5:
---
Mainly the v5 patch series are updated according to comments from Jiri Olsa <[email protected]>.

The kernel part doesn't have functional change. It just solve the merge issue.

In userspace, the functions of branch type counting and branch type name resolving are moved to the new files:
util/branch.c, util/branch.h.

And refactor the branch info printing code for better maintenance.

Not changed (or just fix merge issue):
perf/core: Define the common branch type classification perf/x86/intel: Record branch type
perf record: Create a new option save_type in --branch-filter

New patches:
perf report: Refactor the branch info printing code perf util: Create branch.c/.h for common branch functions

Changed:
perf report: Show branch type statistics for stdio mode perf report: Show branch type in callchain entry

v4:
---
1. Describe the major changes in patch description. Thanks for Peter Zijlstra's reminding.

2. Initialize branch type to 0 in intel_pmu_lbr_read_32 and intel_pmu_lbr_read_64. Remove the invalid else code in intel_pmu_lbr_filter.

v3:
---
1. Move the JCC forward/backward and cross page computing from kernel to userspace.

2. Use lookup table to replace original switch/case processing.

Changed:
perf/core: Define the common branch type classification perf/x86/intel: Record branch type
perf report: Show branch type statistics for stdio mode perf report: Show branch type in callchain entry

Not changed:
perf record: Create a new option save_type in --branch-filter

v2:
---
1. Use 4 bits in perf_branch_entry to record branch type.

2. Pull out some common branch types from FAR_BRANCH. Now the branch types defined in perf_event.h:

Jin Yao (7):
perf/core: Define the common branch type classification perf/x86/intel: Record branch type
perf record: Create a new option save_type in --branch-filter perf report: Refactor the branch info printing code perf util: Create branch.c/.h for common branch functions perf report: Show branch type statistics for stdio mode perf report: Show branch type in callchain entry

 arch/x86/events/intel/lbr.c              |  53 +++++++++-
 include/uapi/linux/perf_event.h          |  29 +++++-
 tools/include/uapi/linux/perf_event.h    |  29 +++++-
 tools/perf/Documentation/perf-record.txt |   1 +
 tools/perf/builtin-report.c              |  25 +++++
 tools/perf/util/Build                    |   1 +
 tools/perf/util/branch.c                 | 168 +++++++++++++++++++++++++++++++
 tools/perf/util/branch.h                 |  25 +++++
 tools/perf/util/callchain.c              | 140 ++++++++++++++------------
 tools/perf/util/callchain.h              |   5 +-
 tools/perf/util/event.h                  |   3 +-
 tools/perf/util/hist.c                   |   5 +-
 tools/perf/util/machine.c                |  26 +++--
 tools/perf/util/parse-branch-options.c   |   1 +
 14 files changed, 427 insertions(+), 84 deletions(-)
 create mode 100644 tools/perf/util/branch.c
 create mode 100644 tools/perf/util/branch.h

Signature · 1 line

2.7.4