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

[Jiri Olsa]

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