Re: [PATCH 6/6] perf test: Add pmu-events test

From: Jiri Olsa
Date: Mon Mar 09 2020 - 11:26:49 EST


On Mon, Mar 09, 2020 at 10:12:15AM +0000, John Garry wrote:
> > >
> > > A sample run is as follows for x86:
> > >
> > > Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> > > 10: PMU event aliases :
> > > --- start ---
> > > test child forked, pid 30869
> > > Using CPUID GenuineIntel-6-9E-9
> > > intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
> > > skipping testing PMU software
> > > testing PMU power: skip
> > > testing PMU cpu: matched event segment_reg_loads.any
> > > testing PMU cpu: matched event dispatch_blocked.any
> > > testing PMU cpu: matched event eist_trans
> > > testing PMU cpu: matched event bp_l1_btb_correct
> > > testing PMU cpu: matched event bp_l2_btb_correct
> > > testing PMU cpu: pass
> > > testing PMU cstate_core: skip
> > > testing PMU uncore_cbox_2: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_2: pass
> > > skipping testing PMU breakpoint
> > > testing PMU uncore_cbox_0: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_0: pass
> > > skipping testing PMU tracepoint
> > > testing PMU cstate_pkg: skip
> > > testing PMU uncore_arb: skip
> > > testing PMU msr: skip
> > > testing PMU uncore_cbox_3: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_3: pass
> > > testing PMU intel_pt: skip
> > > testing PMU uncore_cbox_1: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_1: pass
> > > test child finished with 0
> > > ---- end ----
> > > PMU event aliases: Ok
> >
> > SNIP
> >
> > > +int test__pmu_event_aliases(struct test *test __maybe_unused,
> > > + int subtest __maybe_unused)
> > > +{
> > > + struct perf_pmu *pmu = NULL;
> > > +
> > > + while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> > > + int count = 0;
> >
>
> Hi jirka,
>
> > I don't follow the pmu iteration in here.. I'd expect
> > we create 'test' pmu
>
> well, that's what we do next in the call to __test__pmu_event_aliases():
>
> static int __test__pmu_event_aliases(char *pmu_name, int *count)
> {
> struct perf_pmu_alias *alias;
> struct perf_pmu *pmu;
> LIST_HEAD(aliases);
> int res = 0;
>
> pmu = zalloc(sizeof(*pmu));
> if (!pmu)
> return -1;
>
> pmu->name = pmu_name;
>
> pmu_add_cpu_aliases_map(&aliases, pmu, &pmu_events_map_test);
>
>
> Here we clone the HW PMU, create the aliases, and verify them against a
> known, expected list.
>
> and check that all the aliasses
> > are in place as we expect them.. why do we match them
> > to existing events?
>
> The events in test_cpu_aliases[] or test_uncore_aliases[] are checked
> against the events from pmu-events/arch/test/test_cpu/*.json

I don't understand the benefit of this.. so IIUC:

- jevents will go through arch/test and populate pmu-events/pmu-events.c
with:
struct pmu_event pme_test_cpu[] ...
struct pmu_events_map pmu_events_map_test ...

- so we actualy have the parsed json events in C structs and we can go
through them and check it contains fields with strings that we expect

- you go through all detected pmus and check if the tests events we
generated are matching some of the events from these pmus,
and that's where I'm lost ;-) why?

>
> >
> > or as I'm thinking about that now, would it be enough
> > to check pme_test_cpu array to have string that we
> > expect?
>
> Right, I might change this.
>
> So currently we iterate the PMU aliases to ensure that we have a matching
> event in pme_test_cpu[]. It may be better to iterate the events in
> pme_test_cpu[] to ensure that we have an alias.

that's what I described above.. I dont understand the connection/value
of this tests

>
> The problem here is uncore PMUs. They have the "Unit" field, which is used
> for matching the PMU. So we cannot ensure test events from uncore.json will
> always have an event alias created per PMU. But maybe I could use
> pmu_uncore_alias_match() to check if the test event matches in this case.

hum I guess I don't follow all the details.. but some more explanation
of the test would be great

jirka