Re: [PATCH 1/5] perf metricgroup: Support printing metrics for arm64

[John Garry]

Hi Jirka,

-       struct pmu_events_map *map = perf_pmu__find_map(NULL);
+       struct pmu_events_map *map = find_cpumap();
so this is just for arm at the moment right?


Yes - but to be more accurate, arm64.

At the moment, from the archs which use pmu-events, only arm64 and nds32 
have versions of get_cpuid_str() which require a non-NULL pmu argument.

But then apparently nds32 only supports a single CPU, so this issue of 
heterogeneous CPUs should not be a concern there :)

could we rather make this arch specific code, so we don't need
to do the scanning on archs where this is not needed?

like marking perf_pmu__find_map as __weak and add arm specific
version?

Well I was thinking that this code should not be in metricgroup.c anyway.

So there is code which is common in current perf_pmu__find_map() for all 
archs.

I could factor that out into a common function, below. Just a bit 
worried about perf_pmu__find_map() and perf_pmu__find_pmu_map() being 
confused.

Here's how that would look:

+++ b/tools/perf/arch/arm64/util/pmu.c

#include "../../util/cpumap.h"
#include "../../util/pmu.h"

struct pmu_events_map *perf_pmu__find_map(void)
{
        struct perf_pmu *pmu = perf_pmu__find("armv8_pmuv3_0");

        if (!pmu || !pmu->cpus || pmu->cpus->nr != cpu__max_cpu())
                return NULL;

        return perf_pmu__find_pmu_map(pmu);
}

And:

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 26c990e32378..312164ce9299 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -618,7 +618,7 @@ static int metricgroup__print_sys_event_iter(struct 
pmu_event *pe, void *data)
 void metricgroup__print(bool metrics, bool metricgroups, char *filter,
                        bool raw, bool details)
 {
-       struct pmu_events_map *map = perf_pmu__find_map(NULL);
+       struct pmu_events_map *map = perf_pmu__find_map();
        struct pmu_event *pe;
        int i;
        struct rblist groups;
@@ -1253,8 +1253,7 @@ int metricgroup__parse_groups(const struct option 
*opt,
                              struct rblist *metric_events)
 {
        struct evlist *perf_evlist = *(struct evlist **)opt->value;
-       struct pmu_events_map *map = perf_pmu__find_map(NULL);
-
+       struct pmu_events_map *map = perf_pmu__find_map();

        return parse_groups(perf_evlist, str, metric_no_group,
                            metric_no_merge, NULL, metric_events, map);
@@ -1273,7 +1272,7 @@ int metricgroup__parse_groups_test(struct evlist 
*evlist,

 bool metricgroup__has_metric(const char *metric)
 {
-       struct pmu_events_map *map = perf_pmu__find_map(NULL);
+       struct pmu_events_map *map = perf_pmu__find_map();
        struct pmu_event *pe;
        int i;

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 44ef28302fc7..d49bf20b6058 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -690,7 +690,7 @@ static char *perf_pmu__getcpuid(struct perf_pmu *pmu)
        return cpuid;
 }

-struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
+struct pmu_events_map *perf_pmu__find_pmu_map(struct perf_pmu *pmu)
 {
        struct pmu_events_map *map;
        char *cpuid = perf_pmu__getcpuid(pmu);
@@ -717,6 +717,11 @@ struct pmu_events_map *perf_pmu__find_map(struct 
perf_pmu *pmu)
        return map;
 }

+struct pmu_events_map *__weak perf_pmu__find_map(void)
+{
+       return perf_pmu__find_pmu_map(NULL);
+}
+
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
 {
        char *tmp = NULL, *tok, *str;
@@ -805,7 +810,7 @@ static void pmu_add_cpu_aliases(struct list_head 
*head, struct perf_pmu *pmu)
 {
        struct pmu_events_map *map;

-       map = perf_pmu__find_map(pmu);
+       map = perf_pmu__find_pmu_map(pmu);
        if (!map)
                return;


Thoughts?

Thanks!