Skip to contentSkip to conversation

x86, perf: Fix LBR call stack save/restore

PATCH Applied Andi Kleen 5 participants 16 messages Oct 20, 2015 – Nov 23, 2015

Subsystems (3)
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/5 x86, perf: Fix LBR call stack save/restore

From: Andi Kleen <[email protected]>

This fixes a bug added with the earlier 90405aa02. The bug could lead to lost LBR call stacks. When restoring the LBR state we need to use the TOS of the previous context, not the current context. To do that we need to save/restore the tos.

---
 arch/x86/kernel/cpu/perf_event.h           | 1 +
 arch/x86/kernel/cpu/perf_event_intel_lbr.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/perf_event.h +1 -0
index d871c94..1b47164 100644
@@ -631,6 +631,7 @@ struct x86_perf_task_context {
 	u64 lbr_from[MAX_LBR_ENTRIES];
 	u64 lbr_to[MAX_LBR_ENTRIES];
 	u64 lbr_info[MAX_LBR_ENTRIES];
+	int tos;
 	int lbr_callstack_users;
 	int lbr_stack_state;
 };
arch/x86/kernel/cpu/perf_event_intel_lbr.c +3 -1
index ad0b8b0..0e4ea00 100644
@@ -239,7 +239,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 	}
 
 	mask = x86_pmu.lbr_nr - 1;
-	tos = intel_pmu_lbr_tos();
+	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
 		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
@@ -247,6 +247,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
+	wrmsrl(x86_pmu.lbr_tos, tos);
 	task_ctx->lbr_stack_state = LBR_NONE;
 }
 
@@ -270,6 +271,7 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
+	task_ctx->tos = tos;
 	task_ctx->lbr_stack_state = LBR_VALID;
 }
 
Signature · 1 line
2.4.3
author
#1 lore raw

PATCH 2/5 x86, perf: Add option to disable reading branch flags/cycles

From: Andi Kleen <[email protected]>

With LBRv5 reading the extra LBR flags like mispredict, TSX, cycles is not free anymore, as it has moved to a separate MSR.

For callstack mode we don't need any of this information; so we can avoid the unnecessary MSR read. Add flags to the perf interface where perf record can request not collecting this information.

I added branch_sample_type flags for CYCLES and FLAGS. It's a bit unusual for branch_sample_types to be negative (disable), not positive (enable), but since the legacy ABI reported the flags we need some form of explicit disabling to avoid breaking the ABI.

After we have the flags the x86 perf code can keep track if any users need the flags. If noone needs it the information is not collected.

This cuts down the cost of LBR callstack on Skylake significantly. Profiling a kernel build with LBR call stack the average run time of the PMI handler drops by 43%

v2: Rework based on Peter's feedback.
Moved the NO_FLAGS/NO_CYCLES bits to branch_sample_type. Remove extra checks for no info, so that the flags are filled in if there is no lbr_info or if only one is set to reduce number of branches. Instead of counters the information about skipping LBR_INFO is now kept as a extra register bit that is filtered out before writing the register to hardware.

---
 arch/x86/kernel/cpu/perf_event_intel_lbr.c | 19 +++++++++++++++++--
 include/uapi/linux/perf_event.h            |  6 ++++++
 2 files changed, 23 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/perf_event_intel_lbr.c +17 -2
index 0e4ea00..60e71b7 100644
@@ -42,6 +42,13 @@ static enum {
 #define LBR_FAR_BIT		8 /* do not capture far branches */
 #define LBR_CALL_STACK_BIT	9 /* enable call stack */
 
+/*
+ * Following bit only exists in Linux; we mask it out before writing it to
+ * the actual MSR. But it helps the constraint perf code to understand
+ * that this is a separate configuration.
+ */
+#define LBR_NO_INFO_BIT	       63 /* don't read LBR_INFO. */
+
 #define LBR_KERNEL	(1 << LBR_KERNEL_BIT)
 #define LBR_USER	(1 << LBR_USER_BIT)
 #define LBR_JCC		(1 << LBR_JCC_BIT)
@@ -52,6 +59,7 @@ static enum {
 #define LBR_IND_JMP	(1 << LBR_IND_JMP_BIT)
 #define LBR_FAR		(1 << LBR_FAR_BIT)
 #define LBR_CALL_STACK	(1 << LBR_CALL_STACK_BIT)
+#define LBR_NO_INFO	(1ULL << LBR_NO_INFO_BIT)
 
 #define LBR_PLM (LBR_KERNEL | LBR_USER)
 
@@ -152,7 +160,7 @@ static void __intel_pmu_lbr_enable(bool pmi)
 	 * did not change.
 	 */
 	if (cpuc->lbr_sel)
-		lbr_select = cpuc->lbr_sel->config;
+		lbr_select = cpuc->lbr_sel->config & x86_pmu.lbr_sel_mask;
 	if (!pmi)
 		wrmsrl(MSR_LBR_SELECT, lbr_select);
 
@@ -422,6 +430,7 @@ static void intel_pmu_lbr_read_32(struct cpu_hw_events *cpuc)
  */
 static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 {
+	bool need_info = !(cpuc->lbr_sel->config & LBR_NO_INFO);
 	unsigned long mask = x86_pmu.lbr_nr - 1;
 	int lbr_format = x86_pmu.intel_cap.lbr_format;
 	u64 tos = intel_pmu_lbr_tos();
@@ -442,7 +451,7 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 		rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
 		rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
 
-		if (lbr_format == LBR_FORMAT_INFO) {
+		if (lbr_format == LBR_FORMAT_INFO && need_info) {
 			u64 info;
 
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, info);
@@ -588,6 +597,7 @@ static int intel_pmu_setup_hw_lbr_filter(struct perf_event *event)
 		if (v != LBR_IGN)
 			mask |= v;
 	}
+
 	reg = &event->hw.branch_reg;
 	reg->idx = EXTRA_REG_LBR;
 
@@ -598,6 +608,11 @@ static int intel_pmu_setup_hw_lbr_filter(struct perf_event *event)
 	 */
 	reg->config = mask ^ x86_pmu.lbr_sel_mask;
 
+	if ((br_type & PERF_SAMPLE_BRANCH_NO_CYCLES) &&
+	    (br_type & PERF_SAMPLE_BRANCH_NO_FLAGS) &&
+	    (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO))
+		reg->config |= LBR_NO_INFO;
+
 	return 0;
 }
 
include/uapi/linux/perf_event.h +6 -0
index 2881145..a8ffc76 100644
@@ -169,6 +169,9 @@ enum perf_branch_sample_type_shift {
 	PERF_SAMPLE_BRANCH_CALL_STACK_SHIFT	= 11, /* call/ret stack */
 	PERF_SAMPLE_BRANCH_IND_JUMP_SHIFT	= 12, /* indirect jumps */
 
+	PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT	= 13, /* no flags */
+	PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT	= 14, /* no cycles */
+
 	PERF_SAMPLE_BRANCH_MAX_SHIFT		/* non-ABI */
 };
 
@@ -189,6 +192,9 @@ enum perf_branch_sample_type {
 	PERF_SAMPLE_BRANCH_CALL_STACK	= 1U << PERF_SAMPLE_BRANCH_CALL_STACK_SHIFT,
 	PERF_SAMPLE_BRANCH_IND_JUMP	= 1U << PERF_SAMPLE_BRANCH_IND_JUMP_SHIFT,
 
+	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_MAX		= 1U << PERF_SAMPLE_BRANCH_MAX_SHIFT,
 };
 
Signature · 1 line
2.4.3
#2 lore raw

[tip:perf/core] perf/x86: Add option to disable reading branch flags/cycles

perf/x86: Add option to disable reading branch flags/cycles
Commit-ID:  b16a5b52eb90d92b597257778e51e1fdc6423e64
Gitweb:     http://git.kernel.org/tip/b16a5b52eb90d92b597257778e51e1fdc6423e64
Author:     Andi Kleen <[email protected]>
AuthorDate: Tue, 20 Oct 2015 11:46:34 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 23 Nov 2015 09:58:25 +0100

perf/x86: Add option to disable reading branch flags/cycles

With LBRv5 reading the extra LBR flags like mispredict, TSX, cycles is not free anymore, as it has moved to a separate MSR.

For callstack mode we don't need any of this information; so we can avoid the unnecessary MSR read. Add flags to the perf interface where perf record can request not collecting this information.

Add branch_sample_type flags for CYCLES and FLAGS. It's a bit unusual for branch_sample_types to be negative (disable), not positive (enable), but since the legacy ABI reported the flags we need some form of explicit disabling to avoid breaking the ABI.

After we have the flags the x86 perf code can keep track if any users need the flags. If noone needs it the information is not collected.

This cuts down the cost of LBR callstack on Skylake significantly. Profiling a kernel build with LBR call stack the average run time of the PMI handler drops by 43%.

---
 arch/x86/kernel/cpu/perf_event_intel_lbr.c | 19 +++++++++++++++++--
 include/uapi/linux/perf_event.h            |  6 ++++++
 2 files changed, 23 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/perf_event_intel_lbr.c +17 -2
index 659f01e..e2fad0c 100644
@@ -42,6 +42,13 @@ static enum {
 #define LBR_FAR_BIT		8 /* do not capture far branches */
 #define LBR_CALL_STACK_BIT	9 /* enable call stack */
 
+/*
+ * Following bit only exists in Linux; we mask it out before writing it to
+ * the actual MSR. But it helps the constraint perf code to understand
+ * that this is a separate configuration.
+ */
+#define LBR_NO_INFO_BIT	       63 /* don't read LBR_INFO. */
+
 #define LBR_KERNEL	(1 << LBR_KERNEL_BIT)
 #define LBR_USER	(1 << LBR_USER_BIT)
 #define LBR_JCC		(1 << LBR_JCC_BIT)
@@ -52,6 +59,7 @@ static enum {
 #define LBR_IND_JMP	(1 << LBR_IND_JMP_BIT)
 #define LBR_FAR		(1 << LBR_FAR_BIT)
 #define LBR_CALL_STACK	(1 << LBR_CALL_STACK_BIT)
+#define LBR_NO_INFO	(1ULL << LBR_NO_INFO_BIT)
 
 #define LBR_PLM (LBR_KERNEL | LBR_USER)
 
@@ -152,7 +160,7 @@ static void __intel_pmu_lbr_enable(bool pmi)
 	 * did not change.
 	 */
 	if (cpuc->lbr_sel)
-		lbr_select = cpuc->lbr_sel->config;
+		lbr_select = cpuc->lbr_sel->config & x86_pmu.lbr_sel_mask;
 	if (!pmi)
 		wrmsrl(MSR_LBR_SELECT, lbr_select);
 
@@ -422,6 +430,7 @@ static void intel_pmu_lbr_read_32(struct cpu_hw_events *cpuc)
  */
 static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 {
+	bool need_info = !(cpuc->lbr_sel->config & LBR_NO_INFO);
 	unsigned long mask = x86_pmu.lbr_nr - 1;
 	int lbr_format = x86_pmu.intel_cap.lbr_format;
 	u64 tos = intel_pmu_lbr_tos();
@@ -442,7 +451,7 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 		rdmsrl(x86_pmu.lbr_from + lbr_idx, from);
 		rdmsrl(x86_pmu.lbr_to   + lbr_idx, to);
 
-		if (lbr_format == LBR_FORMAT_INFO) {
+		if (lbr_format == LBR_FORMAT_INFO && need_info) {
 			u64 info;
 
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, info);
@@ -590,6 +599,7 @@ static int intel_pmu_setup_hw_lbr_filter(struct perf_event *event)
 		if (v != LBR_IGN)
 			mask |= v;
 	}
+
 	reg = &event->hw.branch_reg;
 	reg->idx = EXTRA_REG_LBR;
 
@@ -600,6 +610,11 @@ static int intel_pmu_setup_hw_lbr_filter(struct perf_event *event)
 	 */
 	reg->config = mask ^ x86_pmu.lbr_sel_mask;
 
+	if ((br_type & PERF_SAMPLE_BRANCH_NO_CYCLES) &&
+	    (br_type & PERF_SAMPLE_BRANCH_NO_FLAGS) &&
+	    (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO))
+		reg->config |= LBR_NO_INFO;
+
 	return 0;
 }
 
include/uapi/linux/perf_event.h +6 -0
index d801bb0..1afe962 100644
@@ -171,6 +171,9 @@ enum perf_branch_sample_type_shift {
 	PERF_SAMPLE_BRANCH_IND_JUMP_SHIFT	= 12, /* indirect jumps */
 	PERF_SAMPLE_BRANCH_CALL_SHIFT		= 13, /* direct call */
 
+	PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT	= 14, /* no flags */
+	PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT	= 15, /* no cycles */
+
 	PERF_SAMPLE_BRANCH_MAX_SHIFT		/* non-ABI */
 };
 
@@ -192,6 +195,9 @@ enum perf_branch_sample_type {
 	PERF_SAMPLE_BRANCH_IND_JUMP	= 1U << PERF_SAMPLE_BRANCH_IND_JUMP_SHIFT,
 	PERF_SAMPLE_BRANCH_CALL		= 1U << PERF_SAMPLE_BRANCH_CALL_SHIFT,
 
+	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_MAX		= 1U << PERF_SAMPLE_BRANCH_MAX_SHIFT,
 };
 
author
#3 lore raw

PATCH 3/5 perf, tools: Disable branch flags/cycles for --callgraph lbr

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

Automatically disable collecting branch flags and cycles with --call-graph lbr. This allows avoiding a bunch of extra MSR reads in the PMI on Skylake.

When the kernel doesn't support the new flags they are automatically cleared in the fallback code.

v2: Switch to use branch_sample_type instead of sample_type. Adjust description.
Fix the fallback logic.

---
 tools/perf/util/evsel.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
tools/perf/util/evsel.c +13 -1
index 8be867c..e8724b4 100644
@@ -36,6 +36,7 @@ static struct {
 	bool cloexec;
 	bool clockid;
 	bool clockid_wrong;
+	bool lbr_flags;
 } perf_missing_features;
 
 static clockid_t clockid;
@@ -573,7 +574,9 @@ perf_evsel__config_callgraph(struct perf_evsel *evsel,
 			} else {
 				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
 				attr->branch_sample_type = PERF_SAMPLE_BRANCH_USER |
-							PERF_SAMPLE_BRANCH_CALL_STACK;
+							PERF_SAMPLE_BRANCH_CALL_STACK |
+							PERF_SAMPLE_BRANCH_NO_CYCLES |
+							PERF_SAMPLE_BRANCH_NO_FLAGS;
 			}
 		} else
 			 pr_warning("Cannot use LBR callstack with branch stack. "
@@ -1312,6 +1315,9 @@ fallback_missing_features:
 		evsel->attr.mmap2 = 0;
 	if (perf_missing_features.exclude_guest)
 		evsel->attr.exclude_guest = evsel->attr.exclude_host = 0;
+	if (perf_missing_features.lbr_flags)
+		evsel->attr.branch_sample_type &= ~(PERF_SAMPLE_BRANCH_NO_FLAGS |
+				     PERF_SAMPLE_BRANCH_NO_CYCLES);
 retry_sample_id:
 	if (perf_missing_features.sample_id_all)
 		evsel->attr.sample_id_all = 0;
@@ -1414,6 +1420,12 @@ try_fallback:
 	} else if (!perf_missing_features.sample_id_all) {
 		perf_missing_features.sample_id_all = true;
 		goto retry_sample_id;
+	} else if (!perf_missing_features.lbr_flags &&
+			(evsel->attr.branch_sample_type &
+			 (PERF_SAMPLE_BRANCH_NO_CYCLES |
+			  PERF_SAMPLE_BRANCH_NO_FLAGS))) {
+		perf_missing_features.lbr_flags = true;
+		goto fallback_missing_features;
 	}
 
 out_close:
Signature · 1 line
2.4.3
maintainer
#4 lore raw

Re: [PATCH 3/5] perf, tools: Disable branch flags/cycles for --callgraph lbr

Quoted 11 lines · Andi Kleen

From: Andi Kleen <[email protected]>

Automatically disable collecting branch flags and cycles with --call-graph lbr. This allows avoiding a bunch of extra MSR reads in the PMI on Skylake.

When the kernel doesn't support the new flags they are automatically cleared in the fallback code.

---

Arnaldo, ACK?

Quoted 52 lines
 tools/perf/util/evsel.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
tools/perf/util/evsel.c +13 -1
index 8be867c..e8724b4 100644
@@ -36,6 +36,7 @@ static struct {
 	bool cloexec;
 	bool clockid;
 	bool clockid_wrong;
+	bool lbr_flags;
 } perf_missing_features;
 
 static clockid_t clockid;
@@ -573,7 +574,9 @@ perf_evsel__config_callgraph(struct perf_evsel *evsel,
 			} else {
 				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
 				attr->branch_sample_type = PERF_SAMPLE_BRANCH_USER |
-							PERF_SAMPLE_BRANCH_CALL_STACK;
+							PERF_SAMPLE_BRANCH_CALL_STACK |
+							PERF_SAMPLE_BRANCH_NO_CYCLES |
+							PERF_SAMPLE_BRANCH_NO_FLAGS;
 			}
 		} else
 			 pr_warning("Cannot use LBR callstack with branch stack. "
@@ -1312,6 +1315,9 @@ fallback_missing_features:
 		evsel->attr.mmap2 = 0;
 	if (perf_missing_features.exclude_guest)
 		evsel->attr.exclude_guest = evsel->attr.exclude_host = 0;
+	if (perf_missing_features.lbr_flags)
+		evsel->attr.branch_sample_type &= ~(PERF_SAMPLE_BRANCH_NO_FLAGS |
+				     PERF_SAMPLE_BRANCH_NO_CYCLES);
 retry_sample_id:
 	if (perf_missing_features.sample_id_all)
 		evsel->attr.sample_id_all = 0;
@@ -1414,6 +1420,12 @@ try_fallback:
 	} else if (!perf_missing_features.sample_id_all) {
 		perf_missing_features.sample_id_all = true;
 		goto retry_sample_id;
+	} else if (!perf_missing_features.lbr_flags &&
+			(evsel->attr.branch_sample_type &
+			 (PERF_SAMPLE_BRANCH_NO_CYCLES |
+			  PERF_SAMPLE_BRANCH_NO_FLAGS))) {
+		perf_missing_features.lbr_flags = true;
+		goto fallback_missing_features;
 	}
 
 out_close:
Signature · 1 line
2.4.3
maintainer Acked-by
#5 lore raw

Re: [PATCH 3/5] perf, tools: Disable branch flags/cycles for --callgraph lbr

Em Wed, Oct 21, 2015 at 03:27:42PM +0200, Peter Zijlstra escreveu:

Quoted 7 lines · Andi Kleen
Quoted 6 lines

From: Andi Kleen <[email protected]>

Automatically disable collecting branch flags and cycles with --call-graph lbr. This allows avoiding a bunch of extra MSR reads in the PMI on Skylake.

Quoted 7 lines
Quoted 5 lines

When the kernel doesn't support the new flags they are automatically cleared in the fallback code.

---

Arnaldo, ACK?

I looked at this yesterday, seems to follow the existing mechanisms for fallbacking on older kernels, etc.

Quoted 52 lines
Quoted 52 lines
 tools/perf/util/evsel.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
tools/perf/util/evsel.c +13 -1
index 8be867c..e8724b4 100644
@@ -36,6 +36,7 @@ static struct {
 	bool cloexec;
 	bool clockid;
 	bool clockid_wrong;
+	bool lbr_flags;
 } perf_missing_features;
 
 static clockid_t clockid;
@@ -573,7 +574,9 @@ perf_evsel__config_callgraph(struct perf_evsel *evsel,
 			} else {
 				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
 				attr->branch_sample_type = PERF_SAMPLE_BRANCH_USER |
-							PERF_SAMPLE_BRANCH_CALL_STACK;
+							PERF_SAMPLE_BRANCH_CALL_STACK |
+							PERF_SAMPLE_BRANCH_NO_CYCLES |
+							PERF_SAMPLE_BRANCH_NO_FLAGS;
 			}
 		} else
 			 pr_warning("Cannot use LBR callstack with branch stack. "
@@ -1312,6 +1315,9 @@ fallback_missing_features:
 		evsel->attr.mmap2 = 0;
 	if (perf_missing_features.exclude_guest)
 		evsel->attr.exclude_guest = evsel->attr.exclude_host = 0;
+	if (perf_missing_features.lbr_flags)
+		evsel->attr.branch_sample_type &= ~(PERF_SAMPLE_BRANCH_NO_FLAGS |
+				     PERF_SAMPLE_BRANCH_NO_CYCLES);
 retry_sample_id:
 	if (perf_missing_features.sample_id_all)
 		evsel->attr.sample_id_all = 0;
@@ -1414,6 +1420,12 @@ try_fallback:
 	} else if (!perf_missing_features.sample_id_all) {
 		perf_missing_features.sample_id_all = true;
 		goto retry_sample_id;
+	} else if (!perf_missing_features.lbr_flags &&
+			(evsel->attr.branch_sample_type &
+			 (PERF_SAMPLE_BRANCH_NO_CYCLES |
+			  PERF_SAMPLE_BRANCH_NO_FLAGS))) {
+		perf_missing_features.lbr_flags = true;
+		goto fallback_missing_features;
 	}
 
 out_close:
Signature · 1 line
2.4.3
author
#6 lore raw

PATCH 4/5 perf, tools: Print branch filter state with -vv

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

Add a missing field to the perf_event_attr debug output.

---
 tools/perf/util/evsel.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/util/evsel.c +1 -0
index e8724b4..2b75d56 100644
@@ -1232,6 +1232,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 	PRINT_ATTRf(config, p_hex);
 	PRINT_ATTRn("{ sample_period, sample_freq }", sample_period, p_unsigned);
 	PRINT_ATTRf(sample_type, p_sample_type);
+	PRINT_ATTRf(branch_sample_type, p_unsigned);
 	PRINT_ATTRf(read_format, p_read_format);
 
 	PRINT_ATTRf(disabled, p_unsigned);
Signature · 1 line
2.4.3
maintainer Applied
#7 lore raw

Re: [PATCH 4/5] perf, tools: Print branch filter state with -vv

Em Tue, Oct 20, 2015 at 11:46:36AM -0700, Andi Kleen escreveu:

Quoted 3 lines

From: Andi Kleen <[email protected]>

Add a missing field to the perf_event_attr debug output.

Thanks, applied.

- Arnaldo

Quoted 19 lines
---
 tools/perf/util/evsel.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/util/evsel.c +1 -0
index e8724b4..2b75d56 100644
@@ -1232,6 +1232,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 	PRINT_ATTRf(config, p_hex);
 	PRINT_ATTRn("{ sample_period, sample_freq }", sample_period, p_unsigned);
 	PRINT_ATTRf(sample_type, p_sample_type);
+	PRINT_ATTRf(branch_sample_type, p_unsigned);
 	PRINT_ATTRf(read_format, p_read_format);
 
 	PRINT_ATTRf(disabled, p_unsigned);
Signature · 1 line
2.4.3
maintainer
#8 lore raw

Re: [PATCH 4/5] perf, tools: Print branch filter state with -vv

Quoted 18 lines · Arnaldo Carvalho de Melo

Em Tue, Oct 20, 2015 at 11:46:36AM -0700, Andi Kleen escreveu:

Quoted 3 lines

From: Andi Kleen <[email protected]>

Add a missing field to the perf_event_attr debug output.

Thanks, applied.

Quoted 11 lines
tools/perf/util/evsel.c +1 -0
index e8724b4..2b75d56 100644
@@ -1232,6 +1232,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 	PRINT_ATTRf(config, p_hex);
 	PRINT_ATTRn("{ sample_period, sample_freq }", sample_period, p_unsigned);
 	PRINT_ATTRf(sample_type, p_sample_type);
+	PRINT_ATTRf(branch_sample_type, p_unsigned);
 	PRINT_ATTRf(read_format, p_read_format);
 
Quoted 1 line

tools/perf/util/evsel.c int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr, · line 1238

Quoted 1 line
tools/perf/util/evsel.c +0 -0
 	PRINT_ATTRf(disabled, p_unsigned);

That's the wrong place.. it should be between config2 and sample_regs_user.

#9 lore raw

[tip:perf/core] perf evsel: Print branch filter state with -vv

In reply to Andi Kleen
perf evsel: Print branch filter state with -vv
Commit-ID:  8b8cde49586566471d65af9a59e25d3edb941387
Gitweb:     http://git.kernel.org/tip/8b8cde49586566471d65af9a59e25d3edb941387
Author:     Andi Kleen <[email protected]>
AuthorDate: Tue, 20 Oct 2015 11:46:36 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 21 Oct 2015 18:12:29 -0300

perf evsel: Print branch filter state with -vv

Add a missing field to the perf_event_attr debug output.

---
 tools/perf/util/evsel.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/util/evsel.c +1 -0
index 8be867c..ab05fa5 100644
@@ -1262,6 +1262,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 	PRINT_ATTRf(bp_type, p_unsigned);
 	PRINT_ATTRn("{ bp_addr, config1 }", bp_addr, p_hex);
 	PRINT_ATTRn("{ bp_len, config2 }", bp_len, p_hex);
+	PRINT_ATTRf(branch_sample_type, p_unsigned);
 	PRINT_ATTRf(sample_regs_user, p_hex);
 	PRINT_ATTRf(sample_stack_user, p_unsigned);
 	PRINT_ATTRf(clockid, p_signed);
author
#10 lore raw

PATCH 5/5 x86, perf: Avoid context switching LBR_INFO when not needed

In reply to Andi Kleen

From: Andi Kleen <[email protected]>

We context switch LBRs in call stack mode. Currently LBR_INFO was also context switched, but we normally don't need that in call stack mode.

Make the context switch check the NO_CYCLES|NO_FLAGS event flags that were earlier added, and if set avoid writing the LBR_INFO MSRs unnecessarily.

The same is done for the LBR reset code.

---
 arch/x86/kernel/cpu/perf_event.h           |  3 ++-
 arch/x86/kernel/cpu/perf_event_intel.c     |  2 +-
 arch/x86/kernel/cpu/perf_event_intel_lbr.c | 25 ++++++++++++++++---------
 3 files changed, 19 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/perf_event.h +2 -1
index 1b47164..4ae66e3 100644
@@ -634,6 +634,7 @@ struct x86_perf_task_context {
 	int tos;
 	int lbr_callstack_users;
 	int lbr_stack_state;
+	int need_info;
 };
 
 #define x86_add_quirk(func_)						\
@@ -887,7 +888,7 @@ void intel_ds_init(void);
 
 void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in);
 
-void intel_pmu_lbr_reset(void);
+void intel_pmu_lbr_reset(bool need_info);
 
 void intel_pmu_lbr_enable(struct perf_event *event);
 
arch/x86/kernel/cpu/perf_event_intel.c +1 -1
index f17772a..42f21f0 100644
@@ -2844,7 +2844,7 @@ static void intel_pmu_cpu_starting(int cpu)
 	/*
 	 * Deal with CPUs that don't clear their LBRs on power-up.
 	 */
-	intel_pmu_lbr_reset();
+	intel_pmu_lbr_reset(1);
 
 	cpuc->lbr_sel = NULL;
 
arch/x86/kernel/cpu/perf_event_intel_lbr.c +16 -9
index 60e71b7..7c21efb 100644
@@ -195,27 +195,30 @@ static void intel_pmu_lbr_reset_32(void)
 		wrmsrl(x86_pmu.lbr_from + i, 0);
 }
 
-static void intel_pmu_lbr_reset_64(void)
+static void intel_pmu_lbr_reset_64(bool need_info)
 {
 	int i;
 
 	for (i = 0; i < x86_pmu.lbr_nr; i++) {
 		wrmsrl(x86_pmu.lbr_from + i, 0);
 		wrmsrl(x86_pmu.lbr_to   + i, 0);
-		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
+		if (need_info)
 			wrmsrl(MSR_LBR_INFO_0 + i, 0);
 	}
 }
 
-void intel_pmu_lbr_reset(void)
+void intel_pmu_lbr_reset(bool need_info)
 {
 	if (!x86_pmu.lbr_nr)
 		return;
 
+	if (x86_pmu.intel_cap.lbr_format != LBR_FORMAT_INFO)
+		need_info = false;
+
 	if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_32)
 		intel_pmu_lbr_reset_32();
 	else
-		intel_pmu_lbr_reset_64();
+		intel_pmu_lbr_reset_64(need_info);
 }
 
 /*
@@ -242,7 +245,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 
 	if (task_ctx->lbr_callstack_users == 0 ||
 	    task_ctx->lbr_stack_state == LBR_NONE) {
-		intel_pmu_lbr_reset();
+		intel_pmu_lbr_reset(task_ctx->need_info > 0);
 		return;
 	}
 
@@ -252,7 +255,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 		lbr_idx = (tos - i) & mask;
 		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
 		wrmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
-		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
+		if (task_ctx->need_info)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
 	wrmsrl(x86_pmu.lbr_tos, tos);
@@ -276,7 +279,7 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 		lbr_idx = (tos - i) & mask;
 		rdmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
 		rdmsrl(x86_pmu.lbr_to + lbr_idx, task_ctx->lbr_to[i]);
-		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
+		if (task_ctx->need_info)
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
 	task_ctx->tos = tos;
@@ -317,7 +320,7 @@ void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in)
 	 * stack with branch from multiple tasks.
  	 */
 	if (sched_in) {
-		intel_pmu_lbr_reset();
+		intel_pmu_lbr_reset(!task_ctx || task_ctx->need_info > 0);
 		cpuc->lbr_context = ctx;
 	}
 }
@@ -340,7 +343,7 @@ void intel_pmu_lbr_enable(struct perf_event *event)
 	 * avoid data leaks.
 	 */
 	if (event->ctx->task && cpuc->lbr_context != event->ctx) {
-		intel_pmu_lbr_reset();
+		intel_pmu_lbr_reset(!(event->hw.branch_reg.reg & LBR_NO_INFO));
 		cpuc->lbr_context = event->ctx;
 	}
 	cpuc->br_sel = event->hw.branch_reg.reg;
@@ -349,6 +352,8 @@ void intel_pmu_lbr_enable(struct perf_event *event)
 					event->ctx->task_ctx_data) {
 		task_ctx = event->ctx->task_ctx_data;
 		task_ctx->lbr_callstack_users++;
+		if (!(cpuc->br_sel & LBR_NO_INFO))
+			task_ctx->need_info++;
 	}
 
 	cpuc->lbr_users++;
@@ -367,6 +372,8 @@ void intel_pmu_lbr_disable(struct perf_event *event)
 					event->ctx->task_ctx_data) {
 		task_ctx = event->ctx->task_ctx_data;
 		task_ctx->lbr_callstack_users--;
+		if (!(cpuc->br_sel & LBR_NO_INFO))
+			task_ctx->need_info--;
 	}
 
 	cpuc->lbr_users--;
Signature · 1 line
2.4.3
maintainer
#11 lore raw

Re: [PATCH 5/5] x86, perf: Avoid context switching LBR_INFO when not needed

arch/x86/kernel/cpu/perf_event_intel.c static void intel_pmu_cpu_starting(int cpu) · line 2847

Quoted 7 lines · Andi Kleen

+++ b/arch/x86/kernel/cpu/perf_event_intel.c

arch/x86/kernel/cpu/perf_event_intel.c +1 -1
@@ -2844,7 +2844,7 @@ static void intel_pmu_cpu_starting(int cpu)
 	/*
 	 * Deal with CPUs that don't clear their LBRs on power-up.
 	 */
-	intel_pmu_lbr_reset();
+	intel_pmu_lbr_reset(1);

s/1/true/ ?

Quoted 19 lines
arch/x86/kernel/cpu/perf_event_intel_lbr.c +2 -2
@@ -242,7 +245,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 
 	if (task_ctx->lbr_callstack_users == 0 ||
 	    task_ctx->lbr_stack_state == LBR_NONE) {
-		intel_pmu_lbr_reset();
+		intel_pmu_lbr_reset(task_ctx->need_info > 0);
 		return;
 	}
 
@@ -317,7 +320,7 @@ void intel_pmu_lbr_sched_task(struct perf_event_context *ctx, bool sched_in)
 	 * stack with branch from multiple tasks.
  	 */
 	if (sched_in) {
-		intel_pmu_lbr_reset();
+		intel_pmu_lbr_reset(!task_ctx || task_ctx->need_info > 0);
 		cpuc->lbr_context = ctx;
 	}
 }

arch/x86/kernel/cpu/perf_event_intel_lbr.c void intel_pmu_lbr_enable(struct perf_event *event) · line 349

Quoted 8 lines
arch/x86/kernel/cpu/perf_event_intel_lbr.c +1 -1
@@ -340,7 +343,7 @@ void intel_pmu_lbr_enable(struct perf_event *event)
 	 * avoid data leaks.
 	 */
 	if (event->ctx->task && cpuc->lbr_context != event->ctx) {
-		intel_pmu_lbr_reset();
+		intel_pmu_lbr_reset(!(event->hw.branch_reg.reg & LBR_NO_INFO));
 		cpuc->lbr_context = event->ctx;
 	}
 	cpuc->br_sel = event->hw.branch_reg.reg;

Are you sure none of that will result in some data leak in a weird corner case?

maintainer
#12 lore raw
In reply to Andi Kleen
Quoted 6 lines · Andi Kleen

From: Andi Kleen <[email protected]>

This fixes a bug added with the earlier 90405aa02. The bug could lead to lost LBR call stacks. When restoring the LBR state we need to use the TOS of the previous context, not the current context. To do that we need to save/restore the tos.

Current best practise also asks for:

Fixes 90405aa02247 ("perf/x86/intel/lbr: Limit LBR accesses to TOS in callstack mode")
Quoted 15 lines
arch/x86/kernel/cpu/perf_event_intel_lbr.c +1 -1
@@ -239,7 +239,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 	}
 
 	mask = x86_pmu.lbr_nr - 1;
-	tos = intel_pmu_lbr_tos();
+	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
 		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);

arch/x86/kernel/cpu/perf_event_intel_lbr.c static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx) · line 252

Quoted 6 lines
arch/x86/kernel/cpu/perf_event_intel_lbr.c +1 -0
@@ -247,6 +247,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
+	wrmsrl(x86_pmu.lbr_tos, tos);
 	task_ctx->lbr_stack_state = LBR_NONE;
 }

Any idea who much more expensive that wrmsr() is compared to the rdmsr() it replaces?

If its significant we could think about having this behaviour depend on callstacks.

author
#13 lore raw
Quoted 2 lines

Any idea who much more expensive that wrmsr() is compared to the rdmsr() it replaces?

I don't know.

Quoted 3 lines

If its significant we could think about having this behaviour depend on callstacks.

This function is only used for callstacks, otherwise it uses the LBR reset path.

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only.
#14 lore raw
In reply to Peter Zijlstra
Quoted 17 lines · Peter Zijlstra

arch/x86/kernel/cpu/perf_event_intel_lbr.c static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx) · line 252

Quoted 13 lines
arch/x86/kernel/cpu/perf_event_intel_lbr.c +2 -1
 	mask = x86_pmu.lbr_nr - 1;
-	tos = intel_pmu_lbr_tos();
+	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
 		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
@@ -247,6 +247,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
+	wrmsrl(x86_pmu.lbr_tos, tos);
 	task_ctx->lbr_stack_state = LBR_NONE;
 }

Any idea who much more expensive that wrmsr() is compared to the rdmsr() it replaces?

Quoted 2 lines

If its significant we could think about having this behaviour depend on callstacks.

The WRMSR extra cost is probably rather significant - here is a typical Intel WRMSR vs. RDMSR (non-hardwired) cache-hot/cache-cold cost difference:

[  170.798574] x86/bench: -------------------------------------------------------------------
[  170.807258] x86/bench: |                 RDTSC-cycles:    hot  (�noise) /   cold  (�noise)
[  170.816115] x86/bench: -------------------------------------------------------------------
[  212.146982] x86/bench: rdtsc                         :     16           /     60
[  213.725998] x86/bench: rdmsr                         :    100           /    148
[  215.469958] x86/bench: wrmsr                         :    456           /    708

That's on a Xeon E7-4890 (22nm IvyBridge-EX).

So it's 350-550 RDTSC cycles ...

Thanks,

Ingo

#15 lore raw

[tip:perf/core] perf/x86: Fix LBR call stack save/restore

In reply to Andi Kleen
perf/x86: Fix LBR call stack save/restore
Commit-ID:  b28ae9560b693bcd2e9f4d6d9c415d5380b7c3c5
Gitweb:     http://git.kernel.org/tip/b28ae9560b693bcd2e9f4d6d9c415d5380b7c3c5
Author:     Andi Kleen <[email protected]>
AuthorDate: Tue, 20 Oct 2015 11:46:33 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 23 Nov 2015 09:44:57 +0100

perf/x86: Fix LBR call stack save/restore

This fixes a bug I added in the following commit:

90405aa02247 ("perf/x86/intel/lbr: Limit LBR accesses to TOS in callstack mode")

The bug could lead to lost LBR call stacks. When restoring the LBR state we need to use the TOS of the previous context, not the current context. To do that we need to save/restore the TOS.

---
 arch/x86/kernel/cpu/perf_event.h           | 1 +
 arch/x86/kernel/cpu/perf_event_intel_lbr.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/perf_event.h +1 -0
index 499f533..ffa7a92 100644
@@ -627,6 +627,7 @@ struct x86_perf_task_context {
 	u64 lbr_from[MAX_LBR_ENTRIES];
 	u64 lbr_to[MAX_LBR_ENTRIES];
 	u64 lbr_info[MAX_LBR_ENTRIES];
+	int tos;
 	int lbr_callstack_users;
 	int lbr_stack_state;
 };
arch/x86/kernel/cpu/perf_event_intel_lbr.c +3 -1
index bfd0b71..659f01e 100644
@@ -239,7 +239,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 	}
 
 	mask = x86_pmu.lbr_nr - 1;
-	tos = intel_pmu_lbr_tos();
+	tos = task_ctx->tos;
 	for (i = 0; i < tos; i++) {
 		lbr_idx = (tos - i) & mask;
 		wrmsrl(x86_pmu.lbr_from + lbr_idx, task_ctx->lbr_from[i]);
@@ -247,6 +247,7 @@ static void __intel_pmu_lbr_restore(struct x86_perf_task_context *task_ctx)
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			wrmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
+	wrmsrl(x86_pmu.lbr_tos, tos);
 	task_ctx->lbr_stack_state = LBR_NONE;
 }
 
@@ -270,6 +271,7 @@ static void __intel_pmu_lbr_save(struct x86_perf_task_context *task_ctx)
 		if (x86_pmu.intel_cap.lbr_format == LBR_FORMAT_INFO)
 			rdmsrl(MSR_LBR_INFO_0 + lbr_idx, task_ctx->lbr_info[i]);
 	}
+	task_ctx->tos = tos;
 	task_ctx->lbr_stack_state = LBR_VALID;
 }