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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6a7f8e76-981e-0a84-0043-262c0e2d00d1@huawei.com>
Date:   Fri, 15 May 2020 15:36:44 +0100
From:   John Garry <john.garry@...wei.com>
To:     Jiri Olsa <jolsa@...hat.com>
CC:     Ian Rogers <irogers@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Leo Yan <leo.yan@...aro.org>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Paul Clarke <pc@...ibm.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 2/2] perf test: Improve pmu event metric testing

On 15/05/2020 12:48, Jiri Olsa wrote:
> 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.garry@...wei.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@...d\\-cycles@ * 4 * 4", },
>> +	{ .metric = "imx8_ddr0@...d\\-read\\,axi_mask\\=0xffff\\,axi_id\\=0x0000@
>> * 4", },
>> +	{ .metric = "(cstate_pkg@c2\\-residency@ / msr@tsc@) * 100", },
>> +	{ .metric = "(imx8_ddr0@...d\\-cycles@ + imx8_ddr0@...te\\-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?

I'm not sure about a test comparable to the alias match testing, but at 
least it should be possible to verify that jevents generates as-expected 
metric events (as done in __test_pmu_event_table() for regular events).

Cheers,
John