Messages in this thread | | | Subject | Re: [PATCH v3 4/7] MIPS: perf: Fix perf with MT counting other threads | From | Matt Redfearn <> | Date | Thu, 17 May 2018 11:35:17 +0100 |
| |
Hi James,
On 16/05/18 18:59, James Hogan wrote: > On Fri, Apr 20, 2018 at 11:23:06AM +0100, Matt Redfearn wrote: >> diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c >> index 7e2b7d38a774..fe50986e83c6 100644 >> --- a/arch/mips/kernel/perf_event_mipsxx.c >> +++ b/arch/mips/kernel/perf_event_mipsxx.c >> @@ -323,7 +323,11 @@ static int mipsxx_pmu_alloc_counter(struct cpu_hw_events *cpuc, >> >> static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx) >> { >> + struct perf_event *event = container_of(evt, struct perf_event, hw); >> struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events); >> +#ifdef CONFIG_MIPS_MT_SMP >> + unsigned int range = evt->event_base >> 24; >> +#endif /* CONFIG_MIPS_MT_SMP */ >> >> WARN_ON(idx < 0 || idx >= mipspmu.num_counters); >> >> @@ -331,11 +335,37 @@ static void mipsxx_pmu_enable_event(struct hw_perf_event *evt, int idx) >> (evt->config_base & M_PERFCTL_CONFIG_MASK) | >> /* Make sure interrupt enabled. */ >> MIPS_PERFCTRL_IE; >> - if (IS_ENABLED(CONFIG_CPU_BMIPS5000)) >> + >> +#ifdef CONFIG_CPU_BMIPS5000 >> + { >> /* enable the counter for the calling thread */ >> cpuc->saved_ctrl[idx] |= >> (1 << (12 + vpe_id())) | BRCM_PERFCTRL_TC; >> + } >> +#else >> +#ifdef CONFIG_MIPS_MT_SMP >> + if (range > V) { >> + /* The counter is processor wide. Set it up to count all TCs. */ >> + pr_debug("Enabling perf counter for all TCs\n"); >> + cpuc->saved_ctrl[idx] |= M_TC_EN_ALL; >> + } else >> +#endif /* CONFIG_MIPS_MT_SMP */ >> + { >> + unsigned int cpu, ctrl; >> >> + /* >> + * Set up the counter for a particular CPU when event->cpu is >> + * a valid CPU number. Otherwise set up the counter for the CPU >> + * scheduling this thread. >> + */ >> + cpu = (event->cpu >= 0) ? event->cpu : smp_processor_id(); >> + >> + ctrl = M_PERFCTL_VPEID(cpu_vpe_id(&cpu_data[cpu])); >> + ctrl |= M_TC_EN_VPE; >> + cpuc->saved_ctrl[idx] |= ctrl; >> + pr_debug("Enabling perf counter for CPU%d\n", cpu); >> + } >> +#endif /* CONFIG_CPU_BMIPS5000 */ > > I'm not a huge fan of the ifdefery tbh, I don't think it makes it very > easy to read having a combination of ifs and #ifdefs. I reckon > IF_ENABLED would be better, perhaps with having the BMIPS5000 case > return to avoid too much nesting.
OK, I'll try and tidy it up.
Thanks, Matt
> > Otherwise the patch looks okay to me. > > Thanks > James >
| |