* [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
@ 2022-04-13 7:51 Leo Yan
2022-04-13 8:15 ` German Gomez
0 siblings, 1 reply; 7+ messages in thread
From: Leo Yan @ 2022-04-13 7:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ravi Bangoria, James Clark, German Gomez, linux-perf-users,
linux-kernel
Cc: Leo Yan
Since commit bb30acae4c4d ("perf report: Bail out --mem-mode if mem info
is not available") "perf mem report" and "perf report --mem-mode"
don't report result if the PERF_SAMPLE_DATA_SRC bit is missed in sample
type.
The commit ffab48705205 ("perf: arm-spe: Fix perf report --mem-mode")
partially fixes the issue. It adds PERF_SAMPLE_DATA_SRC bit for Arm SPE
event, this allows the perf data file generated by kernel v5.18-rc1 or
later version can be reported properly.
On the other hand, perf tool still fails to be backward compatibility
for a data file recorded by an older version's perf which contains Arm
SPE trace data. This patch is a workaround in reporting phase, when
detects ARM SPE PMU event and without PERF_SAMPLE_DATA_SRC bit, it will
force to set the bit in the sample type and give a warning info.
Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/builtin-report.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1ad75c7ba074..f26dd14eb852 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -353,6 +353,7 @@ static int report__setup_sample_type(struct report *rep)
struct perf_session *session = rep->session;
u64 sample_type = evlist__combined_sample_type(session->evlist);
bool is_pipe = perf_data__is_pipe(session->data);
+ struct evsel *evsel;
if (session->itrace_synth_opts->callchain ||
session->itrace_synth_opts->add_callchain ||
@@ -407,6 +408,21 @@ static int report__setup_sample_type(struct report *rep)
}
if (sort__mode == SORT_MODE__MEMORY) {
+ /*
+ * FIXUP: prior to kernel 5.18, Arm SPE missed to set
+ * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
+ * compatibility, set the bit if it's an old perf data file.
+ */
+ evlist__for_each_entry(session->evlist, evsel) {
+ if (strstr(evsel->name, "arm_spe_") &&
+ !(sample_type & PERF_SAMPLE_DATA_SRC)) {
+ ui__warning("PERF_SAMPLE_DATA_SRC bit is not set "
+ "for Arm SPE event.\n");
+ evsel->core.attr.sample_type |= PERF_SAMPLE_DATA_SRC;
+ sample_type |= PERF_SAMPLE_DATA_SRC;
+ }
+ }
+
if (!is_pipe && !(sample_type & PERF_SAMPLE_DATA_SRC)) {
ui__error("Selected --mem-mode but no mem data. "
"Did you call perf record without -d?\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
2022-04-13 7:51 [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event Leo Yan
@ 2022-04-13 8:15 ` German Gomez
2022-04-13 8:49 ` Leo Yan
0 siblings, 1 reply; 7+ messages in thread
From: German Gomez @ 2022-04-13 8:15 UTC (permalink / raw)
To: Leo Yan, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ravi Bangoria, James Clark, linux-perf-users, linux-kernel
Hi Leo,
On 13/04/2022 08:51, Leo Yan wrote:
> Since commit bb30acae4c4d ("perf report: Bail out --mem-mode if mem info
> is not available") "perf mem report" and "perf report --mem-mode"
> don't report result if the PERF_SAMPLE_DATA_SRC bit is missed in sample
> type.
>
> The commit ffab48705205 ("perf: arm-spe: Fix perf report --mem-mode")
> partially fixes the issue. It adds PERF_SAMPLE_DATA_SRC bit for Arm SPE
> event, this allows the perf data file generated by kernel v5.18-rc1 or
> later version can be reported properly.
>
> On the other hand, perf tool still fails to be backward compatibility
> for a data file recorded by an older version's perf which contains Arm
> SPE trace data. This patch is a workaround in reporting phase, when
> detects ARM SPE PMU event and without PERF_SAMPLE_DATA_SRC bit, it will
> force to set the bit in the sample type and give a warning info.
>
> Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/builtin-report.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 1ad75c7ba074..f26dd14eb852 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -353,6 +353,7 @@ static int report__setup_sample_type(struct report *rep)
> struct perf_session *session = rep->session;
> u64 sample_type = evlist__combined_sample_type(session->evlist);
> bool is_pipe = perf_data__is_pipe(session->data);
> + struct evsel *evsel;
>
> if (session->itrace_synth_opts->callchain ||
> session->itrace_synth_opts->add_callchain ||
> @@ -407,6 +408,21 @@ static int report__setup_sample_type(struct report *rep)
> }
>
> if (sort__mode == SORT_MODE__MEMORY) {
> + /*
> + * FIXUP: prior to kernel 5.18, Arm SPE missed to set
> + * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
> + * compatibility, set the bit if it's an old perf data file.
> + */
> + evlist__for_each_entry(session->evlist, evsel) {
> + if (strstr(evsel->name, "arm_spe_") &&
This didn't work for me when the file recorded "-e arm_spe//" instead of
"-e arm_spe_0//". Could you remove the trailing _? With that:
Tested-by: German Gomez <german.gomez@arm.com>
> + !(sample_type & PERF_SAMPLE_DATA_SRC)) {
> + ui__warning("PERF_SAMPLE_DATA_SRC bit is not set "
> + "for Arm SPE event.\n");
> + evsel->core.attr.sample_type |= PERF_SAMPLE_DATA_SRC;
> + sample_type |= PERF_SAMPLE_DATA_SRC;
> + }
> + }
> +
> if (!is_pipe && !(sample_type & PERF_SAMPLE_DATA_SRC)) {
> ui__error("Selected --mem-mode but no mem data. "
> "Did you call perf record without -d?\n");
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
2022-04-13 8:15 ` German Gomez
@ 2022-04-13 8:49 ` Leo Yan
2022-04-13 9:13 ` Leo Yan
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Leo Yan @ 2022-04-13 8:49 UTC (permalink / raw)
To: German Gomez
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ravi Bangoria, James Clark, linux-perf-users, linux-kernel
On Wed, Apr 13, 2022 at 09:15:40AM +0100, German Gomez wrote:
[...]
> > if (sort__mode == SORT_MODE__MEMORY) {
> > + /*
> > + * FIXUP: prior to kernel 5.18, Arm SPE missed to set
> > + * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
> > + * compatibility, set the bit if it's an old perf data file.
> > + */
> > + evlist__for_each_entry(session->evlist, evsel) {
> > + if (strstr(evsel->name, "arm_spe_") &&
>
> This didn't work for me when the file recorded "-e arm_spe//" instead of
> "-e arm_spe_0//". Could you remove the trailing _? With that:
Sure, will change to "arm_spe". Just curious, if there any local
change at your side so we have the different event name?
> Tested-by: German Gomez <german.gomez@arm.com>
Thanks a lot, German!
Leo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
2022-04-13 8:49 ` Leo Yan
@ 2022-04-13 9:13 ` Leo Yan
2022-04-13 9:14 ` German Gomez
2022-04-14 1:24 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 7+ messages in thread
From: Leo Yan @ 2022-04-13 9:13 UTC (permalink / raw)
To: German Gomez
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ravi Bangoria, James Clark, linux-perf-users, linux-kernel
On Wed, Apr 13, 2022 at 04:49:41PM +0800, Leo Yan wrote:
> On Wed, Apr 13, 2022 at 09:15:40AM +0100, German Gomez wrote:
>
> [...]
>
> > > if (sort__mode == SORT_MODE__MEMORY) {
> > > + /*
> > > + * FIXUP: prior to kernel 5.18, Arm SPE missed to set
> > > + * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
> > > + * compatibility, set the bit if it's an old perf data file.
> > > + */
> > > + evlist__for_each_entry(session->evlist, evsel) {
> > > + if (strstr(evsel->name, "arm_spe_") &&
> >
> > This didn't work for me when the file recorded "-e arm_spe//" instead of
> > "-e arm_spe_0//". Could you remove the trailing _? With that:
>
> Sure, will change to "arm_spe". Just curious, if there any local
> change at your side so we have the different event name?
I think I know the reason now, though the PMU event is 'arm_spe_0',
but we could use '-e arm_spe//' to record, so finally the event name
is 'arm_spe'.
Thanks,
Leo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
2022-04-13 8:49 ` Leo Yan
2022-04-13 9:13 ` Leo Yan
@ 2022-04-13 9:14 ` German Gomez
2022-04-13 9:16 ` Leo Yan
2022-04-14 1:24 ` Arnaldo Carvalho de Melo
2 siblings, 1 reply; 7+ messages in thread
From: German Gomez @ 2022-04-13 9:14 UTC (permalink / raw)
To: Leo Yan
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ravi Bangoria, James Clark, linux-perf-users, linux-kernel
On 13/04/2022 09:49, Leo Yan wrote:
> On Wed, Apr 13, 2022 at 09:15:40AM +0100, German Gomez wrote:
>
> [...]
>
>>> if (sort__mode == SORT_MODE__MEMORY) {
>>> + /*
>>> + * FIXUP: prior to kernel 5.18, Arm SPE missed to set
>>> + * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
>>> + * compatibility, set the bit if it's an old perf data file.
>>> + */
>>> + evlist__for_each_entry(session->evlist, evsel) {
>>> + if (strstr(evsel->name, "arm_spe_") &&
>> This didn't work for me when the file recorded "-e arm_spe//" instead of
>> "-e arm_spe_0//". Could you remove the trailing _? With that:
> Sure, will change to "arm_spe". Just curious, if there any local
> change at your side so we have the different event name?
No local changes. I've always used "arm_spe//" and it always defaults to
"arm_spe_0//". But it's still stored as the former in the data file. I
haven't checked where this default happens though. Isn't it the same for
you?
Thanks,
German
>
>> Tested-by: German Gomez <german.gomez@arm.com>
> Thanks a lot, German!
>
> Leo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
2022-04-13 9:14 ` German Gomez
@ 2022-04-13 9:16 ` Leo Yan
0 siblings, 0 replies; 7+ messages in thread
From: Leo Yan @ 2022-04-13 9:16 UTC (permalink / raw)
To: German Gomez
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ravi Bangoria, James Clark, linux-perf-users, linux-kernel
On Wed, Apr 13, 2022 at 10:14:00AM +0100, German Gomez wrote:
>
> On 13/04/2022 09:49, Leo Yan wrote:
> > On Wed, Apr 13, 2022 at 09:15:40AM +0100, German Gomez wrote:
> >
> > [...]
> >
> >>> if (sort__mode == SORT_MODE__MEMORY) {
> >>> + /*
> >>> + * FIXUP: prior to kernel 5.18, Arm SPE missed to set
> >>> + * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
> >>> + * compatibility, set the bit if it's an old perf data file.
> >>> + */
> >>> + evlist__for_each_entry(session->evlist, evsel) {
> >>> + if (strstr(evsel->name, "arm_spe_") &&
> >> This didn't work for me when the file recorded "-e arm_spe//" instead of
> >> "-e arm_spe_0//". Could you remove the trailing _? With that:
> > Sure, will change to "arm_spe". Just curious, if there any local
> > change at your side so we have the different event name?
>
> No local changes. I've always used "arm_spe//" and it always defaults to
> "arm_spe_0//". But it's still stored as the former in the data file. I
> haven't checked where this default happens though. Isn't it the same for
> you?
Yeah, this is same with me. Thanks for explanation.
Leo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event
2022-04-13 8:49 ` Leo Yan
2022-04-13 9:13 ` Leo Yan
2022-04-13 9:14 ` German Gomez
@ 2022-04-14 1:24 ` Arnaldo Carvalho de Melo
2 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-04-14 1:24 UTC (permalink / raw)
To: Leo Yan
Cc: German Gomez, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Ravi Bangoria,
James Clark, linux-perf-users, linux-kernel
Em Wed, Apr 13, 2022 at 04:49:41PM +0800, Leo Yan escreveu:
> On Wed, Apr 13, 2022 at 09:15:40AM +0100, German Gomez wrote:
>
> [...]
>
> > > if (sort__mode == SORT_MODE__MEMORY) {
> > > + /*
> > > + * FIXUP: prior to kernel 5.18, Arm SPE missed to set
> > > + * PERF_SAMPLE_DATA_SRC bit in sample type. For backward
> > > + * compatibility, set the bit if it's an old perf data file.
> > > + */
> > > + evlist__for_each_entry(session->evlist, evsel) {
> > > + if (strstr(evsel->name, "arm_spe_") &&
> >
> > This didn't work for me when the file recorded "-e arm_spe//" instead of
> > "-e arm_spe_0//". Could you remove the trailing _? With that:
>
> Sure, will change to "arm_spe". Just curious, if there any local
> change at your side so we have the different event name?
Ok, waiting for v2
> > Tested-by: German Gomez <german.gomez@arm.com>
>
> Thanks a lot, German!
>
> Leo
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-04-14 1:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-13 7:51 [PATCH] perf report: Set PERF_SAMPLE_DATA_SRC bit for Arm SPE event Leo Yan
2022-04-13 8:15 ` German Gomez
2022-04-13 8:49 ` Leo Yan
2022-04-13 9:13 ` Leo Yan
2022-04-13 9:14 ` German Gomez
2022-04-13 9:16 ` Leo Yan
2022-04-14 1:24 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).