Re: [PATCH 2/2] perf test: Improve pmu event metric testing

From: John Garry
Date: Fri May 15 2020 - 05:10:11 EST


On 15/05/2020 00:02, Ian Rogers wrote:
On Thu, May 14, 2020 at 2:00 AM John Garry <john.garry@xxxxxxxxxx> 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.

Thanks,
John