Messages in this thread | | | Date | Wed, 28 Nov 2012 11:42:20 -0600 | From | Jacob Shin <> | Subject | Re: [PATCH 4/4] perf, amd: Enable northbridge performance counters on AMD family 15h |
| |
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 >
| |