From: James Clark <james.clark@arm.com> To: Leo Yan <leo.yan@linaro.org>, Arnaldo Carvalho de Melo <acme@kernel.org>, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 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, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, Al Grant <Al.Grant@arm.com> Subject: Re: [PATCH v1 2/3] perf arm-spe: Correct sample flags for dummy event Date: Wed, 12 May 2021 17:39:56 +0300 [thread overview] Message-ID: <f4e483ae-acbb-7afa-c215-cb4244c2e820@arm.com> (raw) In-Reply-To: <20210429150100.282180-3-leo.yan@linaro.org> On 29/04/2021 18:00, Leo Yan wrote: > The dummy event is mainly used for mmap, the TIME sample is only needed > for per-cpu case so that the perf tool can rely on the correct timing > for parsing symbols. And the CPU sample is useless for mmap. > > This patch enables TIME sample for per-cpu mmap and doesn't enable CPU > sample. For later extension (e.g. support multiple AUX events), it sets > the dummy event when the condition "opts->full_auxtrace" is true. > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > tools/perf/arch/arm64/util/arm-spe.c | 30 ++++++++++++++++------------ > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c > index 902e73a64184..f6eec0900604 100644 > --- a/tools/perf/arch/arm64/util/arm-spe.c > +++ b/tools/perf/arch/arm64/util/arm-spe.c > @@ -70,7 +70,6 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > struct evsel *evsel, *arm_spe_evsel = NULL; > struct perf_cpu_map *cpus = evlist->core.cpus; > bool privileged = perf_event_paranoid_check(-1); > - struct evsel *tracking_evsel; > int err; > > sper->evlist = evlist; > @@ -126,18 +125,23 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > evsel__set_sample_bit(arm_spe_evsel, CPU); > > /* Add dummy event to keep tracking */ > - err = parse_events(evlist, "dummy:u", NULL); > - if (err) > - return err; > - > - tracking_evsel = evlist__last(evlist); > - evlist__set_tracking_event(evlist, tracking_evsel); > - > - tracking_evsel->core.attr.freq = 0; > - tracking_evsel->core.attr.sample_period = 1; > - evsel__set_sample_bit(tracking_evsel, TIME); > - evsel__set_sample_bit(tracking_evsel, CPU); > - evsel__reset_sample_bit(tracking_evsel, BRANCH_STACK); > + if (opts->full_auxtrace) { > + struct evsel *tracking_evsel; Hi Leo, I know the "if (opts->full_auxtrace)" pattern is copied from other auxtrace files, but I don't think it does anything because there is this at the top of the function: if (!opts->full_auxtrace) return 0; The same applies for other usages of "full_auxtrace" in the same function. They are all always true. I'm also not sure if it's ever defined what full_auxtrace means. James > + > + err = parse_events(evlist, "dummy:u", NULL); > + if (err) > + return err; > + > + tracking_evsel = evlist__last(evlist); > + evlist__set_tracking_event(evlist, tracking_evsel); > + > + tracking_evsel->core.attr.freq = 0; > + tracking_evsel->core.attr.sample_period = 1; > + > + /* In per-cpu case, always need the time of mmap events etc */ > + if (!perf_cpu_map__empty(cpus)) > + evsel__set_sample_bit(tracking_evsel, TIME); > + } > > return 0; > } >
WARNING: multiple messages have this Message-ID (diff)
From: James Clark <james.clark@arm.com> To: Leo Yan <leo.yan@linaro.org>, Arnaldo Carvalho de Melo <acme@kernel.org>, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 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, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, Al Grant <Al.Grant@arm.com> Subject: Re: [PATCH v1 2/3] perf arm-spe: Correct sample flags for dummy event Date: Wed, 12 May 2021 17:39:56 +0300 [thread overview] Message-ID: <f4e483ae-acbb-7afa-c215-cb4244c2e820@arm.com> (raw) In-Reply-To: <20210429150100.282180-3-leo.yan@linaro.org> On 29/04/2021 18:00, Leo Yan wrote: > The dummy event is mainly used for mmap, the TIME sample is only needed > for per-cpu case so that the perf tool can rely on the correct timing > for parsing symbols. And the CPU sample is useless for mmap. > > This patch enables TIME sample for per-cpu mmap and doesn't enable CPU > sample. For later extension (e.g. support multiple AUX events), it sets > the dummy event when the condition "opts->full_auxtrace" is true. > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > tools/perf/arch/arm64/util/arm-spe.c | 30 ++++++++++++++++------------ > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c > index 902e73a64184..f6eec0900604 100644 > --- a/tools/perf/arch/arm64/util/arm-spe.c > +++ b/tools/perf/arch/arm64/util/arm-spe.c > @@ -70,7 +70,6 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > struct evsel *evsel, *arm_spe_evsel = NULL; > struct perf_cpu_map *cpus = evlist->core.cpus; > bool privileged = perf_event_paranoid_check(-1); > - struct evsel *tracking_evsel; > int err; > > sper->evlist = evlist; > @@ -126,18 +125,23 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > evsel__set_sample_bit(arm_spe_evsel, CPU); > > /* Add dummy event to keep tracking */ > - err = parse_events(evlist, "dummy:u", NULL); > - if (err) > - return err; > - > - tracking_evsel = evlist__last(evlist); > - evlist__set_tracking_event(evlist, tracking_evsel); > - > - tracking_evsel->core.attr.freq = 0; > - tracking_evsel->core.attr.sample_period = 1; > - evsel__set_sample_bit(tracking_evsel, TIME); > - evsel__set_sample_bit(tracking_evsel, CPU); > - evsel__reset_sample_bit(tracking_evsel, BRANCH_STACK); > + if (opts->full_auxtrace) { > + struct evsel *tracking_evsel; Hi Leo, I know the "if (opts->full_auxtrace)" pattern is copied from other auxtrace files, but I don't think it does anything because there is this at the top of the function: if (!opts->full_auxtrace) return 0; The same applies for other usages of "full_auxtrace" in the same function. They are all always true. I'm also not sure if it's ever defined what full_auxtrace means. James > + > + err = parse_events(evlist, "dummy:u", NULL); > + if (err) > + return err; > + > + tracking_evsel = evlist__last(evlist); > + evlist__set_tracking_event(evlist, tracking_evsel); > + > + tracking_evsel->core.attr.freq = 0; > + tracking_evsel->core.attr.sample_period = 1; > + > + /* In per-cpu case, always need the time of mmap events etc */ > + if (!perf_cpu_map__empty(cpus)) > + evsel__set_sample_bit(tracking_evsel, TIME); > + } > > return 0; > } > _______________________________________________ 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-05-12 14:40 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-29 15:00 [PATCH v1 0/3] perf arm-spe: Correct recording configurations Leo Yan 2021-04-29 15:00 ` Leo Yan 2021-04-29 15:00 ` [PATCH v1 1/3] perf arm-spe: Correct sample flags for SPE event Leo Yan 2021-04-29 15:00 ` Leo Yan 2021-04-29 15:00 ` [PATCH v1 2/3] perf arm-spe: Correct sample flags for dummy event Leo Yan 2021-04-29 15:00 ` Leo Yan 2021-05-12 14:39 ` James Clark [this message] 2021-05-12 14:39 ` James Clark 2021-05-12 15:17 ` James Clark 2021-05-12 15:17 ` James Clark 2021-05-12 15:37 ` Leo Yan 2021-05-12 15:37 ` Leo Yan 2021-05-12 15:23 ` Leo Yan 2021-05-12 15:23 ` Leo Yan 2021-05-18 12:54 ` Arnaldo Carvalho de Melo 2021-05-18 12:54 ` Arnaldo Carvalho de Melo 2021-05-18 14:18 ` Leo Yan 2021-05-18 14:18 ` Leo Yan 2021-04-29 15:01 ` [PATCH v1 3/3] perf arm-spe: Enable timestamp for per-cpu mode Leo Yan 2021-04-29 15:01 ` Leo Yan
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=f4e483ae-acbb-7afa-c215-cb4244c2e820@arm.com \ --to=james.clark@arm.com \ --cc=Al.Grant@arm.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.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=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.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.