Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 29 Aug 2023 08:34:25 -0700 | Subject | Re: [PATCH v2 04/18] perf jevents: Group events by PMU |
| |
On Tue, Aug 29, 2023 at 8:28 AM James Clark <james.clark@arm.com> wrote: > > > > On 24/08/2023 05:13, Ian Rogers wrote: > > Prior to this change a cpuid would map to a list of events where the > > PMU would be encoded alongside the event information. This change > > breaks apart each group of events so that there is a group per PMU. A > > new table is added with the PMU's name and the list of events, the > > original table now holding an array of these per PMU tables. > > > > These changes are to make it easier to get per PMU information about > > events, rather than the current approach of scanning all events. The > > perf binary size with BPF skeletons on x86 is reduced by about 1%. The > > unidentified PMU is now always expanded to "cpu". > > > > Hi Ian, > > I noticed that after this commit in perf-tools-next, events and > descriptions aren't printed by perf list anymore (on Arm). Only the > metrics and builtin kernel events are printed. > > I'm not sure if this is something that's related to test failure that > Arnaldo mentioned on the cover letter, or something else. > > I didn't look into the root cause yet, just thought I would mention it > first.
Hi James,
There was a glitch in this patch assuming json events with no PMU were defaulting to CPU, something that happens a lot in the perf code. The later patch: https://lore.kernel.org/lkml/20230826062203.1058041-1-irogers@google.com/ is supposed to address this.
Thanks, Ian
> Thanks > James > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/pmu-events/jevents.py | 181 +++++++++++++++++++++++-------- > > tools/perf/tests/pmu-events.c | 30 +++-- > > 2 files changed, 154 insertions(+), 57 deletions(-) > > > > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py > > index aae5334099b1..1ad20140114c 100755 > > --- a/tools/perf/pmu-events/jevents.py > > +++ b/tools/perf/pmu-events/jevents.py > > @@ -42,7 +42,7 @@ _metricgroups = {} > > # Order specific JsonEvent attributes will be visited. > > _json_event_attributes = [ > > # cmp_sevent related attributes. > > - 'name', 'pmu', 'topic', 'desc', > > + 'name', 'topic', 'desc', > > # Seems useful, put it early. > > 'event', > > # Short things in alphabetical order. > > @@ -53,7 +53,7 @@ _json_event_attributes = [ > > > > # Attributes that are in pmu_metric rather than pmu_event. > > _json_metric_attributes = [ > > - 'pmu', 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold', > > + 'metric_name', 'metric_group', 'metric_expr', 'metric_threshold', > > 'desc', 'long_desc', 'unit', 'compat', 'metricgroup_no_group', > > 'default_metricgroup_name', 'aggr_mode', 'event_grouping' > > ] > > @@ -252,7 +252,7 @@ class JsonEvent: > > def unit_to_pmu(unit: str) -> Optional[str]: > > """Convert a JSON Unit to Linux PMU name.""" > > if not unit: > > - return None > > + return 'cpu' > > # Comment brought over from jevents.c: > > # it's not realistic to keep adding these, we need something more scalable ... > > table = { > > @@ -343,10 +343,13 @@ class JsonEvent: > > self.desc += extra_desc > > if self.long_desc and extra_desc: > > self.long_desc += extra_desc > > - if self.pmu: > > - if self.desc and not self.desc.endswith('. '): > > - self.desc += '. ' > > - self.desc = (self.desc if self.desc else '') + ('Unit: ' + self.pmu + ' ') > > + if self.pmu and self.pmu != 'cpu': > > + if not self.desc: > > + self.desc = 'Unit: ' + self.pmu > > + else: > > + if not self.desc.endswith('. '): > > + self.desc += '. ' > > + self.desc += 'Unit: ' + self.pmu > > if arch_std: > > if arch_std.lower() in _arch_std_events: > > event = _arch_std_events[arch_std.lower()].event > > @@ -437,13 +440,13 @@ def add_events_table_entries(item: os.DirEntry, topic: str) -> None: > > def print_pending_events() -> None: > > """Optionally close events table.""" > > > > - def event_cmp_key(j: JsonEvent) -> Tuple[bool, str, str, str, str]: > > + def event_cmp_key(j: JsonEvent) -> Tuple[str, str, 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.topic), fix_none(j.name), fix_none(j.pmu), > > + return (fix_none(j.pmu).replace(',','_'), fix_none(j.name), j.desc is not None, fix_none(j.topic), > > fix_none(j.metric_name)) > > > > global _pending_events > > @@ -458,13 +461,36 @@ def print_pending_events() -> None: > > global event_tables > > _event_tables.append(_pending_events_tblname) > > > > - _args.output_file.write( > > - f'static const struct compact_pmu_event {_pending_events_tblname}[] = {{\n') > > - > > + first = True > > + last_pmu = None > > + pmus = set() > > for event in sorted(_pending_events, key=event_cmp_key): > > + if event.pmu != last_pmu: > > + if not first: > > + _args.output_file.write('};\n') > > + pmu_name = event.pmu.replace(',', '_') > > + _args.output_file.write( > > + f'static const struct compact_pmu_event {_pending_events_tblname}_{pmu_name}[] = {{\n') > > + first = False > > + last_pmu = event.pmu > > + pmus.add((event.pmu, pmu_name)) > > + > > _args.output_file.write(event.to_c_string(metric=False)) > > _pending_events = [] > > > > + _args.output_file.write(f""" > > +}}; > > + > > +const struct pmu_table_entry {_pending_events_tblname}[] = {{ > > +""") > > + for (pmu, tbl_pmu) in sorted(pmus): > > + pmu_name = f"{pmu}\\000" > > + _args.output_file.write(f"""{{ > > + .entries = {_pending_events_tblname}_{tbl_pmu}, > > + .num_entries = ARRAY_SIZE({_pending_events_tblname}_{tbl_pmu}), > > + .pmu_name = {{ {_bcs.offsets[pmu_name]} /* {pmu_name} */ }}, > > +}}, > > +""") > > _args.output_file.write('};\n\n') > > > > def print_pending_metrics() -> None: > > @@ -490,13 +516,36 @@ def print_pending_metrics() -> None: > > global metric_tables > > _metric_tables.append(_pending_metrics_tblname) > > > > - _args.output_file.write( > > - f'static const struct compact_pmu_event {_pending_metrics_tblname}[] = {{\n') > > - > > + first = True > > + last_pmu = None > > + pmus = set() > > for metric in sorted(_pending_metrics, key=metric_cmp_key): > > + if metric.pmu != last_pmu: > > + if not first: > > + _args.output_file.write('};\n') > > + pmu_name = metric.pmu.replace(',', '_') > > + _args.output_file.write( > > + f'static const struct compact_pmu_event {_pending_metrics_tblname}_{pmu_name}[] = {{\n') > > + first = False > > + last_pmu = metric.pmu > > + pmus.add((metric.pmu, pmu_name)) > > + > > _args.output_file.write(metric.to_c_string(metric=True)) > > _pending_metrics = [] > > > > + _args.output_file.write(f""" > > +}}; > > + > > +const struct pmu_table_entry {_pending_metrics_tblname}[] = {{ > > +""") > > + for (pmu, tbl_pmu) in sorted(pmus): > > + pmu_name = f"{pmu}\\000" > > + _args.output_file.write(f"""{{ > > + .entries = {_pending_metrics_tblname}_{tbl_pmu}, > > + .num_entries = ARRAY_SIZE({_pending_metrics_tblname}_{tbl_pmu}), > > + .pmu_name = {{ {_bcs.offsets[pmu_name]} /* {pmu_name} */ }}, > > +}}, > > +""") > > _args.output_file.write('};\n\n') > > > > def get_topic(topic: str) -> str: > > @@ -532,6 +581,8 @@ def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None: > > > > topic = get_topic(item.name) > > for event in read_json_events(item.path, topic): > > + pmu_name = f"{event.pmu}\\000" > > + _bcs.add(pmu_name) > > if event.name: > > _bcs.add(event.build_c_string(metric=False)) > > if event.metric_name: > > @@ -577,14 +628,14 @@ def print_mapping_table(archs: Sequence[str]) -> None: > > _args.output_file.write(""" > > /* Struct used to make the PMU event table implementation opaque to callers. */ > > struct pmu_events_table { > > - const struct compact_pmu_event *entries; > > - size_t length; > > + const struct pmu_table_entry *pmus; > > + uint32_t num_pmus; > > }; > > > > /* Struct used to make the PMU metric table implementation opaque to callers. */ > > struct pmu_metrics_table { > > - const struct compact_pmu_event *entries; > > - size_t length; > > + const struct pmu_table_entry *pmus; > > + uint32_t num_pmus; > > }; > > > > /* > > @@ -614,12 +665,12 @@ const struct pmu_events_map pmu_events_map[] = { > > \t.arch = "testarch", > > \t.cpuid = "testcpu", > > \t.event_table = { > > -\t\t.entries = pmu_events__test_soc_cpu, > > -\t\t.length = ARRAY_SIZE(pmu_events__test_soc_cpu), > > +\t\t.pmus = pmu_events__test_soc_cpu, > > +\t\t.num_pmus = ARRAY_SIZE(pmu_events__test_soc_cpu), > > \t}, > > \t.metric_table = { > > -\t\t.entries = pmu_metrics__test_soc_cpu, > > -\t\t.length = ARRAY_SIZE(pmu_metrics__test_soc_cpu), > > +\t\t.pmus = pmu_metrics__test_soc_cpu, > > +\t\t.num_pmus = ARRAY_SIZE(pmu_metrics__test_soc_cpu), > > \t} > > }, > > """) > > @@ -649,12 +700,12 @@ const struct pmu_events_map pmu_events_map[] = { > > \t.arch = "{arch}", > > \t.cpuid = "{cpuid}", > > \t.event_table = {{ > > -\t\t.entries = {event_tblname}, > > -\t\t.length = {event_size} > > +\t\t.pmus = {event_tblname}, > > +\t\t.num_pmus = {event_size} > > \t}}, > > \t.metric_table = {{ > > -\t\t.entries = {metric_tblname}, > > -\t\t.length = {metric_size} > > +\t\t.pmus = {metric_tblname}, > > +\t\t.num_pmus = {metric_size} > > \t}} > > }}, > > """) > > @@ -685,15 +736,15 @@ static const struct pmu_sys_events pmu_sys_event_tables[] = { > > for tblname in _sys_event_tables: > > _args.output_file.write(f"""\t{{ > > \t\t.event_table = {{ > > -\t\t\t.entries = {tblname}, > > -\t\t\t.length = ARRAY_SIZE({tblname}) > > +\t\t\t.pmus = {tblname}, > > +\t\t\t.num_pmus = ARRAY_SIZE({tblname}) > > \t\t}},""") > > metric_tblname = _sys_event_table_to_metric_table_mapping[tblname] > > if metric_tblname in _sys_metric_tables: > > _args.output_file.write(f""" > > \t\t.metric_table = {{ > > -\t\t\t.entries = {metric_tblname}, > > -\t\t\t.length = ARRAY_SIZE({metric_tblname}) > > +\t\t\t.pmus = {metric_tblname}, > > +\t\t\t.num_pmus = ARRAY_SIZE({metric_tblname}) > > \t\t}},""") > > printed_metric_tables.append(metric_tblname) > > _args.output_file.write(f""" > > @@ -753,18 +804,56 @@ static void decompress_metric(int offset, struct pmu_metric *pm) > > _args.output_file.write('\twhile (*p++);') > > _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, > > + void *data) > > +{ > > + int ret; > > + struct pmu_event pe = { > > + .pmu = &big_c_string[pmu->pmu_name.offset], > > + }; > > + > > + for (uint32_t i = 0; i < pmu->num_entries; i++) { > > + decompress_event(pmu->entries[i].offset, &pe); > > + if (!pe.name) > > + continue; > > + ret = fn(&pe, table, data); > > + if (ret) > > + return ret; > > + } > > + return 0; > > + } > > + > > int pmu_events_table__for_each_event(const struct pmu_events_table *table, > > pmu_event_iter_fn fn, > > void *data) > > { > > - for (size_t i = 0; i < table->length; i++) { > > - struct pmu_event pe; > > - int ret; > > + for (size_t i = 0; i < table->num_pmus; i++) { > > + int ret = pmu_events_table__for_each_event_pmu(table, &table->pmus[i], > > + fn, data); > > > > - decompress_event(table->entries[i].offset, &pe); > > - if (!pe.name) > > + if (ret) > > + return ret; > > + } > > + return 0; > > +} > > + > > +static int pmu_metrics_table__for_each_metric_pmu(const struct pmu_metrics_table *table, > > + const struct pmu_table_entry *pmu, > > + pmu_metric_iter_fn fn, > > + void *data) > > +{ > > + int ret; > > + struct pmu_metric pm = { > > + .pmu = &big_c_string[pmu->pmu_name.offset], > > + }; > > + > > + for (uint32_t i = 0; i < pmu->num_entries; i++) { > > + decompress_metric(pmu->entries[i].offset, &pm); > > + if (!pm.metric_expr) > > continue; > > - ret = fn(&pe, table, data); > > + ret = fn(&pm, table, data); > > if (ret) > > return ret; > > } > > @@ -775,14 +864,10 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table, > > pmu_metric_iter_fn fn, > > void *data) > > { > > - for (size_t i = 0; i < table->length; i++) { > > - struct pmu_metric pm; > > - int ret; > > + for (size_t i = 0; i < table->num_pmus; i++) { > > + int ret = pmu_metrics_table__for_each_metric_pmu(table, &table->pmus[i], > > + fn, data); > > > > - decompress_metric(table->entries[i].offset, &pm); > > - if (!pm.metric_expr) > > - continue; > > - ret = fn(&pm, table, data); > > if (ret) > > return ret; > > } > > @@ -1010,7 +1095,13 @@ such as "arm/cortex-a34".''', > > #include <stddef.h> > > > > struct compact_pmu_event { > > - int offset; > > + int offset; > > +}; > > + > > +struct pmu_table_entry { > > + const struct compact_pmu_event *entries; > > + uint32_t num_entries; > > + struct compact_pmu_event pmu_name; > > }; > > > > """) > > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c > > index 5f541eadc088..0b6efabc3d20 100644 > > --- a/tools/perf/tests/pmu-events.c > > +++ b/tools/perf/tests/pmu-events.c > > @@ -44,6 +44,7 @@ struct perf_pmu_test_pmu { > > > > static const struct perf_pmu_test_event bp_l1_btb_correct = { > > .event = { > > + .pmu = "cpu", > > .name = "bp_l1_btb_correct", > > .event = "event=0x8a", > > .desc = "L1 BTB Correction", > > @@ -55,6 +56,7 @@ static const struct perf_pmu_test_event bp_l1_btb_correct = { > > > > static const struct perf_pmu_test_event bp_l2_btb_correct = { > > .event = { > > + .pmu = "cpu", > > .name = "bp_l2_btb_correct", > > .event = "event=0x8b", > > .desc = "L2 BTB Correction", > > @@ -66,6 +68,7 @@ static const struct perf_pmu_test_event bp_l2_btb_correct = { > > > > static const struct perf_pmu_test_event segment_reg_loads_any = { > > .event = { > > + .pmu = "cpu", > > .name = "segment_reg_loads.any", > > .event = "event=0x6,period=200000,umask=0x80", > > .desc = "Number of segment register loads", > > @@ -77,6 +80,7 @@ static const struct perf_pmu_test_event segment_reg_loads_any = { > > > > static const struct perf_pmu_test_event dispatch_blocked_any = { > > .event = { > > + .pmu = "cpu", > > .name = "dispatch_blocked.any", > > .event = "event=0x9,period=200000,umask=0x20", > > .desc = "Memory cluster signals to block micro-op dispatch for any reason", > > @@ -88,6 +92,7 @@ static const struct perf_pmu_test_event dispatch_blocked_any = { > > > > static const struct perf_pmu_test_event eist_trans = { > > .event = { > > + .pmu = "cpu", > > .name = "eist_trans", > > .event = "event=0x3a,period=200000,umask=0x0", > > .desc = "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions", > > @@ -99,6 +104,7 @@ static const struct perf_pmu_test_event eist_trans = { > > > > static const struct perf_pmu_test_event l3_cache_rd = { > > .event = { > > + .pmu = "cpu", > > .name = "l3_cache_rd", > > .event = "event=0x40", > > .desc = "L3 cache access, read", > > @@ -123,7 +129,7 @@ static const struct perf_pmu_test_event uncore_hisi_ddrc_flux_wcmd = { > > .event = { > > .name = "uncore_hisi_ddrc.flux_wcmd", > > .event = "event=0x2", > > - .desc = "DDRC write commands. Unit: hisi_sccl,ddrc ", > > + .desc = "DDRC write commands. Unit: hisi_sccl,ddrc", > > .topic = "uncore", > > .long_desc = "DDRC write commands", > > .pmu = "hisi_sccl,ddrc", > > @@ -137,7 +143,7 @@ static const struct perf_pmu_test_event unc_cbo_xsnp_response_miss_eviction = { > > .event = { > > .name = "unc_cbo_xsnp_response.miss_eviction", > > .event = "event=0x22,umask=0x81", > > - .desc = "A cross-core snoop resulted from L3 Eviction which misses in some processor core. Unit: uncore_cbox ", > > + .desc = "A cross-core snoop resulted from L3 Eviction which misses in some processor core. Unit: uncore_cbox", > > .topic = "uncore", > > .long_desc = "A cross-core snoop resulted from L3 Eviction which misses in some processor core", > > .pmu = "uncore_cbox", > > @@ -151,7 +157,7 @@ static const struct perf_pmu_test_event uncore_hyphen = { > > .event = { > > .name = "event-hyphen", > > .event = "event=0xe0,umask=0x00", > > - .desc = "UNC_CBO_HYPHEN. Unit: uncore_cbox ", > > + .desc = "UNC_CBO_HYPHEN. Unit: uncore_cbox", > > .topic = "uncore", > > .long_desc = "UNC_CBO_HYPHEN", > > .pmu = "uncore_cbox", > > @@ -165,7 +171,7 @@ static const struct perf_pmu_test_event uncore_two_hyph = { > > .event = { > > .name = "event-two-hyph", > > .event = "event=0xc0,umask=0x00", > > - .desc = "UNC_CBO_TWO_HYPH. Unit: uncore_cbox ", > > + .desc = "UNC_CBO_TWO_HYPH. Unit: uncore_cbox", > > .topic = "uncore", > > .long_desc = "UNC_CBO_TWO_HYPH", > > .pmu = "uncore_cbox", > > @@ -179,7 +185,7 @@ static const struct perf_pmu_test_event uncore_hisi_l3c_rd_hit_cpipe = { > > .event = { > > .name = "uncore_hisi_l3c.rd_hit_cpipe", > > .event = "event=0x7", > > - .desc = "Total read hits. Unit: hisi_sccl,l3c ", > > + .desc = "Total read hits. Unit: hisi_sccl,l3c", > > .topic = "uncore", > > .long_desc = "Total read hits", > > .pmu = "hisi_sccl,l3c", > > @@ -193,7 +199,7 @@ static const struct perf_pmu_test_event uncore_imc_free_running_cache_miss = { > > .event = { > > .name = "uncore_imc_free_running.cache_miss", > > .event = "event=0x12", > > - .desc = "Total cache misses. Unit: uncore_imc_free_running ", > > + .desc = "Total cache misses. Unit: uncore_imc_free_running", > > .topic = "uncore", > > .long_desc = "Total cache misses", > > .pmu = "uncore_imc_free_running", > > @@ -207,7 +213,7 @@ static const struct perf_pmu_test_event uncore_imc_cache_hits = { > > .event = { > > .name = "uncore_imc.cache_hits", > > .event = "event=0x34", > > - .desc = "Total cache hits. Unit: uncore_imc ", > > + .desc = "Total cache hits. Unit: uncore_imc", > > .topic = "uncore", > > .long_desc = "Total cache hits", > > .pmu = "uncore_imc", > > @@ -232,13 +238,13 @@ static const struct perf_pmu_test_event sys_ddr_pmu_write_cycles = { > > .event = { > > .name = "sys_ddr_pmu.write_cycles", > > .event = "event=0x2b", > > - .desc = "ddr write-cycles event. Unit: uncore_sys_ddr_pmu ", > > + .desc = "ddr write-cycles event. Unit: uncore_sys_ddr_pmu", > > .topic = "uncore", > > .pmu = "uncore_sys_ddr_pmu", > > .compat = "v8", > > }, > > .alias_str = "event=0x2b", > > - .alias_long_desc = "ddr write-cycles event. Unit: uncore_sys_ddr_pmu ", > > + .alias_long_desc = "ddr write-cycles event. Unit: uncore_sys_ddr_pmu", > > .matching_pmu = "uncore_sys_ddr_pmu", > > }; > > > > @@ -246,13 +252,13 @@ static const struct perf_pmu_test_event sys_ccn_pmu_read_cycles = { > > .event = { > > .name = "sys_ccn_pmu.read_cycles", > > .event = "config=0x2c", > > - .desc = "ccn read-cycles event. Unit: uncore_sys_ccn_pmu ", > > + .desc = "ccn read-cycles event. Unit: uncore_sys_ccn_pmu", > > .topic = "uncore", > > .pmu = "uncore_sys_ccn_pmu", > > .compat = "0x01", > > }, > > .alias_str = "config=0x2c", > > - .alias_long_desc = "ccn read-cycles event. Unit: uncore_sys_ccn_pmu ", > > + .alias_long_desc = "ccn read-cycles event. Unit: uncore_sys_ccn_pmu", > > .matching_pmu = "uncore_sys_ccn_pmu", > > }; > > > > @@ -403,7 +409,7 @@ static int test__pmu_event_table_core_callback(const struct pmu_event *pe, > > struct perf_pmu_test_event const **test_event_table; > > bool found = false; > > > > - if (pe->pmu) > > + if (strcmp(pe->pmu, "cpu")) > > test_event_table = &uncore_events[0]; > > else > > test_event_table = &core_events[0];
| |