linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jacob Shin <jacob.shin@amd.com>
To: Robert Richter <rric@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Stephane Eranian <eranian@google.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/4] perf, amd: Enable northbridge performance counters on AMD family 15h
Date: Wed, 28 Nov 2012 11:42:20 -0600	[thread overview]
Message-ID: <20121128174220.GD4581@jshin-Toonie> (raw)
In-Reply-To: <20121116193224.GS2504@rric.localhost>

Robert,

On Fri, Nov 16, 2012 at 08:32:24PM +0100, Robert Richter wrote:
> On 16.11.12 13:00:30, Jacob Shin wrote:
> > On Fri, Nov 16, 2012 at 07:43:44PM +0100, Robert Richter wrote:
> > > On 15.11.12 15:31:53, Jacob Shin wrote:
> > > > @@ -323,6 +368,16 @@ __amd_get_nb_event_constraints(struct cpu_hw_events *cpuc, struct perf_event *ev
> > > >  	if (new == -1)
> > > >  		return &emptyconstraint;
> > > >  
> > > > +	/* set up interrupts to be delivered only to this core */
> > > > +	if (cpu_has_perfctr_nb) {
> > > > +		struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
> > > > +
> > > > +		hwc->config |= AMD_PERFMON_EVENTSEL_INT_CORE_ENABLE;
> > > > +		hwc->config &= ~AMD_PERFMON_EVENTSEL_INT_CORE_SEL_MASK;
> > > > +		hwc->config |= (0ULL | (c->cpu_core_id)) <<
> > > > +			AMD_PERFMON_EVENTSEL_INT_CORE_SEL_SHIFT;
> > > > +	}
> > > 
> > > Looks like a hack to me. The constaints handler is only supposed to
> > > determine constraints and not to touch anything in the event's
> > > structure. This should be done later when setting up hwc->config in
> > > amd_nb_event_config() or so.
> > 
> > Hm.. is the hwc->config called after constraints have been set up
> > already? If so, I'll change it ..
> 
> Should be, since the hw register can be setup only after the counter
> is selected.

Ahh .. looking at this further, it looks like ->config is called
before constraints are set up (before we know what cpu we are going to
run on).

Sorry for not seeing this sooner, but it really looks like the event
constraints function is the right time to set up the INT_CORE_SEL bits
. Are you okay with this?

> > > I also do not think that smp_processor_id() is the right thing to do
> > > here. Since cpu_hw_events is per-cpu the cpu is already selected.
> > 
> > Yeah, I could not figure out how to get the cpu number from cpuc. Is
> > there a container_of kind of thing that I can do to get the cpu number
> > ?
> 
> At some point event->cpu is assigned, I think.

Furthermore, event->cpu can only be used if --cpu flag is specified
from userlan, otherwise event->cpu is 0xffff. And we do not know until
the schedule happens, what cpu we are going to be running on.

I tried to figure out if there was a way to get from cpu_hw_events to
a cpu number, but I didn't see any obvious ways. The cpu_hw_events is
derived from __get_cpu_var from the schedule function that calls the
constraints, so smp_processor_id seems okay to use here.

..

So I'll have to change things back, unless do you have any other
ideas ?

Thanks,

-Jacob

> 
> -Robert
> 


  parent reply	other threads:[~2012-11-28 17:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-15 21:31 [PATCH V2 0/4] perf, amd: Enable AMD family 15h northbridge counters Jacob Shin
2012-11-15 21:31 ` [PATCH 1/4] perf, amd: Rework northbridge event constraints handler Jacob Shin
2012-11-15 21:31 ` [PATCH 2/4] perf, amd: Generalize northbridge constraints code for family 15h Jacob Shin
2012-11-15 21:31 ` [PATCH 3/4] perf, x86: Move MSR address offset calculation to architecture specific files Jacob Shin
2012-11-15 21:31 ` [PATCH 4/4] perf, amd: Enable northbridge performance counters on AMD family 15h Jacob Shin
2012-11-16 18:43   ` Robert Richter
2012-11-16 19:00     ` Jacob Shin
2012-11-16 19:32       ` Robert Richter
2012-11-16 21:57         ` Jacob Shin
2012-11-26 12:15           ` Robert Richter
2012-11-28 17:42         ` Jacob Shin [this message]
2012-11-18 16:35       ` Robert Richter
2012-11-15 21:40 ` [PATCH V2 0/4] perf, amd: Enable AMD family 15h northbridge counters Jacob Shin
  -- strict thread matches above, loose matches on Subject: below --
2012-11-10  1:01 [PATCH " Jacob Shin
2012-11-10  1:01 ` [PATCH 4/4] perf, amd: Enable northbridge performance counters on AMD family 15h Jacob Shin

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=20121128174220.GD4581@jshin-Toonie \
    --to=jacob.shin@amd.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=eranian@google.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=rric@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).