| Message ID | 20220408144056.1955535-1-james.clark@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf: arm-spe: Fix perf report --mem-mode | expand |
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 >
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
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
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
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; }
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(-)