[PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts

[Michael Ellerman]

On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
> On 10/08/2013 09:51 AM, Michael Ellerman wrote:
> > On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
> >> Right now the `config_bhrb` PMU specific call happens after write_mmcr0
> >> which actually enables the PMU for event counting and interrupt. So
> >> there is a small window of time where the PMU and BHRB runs without the
> >> required HW branch filter (if any) enabled in BHRB. This can cause some
> >> of the branch samples to be collected through BHRB without any filter
> >> being applied and hence affecting the correctness of the results. This
> >> patch moves the BHRB config function call before enabling the interrupts.
> > 
> > Patch looks good.
> > 
> > But it reminds me I have an item in my TODO list:
> >  - "Why can't config_bhrb() be done in compute_mmcr()" ?
> > 
> 
> compute_mmcr() function deals with generic MMCR* configs for normal PMU
> events. Even if BHRB config touches MMCRA register, it's configuration
> does not interfere with the PMU config for general events. So its best
> to keep them separate. 

I'm unconvinced. If they'd been together to begin with this bug never
would have happened.

And there's the added overhead of another indirect function call.

> Besides, we can always look at these code consolidation
> issues in future. 

The future is now.

> But this patch solves a problem which is happening right now.

Sure, I'm not saying we shouldn't merge it as a fix. But I think we
should do the cleanup to move it into compute_mmcr() for 3.13.

cheers