All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <andi@firstfloor.org>,
	"Liang, Kan" <kan.liang@intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Stephane Eranian <eranian@google.com>,
	Wang Nan <wangnan0@huawei.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH] perf x86: Use PAGE_SIZE for PEBS buffer size on Core2
Date: Tue, 1 Mar 2016 20:03:52 +0100	[thread overview]
Message-ID: <20160301190352.GA8355@krava.redhat.com> (raw)
In-Reply-To: <20160301181207.GZ6356@twins.programming.kicks-ass.net>

On Tue, Mar 01, 2016 at 07:12:07PM +0100, Peter Zijlstra wrote:

SNIP

> > @@ -1327,6 +1328,7 @@ void __init intel_ds_init(void)
> >  		case 0:
> >  			pr_cont("PEBS fmt0%c, ", pebs_type);
> >  			x86_pmu.pebs_record_size = sizeof(struct pebs_record_core);
> 
> At the very least this wants a comment along the lines of:
> 
> 			/*
> 			 * Using >PAGE_SIZE buffers makes the WRMSR to
> 			 * PERF_GLOBAL_CTRL in intel_pmu_enable_all()
> 			 * mysteriously hang on Core2.
> 			 *
> 			 * As a workaround, we don't do this.
> 			 */
> 
> > +			x86_pmu.pebs_buffer_size = PAGE_SIZE;
> >  			x86_pmu.drain_pebs = intel_pmu_drain_pebs_core;
> >  			break;
> >  
> > diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> > index 7bb61e32fb29..1ab6279fed1d 100644
> > --- a/arch/x86/events/perf_event.h
> > +++ b/arch/x86/events/perf_event.h
> > @@ -586,6 +586,7 @@ struct x86_pmu {
> >  			pebs_broken	:1,
> >  			pebs_prec_dist	:1;
> >  	int		pebs_record_size;
> > +	int		pebs_buffer_size;
> >  	void		(*drain_pebs)(struct pt_regs *regs);
> >  	struct event_constraint *pebs_constraints;
> >  	void		(*pebs_aliases)(struct perf_event *event);

sending updated patch
jirka


---
Using PAGE_SIZE buffers makes the WRMSR to
PERF_GLOBAL_CTRL in intel_pmu_enable_all()
mysteriously hang on Core2. As a workaround,
we don't do this.

The hard lockup is easily triggered by running
'perf test attr' repeatedly. Most of the time
it gets stuck on sample session with small periods.

  # perf test attr -vv
  14: struct perf_event_attr setup                             :
  --- start ---
  ...
    'PERF_TEST_ATTR=/tmp/tmpuEKz3B /usr/bin/perf record -o /tmp/tmpuEKz3B/perf.data -c 123 kill >/dev/null 2>&1' ret 1

Reported-by: Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/events/intel/ds.c   | 13 +++++++++++--
 arch/x86/events/perf_event.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index c8a243d6fc82..b8420c364c5d 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -269,7 +269,7 @@ static int alloc_pebs_buffer(int cpu)
 	if (!x86_pmu.pebs)
 		return 0;
 
-	buffer = kzalloc_node(PEBS_BUFFER_SIZE, GFP_KERNEL, node);
+	buffer = kzalloc_node(x86_pmu.pebs_buffer_size, GFP_KERNEL, node);
 	if (unlikely(!buffer))
 		return -ENOMEM;
 
@@ -286,7 +286,7 @@ static int alloc_pebs_buffer(int cpu)
 		per_cpu(insn_buffer, cpu) = ibuffer;
 	}
 
-	max = PEBS_BUFFER_SIZE / x86_pmu.pebs_record_size;
+	max = x86_pmu.pebs_buffer_size / x86_pmu.pebs_record_size;
 
 	ds->pebs_buffer_base = (u64)(unsigned long)buffer;
 	ds->pebs_index = ds->pebs_buffer_base;
@@ -1319,6 +1319,7 @@ void __init intel_ds_init(void)
 
 	x86_pmu.bts  = boot_cpu_has(X86_FEATURE_BTS);
 	x86_pmu.pebs = boot_cpu_has(X86_FEATURE_PEBS);
+	x86_pmu.pebs_buffer_size = PEBS_BUFFER_SIZE;
 	if (x86_pmu.pebs) {
 		char pebs_type = x86_pmu.intel_cap.pebs_trap ?  '+' : '-';
 		int format = x86_pmu.intel_cap.pebs_format;
@@ -1327,6 +1328,14 @@ void __init intel_ds_init(void)
 		case 0:
 			pr_cont("PEBS fmt0%c, ", pebs_type);
 			x86_pmu.pebs_record_size = sizeof(struct pebs_record_core);
+			/*
+			* Using >PAGE_SIZE buffers makes the WRMSR to
+			* PERF_GLOBAL_CTRL in intel_pmu_enable_all()
+			* mysteriously hang on Core2.
+			*
+			* As a workaround, we don't do this.
+			*/
+			x86_pmu.pebs_buffer_size = PAGE_SIZE;
 			x86_pmu.drain_pebs = intel_pmu_drain_pebs_core;
 			break;
 
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 7bb61e32fb29..1ab6279fed1d 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -586,6 +586,7 @@ struct x86_pmu {
 			pebs_broken	:1,
 			pebs_prec_dist	:1;
 	int		pebs_record_size;
+	int		pebs_buffer_size;
 	void		(*drain_pebs)(struct pt_regs *regs);
 	struct event_constraint *pebs_constraints;
 	void		(*pebs_aliases)(struct perf_event *event);
-- 
2.4.3

  reply	other threads:[~2016-03-01 19:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-27 12:37 [BUG] Core2 cpu triggers hard lockup with perf test Jiri Olsa
2016-02-27 14:48 ` Peter Zijlstra
2016-02-27 15:46 ` Andi Kleen
2016-02-29 22:12 ` Liang, Kan
2016-03-01  6:55   ` Jiri Olsa
2016-03-01  9:17   ` Peter Zijlstra
2016-03-01 11:06     ` Jiri Olsa
2016-03-01 11:20       ` Peter Zijlstra
2016-03-01 14:51       ` Andi Kleen
2016-03-01 14:59         ` Peter Zijlstra
2016-03-01 17:17           ` Jiri Olsa
2016-03-01 17:32             ` Andi Kleen
2016-03-01 17:49             ` Peter Zijlstra
2016-03-01 18:04               ` Jiri Olsa
2016-03-01 18:14                 ` Peter Zijlstra
2016-03-01 18:12             ` Peter Zijlstra
2016-03-01 19:03               ` Jiri Olsa [this message]
2016-03-08 13:15                 ` [tip:perf/core] perf/x86/intel: Use PAGE_SIZE for PEBS buffer size on Core2 tip-bot for Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160301190352.GA8355@krava.redhat.com \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=eranian@google.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.