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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
