Re: [perf metricgroup] fcc9c5243c: perf-sanity-tests.Parse_and_process_metrics.fail

[John Garry]

On 03/11/2020 16:05, Ian Rogers wrote:
> On Tue, Nov 3, 2020 at 6:43 AM John Garry<john.garry@xxxxxxxxxx>  wrote:
> > On 20/10/2020 17:53, Ian Rogers wrote:
> > > > > Thanks for taking a look John. If you want help you can send the
> > > > > output of "perf test 67 -vvv" to me. It is possible Broadwell has
> > > > > similar glitches in the json to Skylake. I tested the original test on
> > > > > server parts as I can access them as cloud machines.
> > > > >
> > > > > > I will have a look, but I was hoping that Ian would have a proper fix
> > > > > > for this on top of ("perf metricgroup: Fix uncore metric expressions"),
> > > > > > which now looks to be merged.
> > > > > I still have these changes to look at in my inbox but I'm assuming
> > > > > they're good:-)  Sorry for not getting to them, but it's good they are
> > > > > merged.
> > > > Hi Ian,
> > > >      Checked in upstream kernel with your fix patch, in powerpc also test case 67 is passing.
> > > > But I am getting issue in test 10 for powerpc
> > > >
> > > > [command]# ./perf test 10
> > > > 10: PMU events                                                      :
> > > > 10.1: PMU event table sanity                                        : Ok
> > > > 10.2: PMU event map aliases                                         : Ok
> > > > 10.3: Parsing of PMU event table metrics                            : Skip (some metrics failed)
> > > > 10.4: Parsing of PMU event table metrics with fake PMUs             : FAILED!
> > > >
> > > > Was debugging it, issue is with commit e1c92a7fbbc5 perf tests: Add another metric parsing test.
> > > >
> > > > So, there we are passing different runtime parameter value in "expr__find_other and expr__parse"
> > > > in function `metric_parse_fake`. I believe we need to send same value.
> > > > I will send fix patch for the same.
> > Just wondering, was a patch ever submitted for this? Something still
> > broken? I can't see any recent relevant changes to tests/pmu-events.c
> The test itself shouldn't have changed, but the json files parsed by
> jevents and turned into C code that the test exercises should have
> changed. Jin Yao has sent two patch sets fixing a metric issue on SKL
> (Skylake non-server) that should hopefully fix the issue there - I'll
> check the status on these. Are you testing on Skylake?

So I have re-read this thread, and it seems that 2x different things are 
being discussed:
a. some breakage for test #10 on skylake
b. test #67 being broken

It seems that a. has been addressed. That's what I was asking about just 
now.

So about b., which I thought may be broken for some other reason apart 
from my hacky patch. But it seems not the case, and a proper patch is 
needed there.

Ian, have you had a chance to consider this issue in b.? That is, we 
have breakage for metrics using uncore alias expressions for when 
multiple uncore PMUs associated exist in the system? As before, looks 
broken by ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap")

Thanks,
John