From: John Garry <john.garry@huawei.com> To: Jiri Olsa <jolsa@redhat.com> Cc: <will@kernel.org>, <mathieu.poirier@linaro.org>, <leo.yan@linaro.org>, <peterz@infradead.org>, <mingo@redhat.com>, <acme@kernel.org>, <mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>, <namhyung@kernel.org>, <irogers@google.com>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linuxarm@huawei.com>, <zhangshaokun@hisilicon.com>, <qiangqing.zhang@nxp.com>, <kjain@linux.ibm.com> Subject: Re: [PATCH 1/5] perf metricgroup: Support printing metrics for arm64 Date: Fri, 5 Mar 2021 11:06:58 +0000 [thread overview] Message-ID: <95205463-4c80-4e8a-a7c0-c2a4e4553838@huawei.com> (raw) In-Reply-To: <YEE9oInI38txHWmo@krava> 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!
WARNING: multiple messages have this Message-ID (diff)
From: John Garry <john.garry@huawei.com> To: Jiri Olsa <jolsa@redhat.com> Cc: <will@kernel.org>, <mathieu.poirier@linaro.org>, <leo.yan@linaro.org>, <peterz@infradead.org>, <mingo@redhat.com>, <acme@kernel.org>, <mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>, <namhyung@kernel.org>, <irogers@google.com>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linuxarm@huawei.com>, <zhangshaokun@hisilicon.com>, <qiangqing.zhang@nxp.com>, <kjain@linux.ibm.com> Subject: Re: [PATCH 1/5] perf metricgroup: Support printing metrics for arm64 Date: Fri, 5 Mar 2021 11:06:58 +0000 [thread overview] Message-ID: <95205463-4c80-4e8a-a7c0-c2a4e4553838@huawei.com> (raw) In-Reply-To: <YEE9oInI38txHWmo@krava> 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! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-05 11:09 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-03 15:22 [PATCH 0/5] perf arm64 metricgroup support John Garry 2021-03-03 15:22 ` John Garry 2021-03-03 15:22 ` [PATCH 1/5] perf metricgroup: Support printing metrics for arm64 John Garry 2021-03-03 15:22 ` John Garry 2021-03-04 20:05 ` Jiri Olsa 2021-03-04 20:05 ` Jiri Olsa 2021-03-05 11:06 ` John Garry [this message] 2021-03-05 11:06 ` John Garry 2021-03-06 19:34 ` Jiri Olsa 2021-03-06 19:34 ` Jiri Olsa 2021-03-08 16:34 ` John Garry 2021-03-08 16:34 ` John Garry 2021-03-11 8:47 ` John Garry 2021-03-11 8:47 ` John Garry 2021-03-03 15:22 ` [PATCH 2/5] perf metricgroup: Support adding " John Garry 2021-03-03 15:22 ` John Garry 2021-03-03 15:22 ` [PATCH 3/5] perf vendor events arm64: Add Hisi hip08 L1 metrics John Garry 2021-03-03 15:22 ` John Garry 2021-03-03 15:22 ` [PATCH 4/5] perf vendor events arm64: Add Hisi hip08 L2 metrics John Garry 2021-03-03 15:22 ` John Garry 2021-03-03 15:22 ` [PATCH 5/5] perf vendor events arm64: Add Hisi hip08 L3 metrics John Garry 2021-03-03 15:22 ` John Garry
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=95205463-4c80-4e8a-a7c0-c2a4e4553838@huawei.com \ --to=john.garry@huawei.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=irogers@google.com \ --cc=jolsa@redhat.com \ --cc=kjain@linux.ibm.com \ --cc=leo.yan@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=qiangqing.zhang@nxp.com \ --cc=will@kernel.org \ --cc=zhangshaokun@hisilicon.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.