From: Andrew Kilroy <andrew.kilroy@arm.com> To: John Garry <john.garry@huawei.com>, Ian Rogers <irogers@google.com> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, acme@kernel.org, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH 1/1] perf arm64: Implement --topdown with metrics Date: Wed, 15 Dec 2021 12:38:47 +0000 [thread overview] Message-ID: <d91a1b98-2c2b-a961-543f-d540b3c7a146@arm.com> (raw) In-Reply-To: <b1640897-10d7-c11e-4a7a-d17633916c8e@huawei.com> Ian, John, thanks for the feedback. On 15/12/2021 10:52, John Garry wrote: > Hi Andrew, > >>> 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) >>> +{ > > AFAICS, only x86 supports topdown today and that is because they have > special kernel topdown events exposed for the kernel CPU PMU driver. So > other architectures - not only arm - would need rely on metricgroups for > topdown support. So let's make this generic for all archs. > >> 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? > > I think that's a good idea. > Taking a look. > In addition we could also add a --topdown arg to force using JSON > metricgroups. > What arg do think would be supplied? > Did you actually test this patch? I have something experimental working > from some time ago, and it was more complicated than this. I need to > check the code again... > I got stats back from this implementation, yes. Let me know if there's things my patch isn't catering for. > Thanks, > John
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Kilroy <andrew.kilroy@arm.com> To: John Garry <john.garry@huawei.com>, Ian Rogers <irogers@google.com> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, acme@kernel.org, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH 1/1] perf arm64: Implement --topdown with metrics Date: Wed, 15 Dec 2021 12:38:47 +0000 [thread overview] Message-ID: <d91a1b98-2c2b-a961-543f-d540b3c7a146@arm.com> (raw) In-Reply-To: <b1640897-10d7-c11e-4a7a-d17633916c8e@huawei.com> Ian, John, thanks for the feedback. On 15/12/2021 10:52, John Garry wrote: > Hi Andrew, > >>> 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) >>> +{ > > AFAICS, only x86 supports topdown today and that is because they have > special kernel topdown events exposed for the kernel CPU PMU driver. So > other architectures - not only arm - would need rely on metricgroups for > topdown support. So let's make this generic for all archs. > >> 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? > > I think that's a good idea. > Taking a look. > In addition we could also add a --topdown arg to force using JSON > metricgroups. > What arg do think would be supplied? > Did you actually test this patch? I have something experimental working > from some time ago, and it was more complicated than this. I need to > check the code again... > I got stats back from this implementation, yes. Let me know if there's things my patch isn't catering for. > Thanks, > John _______________________________________________ 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-12-15 12:39 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-10 12:37 [PATCH v2 0/2] perf vendor events: Arm Neoverse N2 Andrew Kilroy 2021-12-10 12:37 ` Andrew Kilroy 2021-12-10 12:37 ` [PATCH v2 1/2] perf vendor events: For the " Andrew Kilroy 2021-12-10 12:37 ` Andrew Kilroy 2021-12-10 13:21 ` John Garry 2021-12-10 13:21 ` John Garry 2021-12-14 18:42 ` [RFC PATCH 0/1] topdown with metrics Andrew Kilroy 2021-12-14 18:42 ` Andrew Kilroy 2021-12-14 18:42 ` [RFC PATCH 1/1] perf arm64: Implement --topdown " Andrew Kilroy 2021-12-14 18:42 ` Andrew Kilroy 2021-12-14 20:32 ` Ian Rogers 2021-12-14 20:32 ` Ian Rogers 2021-12-15 10:38 ` James Clark 2021-12-15 10:38 ` James Clark 2021-12-15 10:52 ` John Garry 2021-12-15 10:52 ` John Garry 2021-12-15 12:38 ` Andrew Kilroy [this message] 2021-12-15 12:38 ` Andrew Kilroy 2021-12-15 12:53 ` John Garry 2021-12-15 12:53 ` John Garry 2022-01-06 16:33 ` Andrew Kilroy 2022-01-06 16:33 ` Andrew Kilroy 2022-01-06 18:24 ` John Garry 2022-01-06 18:24 ` John Garry 2022-01-11 15:07 ` [RFC PATCH v2 0/5] topdown " Andrew Kilroy 2022-01-11 15:07 ` [RFC PATCH v2 1/5] perf stat: Implement --topdown " Andrew Kilroy 2022-01-28 13:44 ` John Garry 2022-01-11 15:07 ` [RFC PATCH v2 2/5] perf stat: Topdown kernel events setup function Andrew Kilroy 2022-01-11 15:07 ` [RFC PATCH v2 3/5] perf stat: Topdown json metrics " Andrew Kilroy 2022-01-11 15:07 ` [RFC PATCH v2 4/5] perf stat: Detect if topdown kernel events supported Andrew Kilroy 2022-01-11 15:07 ` [RFC PATCH v2 5/5] perf stat: Ensure only topdown kernel events used on x86 Andrew Kilroy 2022-01-20 9:26 ` [RFC PATCH v2 0/5] topdown with metrics John Garry 2022-01-20 16:22 ` Al Grant 2022-01-27 11:42 ` Andrew Kilroy 2022-02-08 15:58 ` Andrew Kilroy 2021-12-20 17:21 ` [RFC PATCH 1/1] perf arm64: Implement --topdown " Andrew Kilroy 2021-12-20 17:21 ` Andrew Kilroy 2021-12-21 14:03 ` Andi Kleen 2021-12-21 14:03 ` Andi Kleen 2022-01-27 11:11 ` Andrew Kilroy 2022-01-27 11:11 ` Andrew Kilroy 2021-12-17 10:19 ` John Garry 2021-12-17 10:19 ` John Garry 2021-12-21 14:31 ` Andrew Kilroy 2021-12-21 14:31 ` Andrew Kilroy 2022-01-05 16:58 ` Andrew Kilroy 2022-01-05 16:58 ` Andrew Kilroy 2022-01-28 18:00 ` John Garry 2022-01-28 18:00 ` John Garry 2021-12-10 12:37 ` [PATCH v2 2/2] perf vendor events: Rename arm64 arch std event files Andrew Kilroy 2021-12-10 12:37 ` Andrew Kilroy 2021-12-10 13:46 ` John Garry 2021-12-10 13:46 ` John Garry 2021-12-10 19:01 ` Arnaldo Carvalho de Melo 2021-12-10 19:01 ` Arnaldo Carvalho de Melo
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=d91a1b98-2c2b-a961-543f-d540b3c7a146@arm.com \ --to=andrew.kilroy@arm.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=irogers@google.com \ --cc=john.garry@huawei.com \ --cc=jolsa@redhat.com \ --cc=leo.yan@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-perf-users@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=namhyung@kernel.org \ --cc=will@kernel.org \ /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.