Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1251955 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/5] x86, perf: Fix LBR call stack save/restore Andi Kleen <andi@firstfloor.org> - 2015-10-20 20:50 +0200
    [PATCH 5/5] x86, perf: Avoid context switching LBR_INFO when not needed Andi Kleen <andi@firstfloor.org> - 2015-10-20 20:50 +0200
      Re: [PATCH 5/5] x86, perf: Avoid context switching LBR_INFO when not  needed Peter Zijlstra <peterz@infradead.org> - 2015-10-21 15:50 +0200
    [PATCH 3/5] perf, tools: Disable branch flags/cycles for --callgraph lbr Andi Kleen <andi@firstfloor.org> - 2015-10-20 21:00 +0200
      Re: [PATCH 3/5] perf, tools: Disable branch flags/cycles for  --callgraph lbr Peter Zijlstra <peterz@infradead.org> - 2015-10-21 15:30 +0200
        Re: [PATCH 3/5] perf, tools: Disable branch flags/cycles for  --callgraph lbr Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-10-21 20:10 +0200
    Re: [PATCH 1/5] x86, perf: Fix LBR call stack save/restore Peter Zijlstra <peterz@infradead.org> - 2015-10-21 15:20 +0200
      Re: [PATCH 1/5] x86, perf: Fix LBR call stack save/restore Andi Kleen <andi@firstfloor.org> - 2015-10-21 16:40 +0200
      Re: [PATCH 1/5] x86, perf: Fix LBR call stack save/restore Ingo Molnar <mingo@kernel.org> - 2015-10-21 18:30 +0200

#1251955 — [PATCH 1/5] x86, perf: Fix LBR call stack save/restore

From: Andi Kleen <ak@linux.intel.com>

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.

Cc: <stable@vger.kernel.org> # 4.2+
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index d871c94..1b47164 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -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;
 };
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index ad0b8b0..0e4ea00 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -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;
 }
 
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

From: Andi Kleen <ak@linux.intel.com>

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.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index 1b47164..4ae66e3 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -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);
 
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index f17772a..42f21f0 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -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;
 
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index 60e71b7..7c21efb 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -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--;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Tue, Oct 20, 2015 at 11:46:37AM -0700, Andi Kleen wrote:
> +++ b/arch/x86/kernel/cpu/perf_event_intel.c
> @@ -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/ ?


> @@ -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;
>  	}
>  }
> @@ -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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

From: Andi Kleen <ak@linux.intel.com>

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.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/evsel.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 8be867c..e8724b4 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -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:
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Tue, Oct 20, 2015 at 11:46:35AM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> 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.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---

Arnaldo, ACK?

>  tools/perf/util/evsel.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 8be867c..e8724b4 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -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:
> -- 
> 2.4.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253098 — 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:
> On Tue, Oct 20, 2015 at 11:46:35AM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > 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.
> > 
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > ---
> 
> Arnaldo, ACK?

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

Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
 
> >  tools/perf/util/evsel.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 8be867c..e8724b4 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -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:
> > -- 
> > 2.4.3
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252803

On Tue, Oct 20, 2015 at 11:46:33AM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> 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")
> Cc: <stable@vger.kernel.org> # 4.2+
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -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;
>  }

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252874

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

I don't know.
> 
> 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


-- 
ak@linux.intel.com -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1253030

* Peter Zijlstra <peterz@infradead.org> wrote:

> >  	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?
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web