On Fri, May 15, 2020 at 10:09:10AM +0100, John Garry wrote: > On 15/05/2020 00:02, Ian Rogers wrote: > > On Thu, May 14, 2020 at 2:00 AM John Garry <john.ga...@huawei.com> wrote: > > > > > > On 13/05/2020 17:10, Ian Rogers wrote: > > > > > Out of interest, if we could move the validation of metrics to > > > > > jevents, > > > > > how much functionality would we still have here? > > > > If we add checking to jevents then the MetricExpr would be known to be > > > > valid, however, the events (aka ids) within the expression could be > > > > invalid. > > > > > > So I think that has some value. I mean, just to detect syntax errors, > > > like those remedied in "perf metrics: fix parse errors in power8 metrics". > > > > > > > I'm not sure we could realistically check the events at > > > > jevents (build) time as there is no guarantee that the machine we run > > > > on is the same as the one we compile on. > > > > > > But we could at least check that there are event aliases for that CPU, > > > right? (by examining the JSONs for that cpu). If the event alias does > > > not actually match on the target CPU, then that can't be helped. > > > > Agreed, I think there will be some cases where something more can be > > done. Jiri has proposed fake pmus as well: > > https://www.spinics.net/lists/linux-perf-users/msg11760.html > > I don't know how much sense it makes trying to get this in jevents, as > > long as 'perf test' is run. > > At a glance, that does not look like something we would want in jevents. But > rather the metric expr parsing error detection and alias checking. > > About jirka's patch: > > --- a/tools/perf/tests/pmu-events.c > +++ b/tools/perf/tests/pmu-events.c > @@ -485,6 +485,102 @@ static int test_parsing(void) > return ret == 0 ? TEST_OK : TEST_SKIP; > } > > + > +static struct test_metric metrics[] = { > + { .metric = "imx8_ddr0@read\\-cycles@ * 4 * 4", }, > + { .metric = > "imx8_ddr0@axid\\-read\\,axi_mask\\=0xffff\\,axi_id\\=0x0000@ > * 4", }, > + { .metric = "(cstate_pkg@c2\\-residency@ / msr@tsc@) * 100", }, > + { .metric = "(imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@)", }, > +}; > > Maybe we could add these to pmu-events/arch/test/test_cpu/metric.json, and > get at them that way.
that test sets the 'fake pmu' stuff.. could we do that in your test? jirka
