Message-ID: <20200313103259.GC389625@krava>
Date:   Fri, 13 Mar 2020 11:32:59 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     John Garry <john.garry@...wei.com>
Cc:     peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
        mark.rutland@....com, alexander.shishkin@...ux.intel.com,
        namhyung@...nel.org, will@...nel.org, ak@...ux.intel.com,
        linuxarm@...wei.com, linux-kernel@...r.kernel.org,
        james.clark@....com, qiangqing.zhang@....com
Subject: Re: [PATCH 6/6] perf test: Add pmu-events test

On Mon, Mar 09, 2020 at 04:20:52PM +0000, John Garry wrote:
> > > 
> > > The events in test_cpu_aliases[] or test_uncore_aliases[] are checked
> > > against the events from pmu-events/arch/test/test_cpu/*.json
> > 
> 
> Hi Jirka,
> 
> > 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 ...
> 
> Right. And the idea is that pme_test_cpu[] can be used as generic set of
> events for testing on any arch/cpuid. (note: I'll just ignore uncore events
> for the moment)
> 
> > 
> >    - 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
> 
> No, we use pme_test_cpu[] to generate the event aliases for a PMU, and
> verify that the aliases are as expected.
> 
> > 
> >    - you go through all detected pmus and check if the tests events we
> >      generated are matching some of the events from these pmus,
> 
> Not exactly.
> 
> >      and that's where I'm lost ;-) why?
> 
> So consider the "cpu" HW PMU. During normal operation, we create the event
> aliases for this PMU in pmu_lookup()->pmu_add_cpu_aliases(). This step looks
> up a map of cpu events for that CPUID, and then creates the event aliases
> for that PMU from that map.
> 
> I want the test to recreate this and verify that the events from the test
> JSONs will have event aliases created properly.

aah ok, my first objective was to have some way to test pmu-events
changes we plan to do and their affect to generated pmu-event.c

you want to test the code paths after that.. perfect

> 
> So in the test when we scan the PMUs and find "cpu" HW PMU, we create a test
> PMU with the same name, create the event aliases from pme_test_cpu[] for
> that test PMU, and then verify that the event aliases created are as
> expected. Then the test PMU is deleted.
> 
> So overall the test covers:
> a. jevents code to generate the struct pmu_event []
> b. util/pmu.c code to create the event aliases for a given PMU
> 
> Note: the test does not (yet) cover matching of events declared in the HW
> PMU sysfs folder. I'm talking about these, for example:

ok

> 
> $ ls /sys/bus/event_source/devices/cpu/events/
> branch-instructions  cache-references  el-abort     el-start ref-cycles
> ...
> 
> > 
> > > 
> > > > 
> > > > 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
> 
> Let's just concentrate on core PMU ATM :)

ok, thanks,
jirka

Powered by blists - more mailing lists