Re: [PATCH V8 08/10] powerpc, perf: Change POWER8 PMU configuration to work with SW filters

[Daniel Axtens <dja@axtens.net>]

[-- Attachment #1: Type: text/plain, Size: 2281 bytes --]

On Wed, 2015-06-10 at 17:40 +0530, Anshuman Khandual wrote:
> On 06/10/2015 11:19 AM, Daniel Axtens wrote:
> > On Mon, 2015-06-08 at 17:08 +0530, Anshuman Khandual wrote:
> >> > The kernel now supports SW based branch filters for book3s systems with
> >> > some specific requirements while dealing with HW supported branch filters
> >> > in order to achieve overall OR semantics prevailing in perf branch stack
> >> > sampling framework. This patch adapts the BHRB branch filter configuration
> >> > to meet those protocols. POWER8 PMU can only handle one HW based branch
> >> > filter request at any point of time. For all other combinations PMU will
> >> > pass it on to the SW.
> >> > 
> >> > Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> >> > ---
> >> >  arch/powerpc/perf/power8-pmu.c | 51 ++++++++++++++++++++++++++++++++++++------
> >> >  1 file changed, 44 insertions(+), 7 deletions(-)
> >> > 
> >> > diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> >> > index 5e17cb5..8fccf6c 100644
> >> > --- a/arch/powerpc/perf/power8-pmu.c
> >> > +++ b/arch/powerpc/perf/power8-pmu.c
> >> > @@ -656,6 +656,16 @@ static int power8_generic_events[] = {
> >> >  
> > This is, I think, the third time you've modified this function in this
> > patch series. I appreciate the fact that you're trying to keep logical
> > changes separate, but it seems to me like this change might be able to
> > be combined with patch 4, and given a single commit message that clearly
> > explains the complete scope of the changes.
> 
> Here I have to disagree with you. The changes in this patch like PMU
> should not handle multiple filter requests as it does not support the
> OR semantic required in the protocol, the fact that we need to pass
> on the entire branch filtering responsibility to the SW comes into
> picture after we have enabled the SW branch filtering support in the
> previous patch. So these changes have to follow that up logically and
> sequentially in that order.
> 
OK. I don't think I understand the patch set quite well enough to follow
your logic, but when you send out the next version I'll try to take a
closer look at how the series fits together as a whole.

-- 
Regards,
Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 860 bytes --]