[PATCH] perf: arm-spe: Fix perf report --mem-mode

[PATCH] perf: arm-spe: Fix perf report --mem-mode

[James Clark]

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
allow opening the file unless one of the events has PERF_SAMPLE_DATA_SRC
set.

SPE doesn't have this set even though synthetic memory data is generated
after it is decoded. Fix this issue by setting DATA_SRC on SPE events.
This has no effect on the data collected because the SPE driver doesn't
do anything with that flag and doesn't generate samples.

Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/arch/arm64/util/arm-spe.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 86e2e926aa0e..a1419e6341c0 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -239,6 +239,12 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 		arm_spe_set_timestamp(itr, arm_spe_evsel);
 	}
 
+	/*
+	 * Set this only so that perf report knows that SPE generates memory info. It has no effect
+	 * on the opening of the event or the SPE data produced.
+	 */
+	evsel__set_sample_bit(arm_spe_evsel, DATA_SRC);
+
 	/* Add dummy event to keep tracking */
 	err = parse_events(evlist, "dummy:u", NULL);
 	if (err)
@@ -523,6 +529,5 @@ struct perf_event_attr
 
 	arm_spe_pmu->selectable = true;
 	arm_spe_pmu->is_uncore = false;
-
 	return attr;
 }
-- 
2.28.0

Re: [PATCH] perf: arm-spe: Fix perf report --mem-mode

[Namhyung Kim]

Hello,

On Fri, Apr 8, 2022 at 7:41 AM James Clark <james.clark@arm.com> 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
> allow opening the file unless one of the events has PERF_SAMPLE_DATA_SRC
> set.
>
> SPE doesn't have this set even though synthetic memory data is generated
> after it is decoded. Fix this issue by setting DATA_SRC on SPE events.
> This has no effect on the data collected because the SPE driver doesn't
> do anything with that flag and doesn't generate samples.
>
> Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
> Signed-off-by: James Clark <james.clark@arm.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> ---
>  tools/perf/arch/arm64/util/arm-spe.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> index 86e2e926aa0e..a1419e6341c0 100644
> --- a/tools/perf/arch/arm64/util/arm-spe.c
> +++ b/tools/perf/arch/arm64/util/arm-spe.c
> @@ -239,6 +239,12 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
>                 arm_spe_set_timestamp(itr, arm_spe_evsel);
>         }
>
> +       /*
> +        * Set this only so that perf report knows that SPE generates memory info. It has no effect
> +        * on the opening of the event or the SPE data produced.
> +        */
> +       evsel__set_sample_bit(arm_spe_evsel, DATA_SRC);
> +
>         /* Add dummy event to keep tracking */
>         err = parse_events(evlist, "dummy:u", NULL);
>         if (err)
> @@ -523,6 +529,5 @@ struct perf_event_attr
>
>         arm_spe_pmu->selectable = true;
>         arm_spe_pmu->is_uncore = false;
> -
>         return attr;
>  }
> --
> 2.28.0
>

Re: [PATCH] perf: arm-spe: Fix perf report --mem-mode

[Leo Yan]

On Fri, Apr 08, 2022 at 11:13:09AM -0700, Namhyung Kim wrote:
> Hello,
> 
> On Fri, Apr 8, 2022 at 7:41 AM James Clark <james.clark@arm.com> 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
> > allow opening the file unless one of the events has PERF_SAMPLE_DATA_SRC
> > set.
> >
> > SPE doesn't have this set even though synthetic memory data is generated
> > after it is decoded. Fix this issue by setting DATA_SRC on SPE events.
> > This has no effect on the data collected because the SPE driver doesn't
> > do anything with that flag and doesn't generate samples.
> >
> > Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Tested-by: Leo Yan <leo.yan@linaro.org>

It's a bit awkward that after the commit bb30acae4c4d we cannot be
backward compatible and any old perf data file will be failed to
support by 'perf mem report' tool.

Thanks,
Leo

Re: [PATCH] perf: arm-spe: Fix perf report --mem-mode

[Arnaldo Carvalho de Melo]

Em Sat, Apr 09, 2022 at 05:59:41PM +0800, Leo Yan escreveu:
> On Fri, Apr 08, 2022 at 11:13:09AM -0700, Namhyung Kim wrote:
> > Hello,
> > 
> > On Fri, Apr 8, 2022 at 7:41 AM James Clark <james.clark@arm.com> 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
> > > allow opening the file unless one of the events has PERF_SAMPLE_DATA_SRC
> > > set.
> > >
> > > SPE doesn't have this set even though synthetic memory data is generated
> > > after it is decoded. Fix this issue by setting DATA_SRC on SPE events.
> > > This has no effect on the data collected because the SPE driver doesn't
> > > do anything with that flag and doesn't generate samples.
> > >
> > > Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
> > > Signed-off-by: James Clark <james.clark@arm.com>
> > 
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Tested-by: Leo Yan <leo.yan@linaro.org>
> 
> It's a bit awkward that after the commit bb30acae4c4d we cannot be
> backward compatible and any old perf data file will be failed to
> support by 'perf mem report' tool.

oh well, I think we should put in place a mechanism to run perf record
before some change, then make sure that a new perf is able to process
it after a rebuild.

If its something unsurmountable, then a proper explanation should be
done with a workaround to process those older files, even if pointing
out  to the perf version that is able to process the old file.

Anyway, applying this pa tch.

- ARnaldo

Re: [PATCH] perf: arm-spe: Fix perf report --mem-mode

[Leo Yan]

On Sat, Apr 09, 2022 at 12:12:37PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Apr 09, 2022 at 05:59:41PM +0800, Leo Yan escreveu:
> > On Fri, Apr 08, 2022 at 11:13:09AM -0700, Namhyung Kim wrote:
> > > Hello,
> > > 
> > > On Fri, Apr 8, 2022 at 7:41 AM James Clark <james.clark@arm.com> 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
> > > > allow opening the file unless one of the events has PERF_SAMPLE_DATA_SRC
> > > > set.
> > > >
> > > > SPE doesn't have this set even though synthetic memory data is generated
> > > > after it is decoded. Fix this issue by setting DATA_SRC on SPE events.
> > > > This has no effect on the data collected because the SPE driver doesn't
> > > > do anything with that flag and doesn't generate samples.
> > > >
> > > > Fixes: bb30acae4c4d ("perf report: Bail out --mem-mode if mem info is not available")
> > > > Signed-off-by: James Clark <james.clark@arm.com>
> > > 
> > > Acked-by: Namhyung Kim <namhyung@kernel.org>
> > 
> > Tested-by: Leo Yan <leo.yan@linaro.org>
> > 
> > It's a bit awkward that after the commit bb30acae4c4d we cannot be
> > backward compatible and any old perf data file will be failed to
> > support by 'perf mem report' tool.
> 
> oh well, I think we should put in place a mechanism to run perf record
> before some change, then make sure that a new perf is able to process
> it after a rebuild.
> 
> If its something unsurmountable, then a proper explanation should be
> done with a workaround to process those older files, even if pointing
> out  to the perf version that is able to process the old file.

I will follow up to add a sub test case for Arm SPE with 'perf mem'.

I tried to find possible workround for old perf data files but failed,
the 'perf mem' tool reports error in a very early time so I cannot add
workaround in Arm SPE specific code (e.g. arm_spe_process_auxtrace_info()).
I will check furthermore, as a backup will send a patch to remind
suitable perf version.

A backlog task for me is to setup an automatic testing envoirnment
for daily testing perf on Arm64.  I will setup an automatic testing
for this.

> Anyway, applying this pa tch.

Thanks!

Leo