All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Stephane Eranian <eranian@google.com>
Cc: "Yan, Zheng" <zheng.z.yan@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Robert Richter <robert.richter@amd.com>,
	mingo@elte.hu
Subject: Re: [BUG] perf: sharing of cpuctx between core and ibs PMU causes problems
Date: Mon, 13 Aug 2012 12:28:02 +0200	[thread overview]
Message-ID: <1344853682.31459.20.camel@twins> (raw)
In-Reply-To: <CABPqkBRzyrmL_yiKnYWU4qaSZXbD=JJVnm5py4PUcgAYvT=hcA@mail.gmail.com>


OK,.. so the AMD IBS PMUs actually have perf_invalid_context.

Lemme have a proper look...


Weirdness.. perf_pmu_register() will allocate a pmu->pmu_cpu_context for
each PMU. find_pmu_context() even special cases the perf_invalid_context
to return NULL to force the allocation instead of sharing it.

So both IBS PMUs should have their own cpuctx.



In any case, I was talking about something like the below.. I hate
growing the per-task ctx array with two entries, esp. since we'll mostly
add two NULL pointer checks on every perf operation for everybody not
using IBS.

---
--- a/arch/x86/kernel/cpu/perf_event_amd_ibs.c
+++ b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
@@ -436,7 +436,7 @@ static void perf_ibs_read(struct perf_ev
 
 static struct perf_ibs perf_ibs_fetch = {
 	.pmu = {
-		.task_ctx_nr	= perf_invalid_context,
+		.task_ctx_nr	= perf_hw2_context,
 
 		.event_init	= perf_ibs_init,
 		.add		= perf_ibs_add,
@@ -459,7 +459,7 @@ static struct perf_ibs perf_ibs_fetch =
 
 static struct perf_ibs perf_ibs_op = {
 	.pmu = {
-		.task_ctx_nr	= perf_invalid_context,
+		.task_ctx_nr	= perf_hw3_context,
 
 		.event_init	= perf_ibs_init,
 		.add		= perf_ibs_add,
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1237,6 +1237,8 @@ enum perf_event_task_context {
 	perf_invalid_context = -1,
 	perf_hw_context = 0,
 	perf_sw_context,
+	perf_hw2_context, /* AMD IBS (fetch) */
+	perf_hw3_context, /* AMD IBS (ops)   */
 	perf_nr_task_contexts,
 };
 


  reply	other threads:[~2012-08-13 10:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-09  0:51 [BUG] perf: sharing of cpuctx between core and ibs PMU causes problems Stephane Eranian
2012-08-09  6:55 ` Yan, Zheng
2012-08-09 14:05   ` Stephane Eranian
2012-08-09 18:08     ` Peter Zijlstra
2012-08-09 19:21       ` Stephane Eranian
2012-08-13 10:28         ` Peter Zijlstra [this message]
2012-08-13 12:53           ` Stephane Eranian
2012-08-13 15:32             ` Peter Zijlstra
2012-08-13 15:36               ` Stephane Eranian

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=1344853682.31459.20.camel@twins \
    --to=peterz@infradead.org \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=robert.richter@amd.com \
    --cc=zheng.z.yan@intel.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.