[RFC PATCH 1/1] perf arm64: Implement --topdown with metrics
James Clark
james.clark at arm.com
Wed Dec 15 02:38:46 PST 2021
On 14/12/2021 20:32, Ian Rogers wrote:
> On Tue, Dec 14, 2021 at 10:43 AM Andrew Kilroy <andrew.kilroy at arm.com> wrote:
>>
>> This patch implements the --topdown option by making use of metrics to
>> dictate what counters are obtained in order to show the various topdown
>> columns, e.g. Frontend Bound, Backend Bound, Retiring and Bad
>> Speculation.
>>
>> The MetricGroup name is used to identify which set of metrics are to be
>> shown. For the moment use TopDownL1 and enable for arm64
>>
>> Signed-off-by: Andrew Kilroy <andrew.kilroy at arm.com>
>> ---
>> tools/perf/arch/arm64/util/Build | 1 +
>> tools/perf/arch/arm64/util/topdown.c | 8 ++++++++
>> tools/perf/builtin-stat.c | 13 +++++++++++++
>> tools/perf/util/metricgroup.c | 12 ++++++++++++
>> tools/perf/util/metricgroup.h | 7 +++++++
>> tools/perf/util/topdown.c | 6 ++++++
>> tools/perf/util/topdown.h | 1 +
>> 7 files changed, 48 insertions(+)
>> create mode 100644 tools/perf/arch/arm64/util/topdown.c
>>
>> diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
>> index 9fcb4e68add9..9807aed981cd 100644
>> --- a/tools/perf/arch/arm64/util/Build
>> +++ b/tools/perf/arch/arm64/util/Build
>> @@ -4,6 +4,7 @@ perf-y += perf_regs.o
>> perf-y += tsc.o
>> perf-y += pmu.o
>> perf-y += kvm-stat.o
>> +perf-y += topdown.o
>> perf-$(CONFIG_DWARF) += dwarf-regs.o
>> perf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
>> perf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
>> diff --git a/tools/perf/arch/arm64/util/topdown.c b/tools/perf/arch/arm64/util/topdown.c
>> new file mode 100644
>> index 000000000000..a2b1f9c01148
>> --- /dev/null
>> +++ b/tools/perf/arch/arm64/util/topdown.c
>> @@ -0,0 +1,8 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <topdown.h>
>> +
>> +
>> +bool arch_topdown_use_json_metrics(void)
>> +{
>> + return true;
>> +}
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index f6ca2b054c5b..08ef80ef345e 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1856,6 +1856,18 @@ static int add_default_attributes(void)
>> if (!force_metric_only)
>> stat_config.metric_only = true;
>>
>> + if (arch_topdown_use_json_metrics()) {
>> + if (metricgroup__parse_groups_to_evlist(evsel_list, "TopDownL1",
>> + stat_config.metric_no_group,
>> + stat_config.metric_no_merge,
>> + &stat_config.metric_events) < 0) {
>> + pr_err("Could not form list of metrics for topdown\n");
>> + return -1;
>> + }
>> +
>> + goto end_of_topdown_setup;
>> + }
>> +
>> if (pmu_have_event("cpu", topdown_metric_L2_attrs[5])) {
>> metric_attrs = topdown_metric_L2_attrs;
>> max_level = 2;
>> @@ -1919,6 +1931,7 @@ static int add_default_attributes(void)
>> fprintf(stderr, "System does not support topdown\n");
>> return -1;
>> }
>> +end_of_topdown_setup:
>> free(str);
>> }
>>
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index 51c99cb08abf..9b0394372096 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -1535,6 +1535,18 @@ int metricgroup__parse_groups(const struct option *opt,
>> metric_no_merge, NULL, metric_events, map);
>> }
>>
>> +int metricgroup__parse_groups_to_evlist(struct evlist *perf_evlist,
>> + const char *str,
>> + bool metric_no_group,
>> + bool metric_no_merge,
>> + struct rblist *metric_events)
>> +{
>> + const struct pmu_events_map *map = pmu_events_map__find();
>> +
>> + return parse_groups(perf_evlist, str, metric_no_group,
>> + metric_no_merge, NULL, metric_events, map);
>> +}
>> +
>> int metricgroup__parse_groups_test(struct evlist *evlist,
>> const struct pmu_events_map *map,
>> const char *str,
>> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
>> index 2b42b778d1bf..1f143ad1d9e1 100644
>> --- a/tools/perf/util/metricgroup.h
>> +++ b/tools/perf/util/metricgroup.h
>> @@ -70,6 +70,13 @@ int metricgroup__parse_groups(const struct option *opt,
>> bool metric_no_group,
>> bool metric_no_merge,
>> struct rblist *metric_events);
>> +
>> +int metricgroup__parse_groups_to_evlist(struct evlist *perf_evlist,
>> + const char *str,
>> + bool metric_no_group,
>> + bool metric_no_merge,
>> + struct rblist *metric_events);
>> +
>> const struct pmu_event *metricgroup__find_metric(const char *metric,
>> const struct pmu_events_map *map);
>> int metricgroup__parse_groups_test(struct evlist *evlist,
>> diff --git a/tools/perf/util/topdown.c b/tools/perf/util/topdown.c
>> index 1081b20f9891..57c0c5f2c6bd 100644
>> --- a/tools/perf/util/topdown.c
>> +++ b/tools/perf/util/topdown.c
>> @@ -56,3 +56,9 @@ __weak bool arch_topdown_sample_read(struct evsel *leader __maybe_unused)
>> {
>> return false;
>> }
>> +
>> +__weak bool arch_topdown_use_json_metrics(void)
>> +{
>
> I like this extension! I've ranted in the past about weak symbols
> breaking with archives due to lazy loading [1]. In this case
> tools/perf/arch/arm64/util/topdown.c has no other symbols within it
> and so the weak symbol has an extra chance of being linked
> incorrectly. We could add a new command line of --topdown-json to
> avoid this, but there seems little difference in doing this over just
> doing '-M TopDownL1'. Is it possible to use the json metric approach
> for when the CPU version fails?
If weak symbols are an issue we could define the function normally for
all known platforms. Or just do arm64 and 'other'. I think the end result is
the same. Or have only one function like this:
bool arch_topdown_use_json_metrics(void)
{
#ifdef aarch64
return true;
#elseif ...
}
There are quite a few ways to avoid it. I'm not sure I like the
--topdown-json argument as it would be quite fragmented from a user's
point of view, especially if it was only to work around some linking
issue.
James
>
> Thanks,
> Ian
>
> [1] https://lore.kernel.org/lkml/CAP-5=fVS2AwZ9bP4BjF9GDcZqmw5fwUZ6OGXdgMnFj3w_2OTaw@mail.gmail.com/
>
>> + return false;
>> +}
>> +
>> diff --git a/tools/perf/util/topdown.h b/tools/perf/util/topdown.h
>> index 2f0d0b887639..0a5275a3f078 100644
>> --- a/tools/perf/util/topdown.h
>> +++ b/tools/perf/util/topdown.h
>> @@ -6,6 +6,7 @@
>> bool arch_topdown_check_group(bool *warn);
>> void arch_topdown_group_warn(void);
>> bool arch_topdown_sample_read(struct evsel *leader);
>> +bool arch_topdown_use_json_metrics(void);
>>
>> int topdown_filter_events(const char **attr, char **str, bool use_group);
>>
>> --
>> 2.17.1
>>
More information about the linux-arm-kernel
mailing list