Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 16 Apr 2024 22:42:46 -0700 | Subject | Re: [RFC PATCH v5 03/16] perf pmu-events: Add functions in jevent.py to parse counter and event info for hardware aware grouping |
| |
On Fri, Apr 12, 2024 at 2:08 PM <weilin.wang@intel.com> wrote: > > From: Weilin Wang <weilin.wang@intel.com> > > These functions are added to parse event counter restrictions and counter > availability info from json files so that the metric grouping method could > do grouping based on the counter restriction of events and the counters > that are available on the system. > > Signed-off-by: Weilin Wang <weilin.wang@intel.com> > --- > tools/perf/pmu-events/jevents.py | 204 +++++++++++++++++++++++++++-- > tools/perf/pmu-events/pmu-events.h | 32 ++++- > 2 files changed, 224 insertions(+), 12 deletions(-) > > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py > index e42efc16723e..7cfd86d77fea 100755 > --- a/tools/perf/pmu-events/jevents.py > +++ b/tools/perf/pmu-events/jevents.py > @@ -23,6 +23,8 @@ _metric_tables = [] > _sys_metric_tables = [] > # Mapping between sys event table names and sys metric table names. > _sys_event_table_to_metric_table_mapping = {} > +# List of regular PMU counter layout tables. > +_pmu_layouts_tables = [] > # Map from an event name to an architecture standard > # JsonEvent. Architecture standard events are in json files in the top > # f'{_args.starting_dir}/{_args.arch}' directory. > @@ -31,6 +33,10 @@ _arch_std_events = {} > _pending_events = [] > # Name of events table to be written out > _pending_events_tblname = None > +# PMU counter layout to write out when the layout table is closed > +_pending_pmu_counts = [] > +# Name of PMU counter layout table to be written out > +_pending_pmu_counts_tblname = None > # Metrics to write out when the table is closed > _pending_metrics = [] > # Name of metrics table to be written out > @@ -51,6 +57,11 @@ _json_event_attributes = [ > 'long_desc' > ] > > +# Attributes that are in pmu_unit_layout. > +_json_layout_attributes = [ > + 'pmu', 'desc' > +] > + > # Attributes that are in pmu_metric rather than pmu_event. > _json_metric_attributes = [ > 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold', > @@ -265,7 +276,7 @@ class JsonEvent: > > def unit_to_pmu(unit: str) -> Optional[str]: > """Convert a JSON Unit to Linux PMU name.""" > - if not unit: > + if not unit or unit == "core": > return 'default_core' > # Comment brought over from jevents.c: > # it's not realistic to keep adding these, we need something more scalable ... > @@ -334,6 +345,19 @@ class JsonEvent: > if 'Errata' in jd: > extra_desc += ' Spec update: ' + jd['Errata'] > self.pmu = unit_to_pmu(jd.get('Unit')) > + # The list of counter(s) the event could be collected with > + class Counter: > + gp = str() > + fixed = str() > + self.counters = {'list': str(), 'num': Counter()} > + self.counters['list'] = jd.get('Counter') > + # Number of generic counter > + self.counters['num'].gp = jd.get('CountersNumGeneric') > + # Number of fixed counter > + self.counters['num'].fixed = jd.get('CountersNumFixed') > + # If the event uses an MSR, other event uses the same MSR could not be > + # schedule to collect at the same time. > + self.msr = jd.get('MSRIndex') > filter = jd.get('Filter') > self.unit = jd.get('ScaleUnit') > self.perpkg = jd.get('PerPkg') > @@ -409,8 +433,20 @@ class JsonEvent: > s += f'\t{attr} = {value},\n' > return s + '}' > > - def build_c_string(self, metric: bool) -> str: > + def build_c_string(self, metric: bool, layout: bool) -> str: > s = '' > + if layout: > + for attr in _json_layout_attributes: > + x = getattr(self, attr) > + if attr in _json_enum_attributes: > + s += x if x else '0' > + else: > + s += f'{x}\\000' if x else '\\000' > + x = self.counters['num'].gp > + s += x if x else '0' > + x = self.counters['num'].fixed > + s += x if x else '0' > + return s > for attr in _json_metric_attributes if metric else _json_event_attributes: > x = getattr(self, attr) > if metric and x and attr == 'metric_expr': > @@ -423,12 +459,15 @@ class JsonEvent: > s += x if x else '0' > else: > s += f'{x}\\000' if x else '\\000' > + if not metric: > + x = self.counters['list'] > + s += f'{x}\\000' if x else '\\000' > return s > > - def to_c_string(self, metric: bool) -> str: > + def to_c_string(self, metric: bool, layout: bool) -> str: > """Representation of the event as a C struct initializer.""" > > - s = self.build_c_string(metric) > + s = self.build_c_string(metric, layout) > return f'{{ { _bcs.offsets[s] } }}, /* {s} */\n' > > > @@ -465,6 +504,8 @@ def preprocess_arch_std_files(archpath: str) -> None: > _arch_std_events[event.name.lower()] = event > if event.metric_name: > _arch_std_events[event.metric_name.lower()] = event > + if event.counters['num'].gp: > + _arch_std_events[event.pmu.lower()] = event > > > def add_events_table_entries(item: os.DirEntry, topic: str) -> None: > @@ -474,6 +515,8 @@ def add_events_table_entries(item: os.DirEntry, topic: str) -> None: > _pending_events.append(e) > if e.metric_name: > _pending_metrics.append(e) > + if e.counters['num'].gp: > + _pending_pmu_counts.append(e) > > > def print_pending_events() -> None: > @@ -514,7 +557,7 @@ def print_pending_events() -> None: > last_pmu = event.pmu > pmus.add((event.pmu, pmu_name)) > > - _args.output_file.write(event.to_c_string(metric=False)) > + _args.output_file.write(event.to_c_string(metric=False, layout=False)) > _pending_events = [] > > _args.output_file.write(f""" > @@ -569,7 +612,7 @@ def print_pending_metrics() -> None: > last_pmu = metric.pmu > pmus.add((metric.pmu, pmu_name)) > > - _args.output_file.write(metric.to_c_string(metric=True)) > + _args.output_file.write(metric.to_c_string(metric=True, layout=False)) > _pending_metrics = [] > > _args.output_file.write(f""" > @@ -587,6 +630,35 @@ const struct pmu_table_entry {_pending_metrics_tblname}[] = {{ > """) > _args.output_file.write('};\n\n') > > +def print_pending_pmu_counter_layout_table() -> None: > + '''Print counter layout data from counter.json file to counter layout table in > + c-string''' > + > + def pmu_counts_cmp_key(j: JsonEvent) -> Tuple[bool, str, str]: > + def fix_none(s: Optional[str]) -> str: > + if s is None: > + return '' > + return s > + > + return (j.desc is not None, fix_none(j.pmu)) > + > + global _pending_pmu_counts > + if not _pending_pmu_counts: > + return > + > + global _pending_pmu_counts_tblname > + global pmu_layouts_tables > + _pmu_layouts_tables.append(_pending_pmu_counts_tblname) > + > + _args.output_file.write( > + f'static const struct compact_pmu_event {_pending_pmu_counts_tblname}[] = {{\n') > + > + for pmu_layout in sorted(_pending_pmu_counts, key=pmu_counts_cmp_key): > + _args.output_file.write(pmu_layout.to_c_string(metric=False, layout=True)) > + _pending_pmu_counts = [] > + > + _args.output_file.write('};\n\n') > + > def get_topic(topic: str) -> str: > if topic.endswith('metrics.json'): > return 'metrics' > @@ -623,10 +695,12 @@ def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None: > pmu_name = f"{event.pmu}\\000" > if event.name: > _bcs.add(pmu_name, metric=False) > - _bcs.add(event.build_c_string(metric=False), metric=False) > + _bcs.add(event.build_c_string(metric=False, layout=False), metric=False) > if event.metric_name: > _bcs.add(pmu_name, metric=True) > - _bcs.add(event.build_c_string(metric=True), metric=True) > + _bcs.add(event.build_c_string(metric=True, layout=False), metric=True) > + if event.counters['num'].gp: > + _bcs.add(event.build_c_string(metric=False, layout=True), metric=False) > > def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None: > """Process a JSON file during the main walk.""" > @@ -640,11 +714,14 @@ def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None: > if item.is_dir() and is_leaf_dir(item.path): > print_pending_events() > print_pending_metrics() > + print_pending_pmu_counter_layout_table() > > global _pending_events_tblname > _pending_events_tblname = file_name_to_table_name('pmu_events_', parents, item.name) > global _pending_metrics_tblname > _pending_metrics_tblname = file_name_to_table_name('pmu_metrics_', parents, item.name) > + global _pending_pmu_counts_tblname > + _pending_pmu_counts_tblname = file_name_to_table_name('pmu_layouts_', parents, item.name) > > if item.name == 'sys': > _sys_event_table_to_metric_table_mapping[_pending_events_tblname] = _pending_metrics_tblname > @@ -678,6 +755,12 @@ struct pmu_metrics_table { > uint32_t num_pmus; > }; > > +/* Struct used to make the PMU counter layout table implementation opaque to callers. */ > +struct pmu_layouts_table { > + const struct compact_pmu_event *entries; > + size_t length; > +}; > + > /* > * Map a CPU to its table of PMU events. The CPU is identified by the > * cpuid field, which is an arch-specific identifier for the CPU. > @@ -691,6 +774,7 @@ struct pmu_events_map { > const char *cpuid; > struct pmu_events_table event_table; > struct pmu_metrics_table metric_table; > + struct pmu_layouts_table layout_table; > }; > > /* > @@ -735,6 +819,12 @@ const struct pmu_events_map pmu_events_map[] = { > metric_size = '0' > if event_size == '0' and metric_size == '0': > continue > + layout_tblname = file_name_to_table_name('pmu_layouts_', [], row[2].replace('/', '_')) > + if layout_tblname in _pmu_layouts_tables: > + layout_size = f'ARRAY_SIZE({layout_tblname})' > + else: > + layout_tblname = 'NULL' > + layout_size = '0' > cpuid = row[0].replace('\\', '\\\\') > _args.output_file.write(f"""{{ > \t.arch = "{arch}", > @@ -746,6 +836,10 @@ const struct pmu_events_map pmu_events_map[] = { > \t.metric_table = {{ > \t\t.pmus = {metric_tblname}, > \t\t.num_pmus = {metric_size} > +\t}}, > +\t.layout_table = {{ > +\t\t.entries = {layout_tblname}, > +\t\t.length = {layout_size} > \t}} > }}, > """) > @@ -756,6 +850,7 @@ const struct pmu_events_map pmu_events_map[] = { > \t.cpuid = 0, > \t.event_table = { 0, 0 }, > \t.metric_table = { 0, 0 }, > +\t.layout_table = { 0, 0 }, > } > }; > """) > @@ -824,6 +919,9 @@ static void decompress_event(int offset, struct pmu_event *pe) > _args.output_file.write('\tp++;') > else: > _args.output_file.write('\twhile (*p++);') > + _args.output_file.write('\twhile (*p++);') > + _args.output_file.write(f'\n\tpe->counters_list = ') > + _args.output_file.write("(*p == '\\0' ? NULL : p);\n") > _args.output_file.write("""} > > static void decompress_metric(int offset, struct pmu_metric *pm) > @@ -844,6 +942,30 @@ static void decompress_metric(int offset, struct pmu_metric *pm) > _args.output_file.write('\twhile (*p++);') > _args.output_file.write("""} > > +static void decompress_layout(int offset, struct pmu_layout *pm) > +{ > +\tconst char *p = &big_c_string[offset]; > +""") > + for attr in _json_layout_attributes: > + _args.output_file.write(f'\n\tpm->{attr} = ') > + if attr in _json_enum_attributes: > + _args.output_file.write("*p - '0';\n") > + else: > + _args.output_file.write("(*p == '\\0' ? NULL : p);\n") > + if attr == _json_layout_attributes[-1]: > + continue > + if attr in _json_enum_attributes: > + _args.output_file.write('\tp++;') > + else: > + _args.output_file.write('\twhile (*p++);') > + _args.output_file.write('\tp++;') > + _args.output_file.write(f'\n\tpm->counters_num_gp = ') > + _args.output_file.write("*p - '0';\n") > + _args.output_file.write('\tp++;') > + _args.output_file.write(f'\n\tpm->counters_num_fixed = ') > + _args.output_file.write("*p - '0';\n") > + _args.output_file.write("""} > + > static int pmu_events_table__for_each_event_pmu(const struct pmu_events_table *table, > const struct pmu_table_entry *pmu, > pmu_event_iter_fn fn, > @@ -999,6 +1121,21 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table, > return 0; > } > > +int pmu_layouts_table__for_each_layout(const struct pmu_layouts_table *table, > + pmu_layout_iter_fn fn, > + void *data) { > + for (size_t i = 0; i < table->length; i++) { > + struct pmu_layout pm; > + int ret; > + > + decompress_layout(table->entries[i].offset, &pm); > + ret = fn(&pm, data); > + if (ret) > + return ret; > + } > + return 0; > +} > + > static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu) > { > static struct { > @@ -1094,6 +1231,33 @@ const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pm > return NULL; > } > > +const struct pmu_layouts_table *perf_pmu__find_layouts_table(struct perf_pmu *pmu) > +{ > + const struct pmu_layouts_table *table = NULL; > + char *cpuid = perf_pmu__getcpuid(pmu); > + int i; > + > + /* on some platforms which uses cpus map, cpuid can be NULL for > + * PMUs other than CORE PMUs. > + */ > + if (!cpuid) > + return NULL; > + > + i = 0; > + for (;;) { > + const struct pmu_events_map *map = &pmu_events_map[i++]; > + if (!map->arch) > + break; > + > + if (!strcmp_cpuid_str(map->cpuid, cpuid)) { > + table = &map->layout_table; > + break; > + } > + } > + free(cpuid); > + return table; > +} > + > const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid) > { > for (const struct pmu_events_map *tables = &pmu_events_map[0]; > @@ -1115,6 +1279,16 @@ const struct pmu_metrics_table *find_core_metrics_table(const char *arch, const > } > return NULL; > } > +const struct pmu_layouts_table *find_core_layouts_table(const char *arch, const char *cpuid) > +{ > + for (const struct pmu_events_map *tables = &pmu_events_map[0]; > + tables->arch; > + tables++) { > + if (!strcmp(tables->arch, arch) && !strcmp_cpuid_str(tables->cpuid, cpuid)) > + return &tables->layout_table; > + } > + return NULL; > +} > > int pmu_for_each_core_event(pmu_event_iter_fn fn, void *data) > { > @@ -1143,6 +1317,19 @@ int pmu_for_each_core_metric(pmu_metric_iter_fn fn, void *data) > return 0; > } > > +int pmu_for_each_core_layout(pmu_layout_iter_fn fn, void *data) > +{ > + for (const struct pmu_events_map *tables = &pmu_events_map[0]; > + tables->arch; > + tables++) { > + int ret = pmu_layouts_table__for_each_layout(&tables->layout_table, fn, data); > + > + if (ret) > + return ret; > + } > + return 0; > +} > + > const struct pmu_events_table *find_sys_events_table(const char *name) > { > for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0]; > @@ -1299,6 +1486,7 @@ struct pmu_table_entry { > ftw(arch_path, [], process_one_file) > print_pending_events() > print_pending_metrics() > + print_pending_pmu_counter_layout_table() > > print_mapping_table(archs) > print_system_mapping_table() > diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h > index f5aa96f1685c..5b42a18693cf 100644 > --- a/tools/perf/pmu-events/pmu-events.h > +++ b/tools/perf/pmu-events/pmu-events.h > @@ -45,6 +45,11 @@ struct pmu_event { > const char *desc; > const char *topic; > const char *long_desc; > + /** > + * The list of counter(s) the event could be collected on. > + * eg., "0,1,2,3,4,5,6,7". > + */ > + const char *counters_list; > const char *pmu; > const char *unit; > bool perpkg; > @@ -67,8 +72,18 @@ struct pmu_metric { > enum metric_event_groups event_grouping; > }; > > +struct pmu_layout { > + const char *pmu; > + const char *desc; > + /** Total number of generic counters*/ > + int counters_num_gp; > + /** Total number of fixed counters. Set to zero if no fixed counter on the unit.*/ > + int counters_num_fixed; > +}; > + > struct pmu_events_table; > struct pmu_metrics_table; > +struct pmu_layouts_table; > > typedef int (*pmu_event_iter_fn)(const struct pmu_event *pe, > const struct pmu_events_table *table, > @@ -78,15 +93,21 @@ typedef int (*pmu_metric_iter_fn)(const struct pmu_metric *pm, > const struct pmu_metrics_table *table, > void *data); > > +typedef int (*pmu_layout_iter_fn)(const struct pmu_layout *pm, > + void *data); > + > int pmu_events_table__for_each_event(const struct pmu_events_table *table, > struct perf_pmu *pmu, > pmu_event_iter_fn fn, > void *data); > int pmu_events_table__find_event(const struct pmu_events_table *table, > - struct perf_pmu *pmu, > - const char *name, > - pmu_event_iter_fn fn, > - void *data); > + struct perf_pmu *pmu, > + const char *name, > + pmu_event_iter_fn fn, > + void *data); > +int pmu_layouts_table__for_each_layout(const struct pmu_layouts_table *table, > + pmu_layout_iter_fn fn, > + void *data); > size_t pmu_events_table__num_events(const struct pmu_events_table *table, > struct perf_pmu *pmu); > > @@ -95,10 +116,13 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table, pm > > const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu); > const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu); > +const struct pmu_layouts_table *perf_pmu__find_layouts_table(struct perf_pmu *pmu); > const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid); > const struct pmu_metrics_table *find_core_metrics_table(const char *arch, const char *cpuid); > +const struct pmu_layouts_table *find_core_layouts_table(const char *arch, const char *cpuid); > int pmu_for_each_core_event(pmu_event_iter_fn fn, void *data); > int pmu_for_each_core_metric(pmu_metric_iter_fn fn, void *data); > +int pmu_for_each_core_layout(pmu_layout_iter_fn fn, void *data);
When python isn't installed a build can happen if NO_JEVENTS=1 is passed to the make command. In that case we use a non-generated pmu-events.c copied from tools/perf/pmu-events/empty-pmu-events.c: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/empty-pmu-events.c?h=perf-tools-next
I wonder we should just generate this file, but currently it is written manually. This change builds and links for me with NO_JEVENTS=1 but I get linker error on patch "perf stat: Add functions to get counter info": ```` $ make -C tools/perf O=/tmp/perf DEBUG=1 NO_JEVENTS=1 .. LINK /tmp/perf/perf /usr/bin/ld: /tmp/perf/perf-in.o: in function `get_pmu_counter_layouts': tools/perf/util/metricgroup.c:1739:(.text+0x159d99): undefined reference to `pmu_layouts_table__for_each_layout' /usr/bin/ld: /tmp/perf/perf-in.o: in function `hw_aware_build_grouping': tools/perf/util/metricgroup.c:1764:(.text+0x159e2a): undefined reference to `perf_pmu__find_layouts_table' collect2: error: ld returned 1 exit status ```
Thanks, Ian
> > const struct pmu_events_table *find_sys_events_table(const char *name); > const struct pmu_metrics_table *find_sys_metrics_table(const char *name); > -- > 2.42.0 >
| |