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

[John Garry <john.garry@huawei.com>]

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

Thanks,
John