| Message ID | 20211102180739.18049-2-german.gomez@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf arm-spe: Track pid/tid for Arm SPE samples | expand |
Hi German, On Tue, Nov 02, 2021 at 06:07:37PM +0000, German Gomez wrote: > When perf report synthesize events from ARM SPE data, it refers to > current cpu, pid and tid in the machine. But there's no place to set > them in the ARM SPE decoder. I'm seeing all pid/tid is set to -1 and > user symbols are not resolved in the output. > > # perf record -a -e arm_spe_0/ts_enable=1/ sleep 1 > > # perf report -q | head > 8.77% 8.77% :-1 [kernel.kallsyms] [k] format_decode > 7.02% 7.02% :-1 [kernel.kallsyms] [k] seq_printf > 7.02% 7.02% :-1 [unknown] [.] 0x0000ffff9f687c34 > 5.26% 5.26% :-1 [kernel.kallsyms] [k] vsnprintf > 3.51% 3.51% :-1 [kernel.kallsyms] [k] string > 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f66ae20 > 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f670b3c > 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f67c040 > 1.75% 1.75% :-1 [kernel.kallsyms] [k] ___cache_free > 1.75% 1.75% :-1 [kernel.kallsyms] [k] > __count_memcg_events > > Like Intel PT, add context switch records to track task info. As ARM > SPE support was added later than PERF_RECORD_SWITCH_CPU_WIDE, I think > we can safely set the attr.context_switch bit and use it. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > Signed-off-by: German Gomez <german.gomez@arm.com> Reviewed-by: Leo Yan <leo.yan@linaro.org> Note for one thing, please keep "Namhyung Kim" as the author for this patch, thanks. Leo > --- > tools/perf/arch/arm64/util/arm-spe.c | 6 +++++- > tools/perf/util/arm-spe.c | 25 +++++++++++++++++++++++++ > 2 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c > index a4420d4df..58ba8d15c 100644 > --- a/tools/perf/arch/arm64/util/arm-spe.c > +++ b/tools/perf/arch/arm64/util/arm-spe.c > @@ -166,8 +166,12 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > 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)) > + if (!perf_cpu_map__empty(cpus)) { > evsel__set_sample_bit(tracking_evsel, TIME); > + evsel__set_sample_bit(tracking_evsel, CPU); > + /* also track task context switch */ > + tracking_evsel->core.attr.context_switch = 1; > + } > > return 0; > } > diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c > index 58b7069c5..230bc7ab2 100644 > --- a/tools/perf/util/arm-spe.c > +++ b/tools/perf/util/arm-spe.c > @@ -681,6 +681,25 @@ static int arm_spe_process_timeless_queues(struct arm_spe *spe, pid_t tid, > return 0; > } > > +static int arm_spe_context_switch(struct arm_spe *spe, union perf_event *event, > + struct perf_sample *sample) > +{ > + pid_t pid, tid; > + int cpu; > + > + if (!(event->header.misc & PERF_RECORD_MISC_SWITCH_OUT)) > + return 0; > + > + pid = event->context_switch.next_prev_pid; > + tid = event->context_switch.next_prev_tid; > + cpu = sample->cpu; > + > + if (tid == -1) > + pr_warning("context_switch event has no tid\n"); > + > + return machine__set_current_tid(spe->machine, cpu, pid, tid); > +} > + > static int arm_spe_process_event(struct perf_session *session, > union perf_event *event, > struct perf_sample *sample, > @@ -718,6 +737,12 @@ static int arm_spe_process_event(struct perf_session *session, > } > } else if (timestamp) { > err = arm_spe_process_queues(spe, timestamp); > + if (err) > + return err; > + > + if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE || > + event->header.type == PERF_RECORD_SWITCH) > + err = arm_spe_context_switch(spe, event, sample); > } > > return err; > -- > 2.25.1 >
Em Sat, Nov 06, 2021 at 11:29:07AM +0800, Leo Yan escreveu: > Hi German, > > On Tue, Nov 02, 2021 at 06:07:37PM +0000, German Gomez wrote: > > When perf report synthesize events from ARM SPE data, it refers to > > current cpu, pid and tid in the machine. But there's no place to set > > them in the ARM SPE decoder. I'm seeing all pid/tid is set to -1 and > > user symbols are not resolved in the output. > > > > # perf record -a -e arm_spe_0/ts_enable=1/ sleep 1 > > > > # perf report -q | head > > 8.77% 8.77% :-1 [kernel.kallsyms] [k] format_decode > > 7.02% 7.02% :-1 [kernel.kallsyms] [k] seq_printf > > 7.02% 7.02% :-1 [unknown] [.] 0x0000ffff9f687c34 > > 5.26% 5.26% :-1 [kernel.kallsyms] [k] vsnprintf > > 3.51% 3.51% :-1 [kernel.kallsyms] [k] string > > 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f66ae20 > > 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f670b3c > > 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f67c040 > > 1.75% 1.75% :-1 [kernel.kallsyms] [k] ___cache_free > > 1.75% 1.75% :-1 [kernel.kallsyms] [k] > > __count_memcg_events > > > > Like Intel PT, add context switch records to track task info. As ARM > > SPE support was added later than PERF_RECORD_SWITCH_CPU_WIDE, I think > > we can safely set the attr.context_switch bit and use it. > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > Signed-off-by: German Gomez <german.gomez@arm.com> > > Reviewed-by: Leo Yan <leo.yan@linaro.org> > > Note for one thing, please keep "Namhyung Kim" as the author for this > patch, thanks. This merits a v2 submission, please do so. - Arnaldo > Leo > > > --- > > tools/perf/arch/arm64/util/arm-spe.c | 6 +++++- > > tools/perf/util/arm-spe.c | 25 +++++++++++++++++++++++++ > > 2 files changed, 30 insertions(+), 1 deletion(-) > > > > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c > > index a4420d4df..58ba8d15c 100644 > > --- a/tools/perf/arch/arm64/util/arm-spe.c > > +++ b/tools/perf/arch/arm64/util/arm-spe.c > > @@ -166,8 +166,12 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > > 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)) > > + if (!perf_cpu_map__empty(cpus)) { > > evsel__set_sample_bit(tracking_evsel, TIME); > > + evsel__set_sample_bit(tracking_evsel, CPU); > > + /* also track task context switch */ > > + tracking_evsel->core.attr.context_switch = 1; > > + } > > > > return 0; > > } > > diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c > > index 58b7069c5..230bc7ab2 100644 > > --- a/tools/perf/util/arm-spe.c > > +++ b/tools/perf/util/arm-spe.c > > @@ -681,6 +681,25 @@ static int arm_spe_process_timeless_queues(struct arm_spe *spe, pid_t tid, > > return 0; > > } > > > > +static int arm_spe_context_switch(struct arm_spe *spe, union perf_event *event, > > + struct perf_sample *sample) > > +{ > > + pid_t pid, tid; > > + int cpu; > > + > > + if (!(event->header.misc & PERF_RECORD_MISC_SWITCH_OUT)) > > + return 0; > > + > > + pid = event->context_switch.next_prev_pid; > > + tid = event->context_switch.next_prev_tid; > > + cpu = sample->cpu; > > + > > + if (tid == -1) > > + pr_warning("context_switch event has no tid\n"); > > + > > + return machine__set_current_tid(spe->machine, cpu, pid, tid); > > +} > > + > > static int arm_spe_process_event(struct perf_session *session, > > union perf_event *event, > > struct perf_sample *sample, > > @@ -718,6 +737,12 @@ static int arm_spe_process_event(struct perf_session *session, > > } > > } else if (timestamp) { > > err = arm_spe_process_queues(spe, timestamp); > > + if (err) > > + return err; > > + > > + if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE || > > + event->header.type == PERF_RECORD_SWITCH) > > + err = arm_spe_context_switch(spe, event, sample); > > } > > > > return err; > > -- > > 2.25.1 > >
On 06/11/2021 19:49, Arnaldo Carvalho de Melo wrote: > Em Sat, Nov 06, 2021 at 11:29:07AM +0800, Leo Yan escreveu: >> [...] >> Reviewed-by: Leo Yan <leo.yan@linaro.org> >> >> Note for one thing, please keep "Namhyung Kim" as the author for this >> patch, thanks. > This merits a v2 submission, please do so. > > - Arnaldo > Will do, Also will fix the authorship of [2/3]. Thanks, German >> Leo >> >>> --- >>> tools/perf/arch/arm64/util/arm-spe.c | 6 +++++- >>> tools/perf/util/arm-spe.c | 25 +++++++++++++++++++++++++ >>> 2 files changed, 30 insertions(+), 1 deletion(-) >>> >>> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c >>> index a4420d4df..58ba8d15c 100644 >>> --- a/tools/perf/arch/arm64/util/arm-spe.c >>> +++ b/tools/perf/arch/arm64/util/arm-spe.c >>> @@ -166,8 +166,12 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, >>> 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)) >>> + if (!perf_cpu_map__empty(cpus)) { >>> evsel__set_sample_bit(tracking_evsel, TIME); >>> + evsel__set_sample_bit(tracking_evsel, CPU); >>> + /* also track task context switch */ >>> + tracking_evsel->core.attr.context_switch = 1; >>> + } >>> >>> return 0; >>> } >>> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c >>> index 58b7069c5..230bc7ab2 100644 >>> --- a/tools/perf/util/arm-spe.c >>> +++ b/tools/perf/util/arm-spe.c >>> @@ -681,6 +681,25 @@ static int arm_spe_process_timeless_queues(struct arm_spe *spe, pid_t tid, >>> return 0; >>> } >>> >>> +static int arm_spe_context_switch(struct arm_spe *spe, union perf_event *event, >>> + struct perf_sample *sample) >>> +{ >>> + pid_t pid, tid; >>> + int cpu; >>> + >>> + if (!(event->header.misc & PERF_RECORD_MISC_SWITCH_OUT)) >>> + return 0; >>> + >>> + pid = event->context_switch.next_prev_pid; >>> + tid = event->context_switch.next_prev_tid; >>> + cpu = sample->cpu; >>> + >>> + if (tid == -1) >>> + pr_warning("context_switch event has no tid\n"); >>> + >>> + return machine__set_current_tid(spe->machine, cpu, pid, tid); >>> +} >>> + >>> static int arm_spe_process_event(struct perf_session *session, >>> union perf_event *event, >>> struct perf_sample *sample, >>> @@ -718,6 +737,12 @@ static int arm_spe_process_event(struct perf_session *session, >>> } >>> } else if (timestamp) { >>> err = arm_spe_process_queues(spe, timestamp); >>> + if (err) >>> + return err; >>> + >>> + if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE || >>> + event->header.type == PERF_RECORD_SWITCH) >>> + err = arm_spe_context_switch(spe, event, sample); >>> } >>> >>> return err; >>> -- >>> 2.25.1 >>>
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c index a4420d4df..58ba8d15c 100644 --- a/tools/perf/arch/arm64/util/arm-spe.c +++ b/tools/perf/arch/arm64/util/arm-spe.c @@ -166,8 +166,12 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, 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)) + if (!perf_cpu_map__empty(cpus)) { evsel__set_sample_bit(tracking_evsel, TIME); + evsel__set_sample_bit(tracking_evsel, CPU); + /* also track task context switch */ + tracking_evsel->core.attr.context_switch = 1; + } return 0; } diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c index 58b7069c5..230bc7ab2 100644 --- a/tools/perf/util/arm-spe.c +++ b/tools/perf/util/arm-spe.c @@ -681,6 +681,25 @@ static int arm_spe_process_timeless_queues(struct arm_spe *spe, pid_t tid, return 0; } +static int arm_spe_context_switch(struct arm_spe *spe, union perf_event *event, + struct perf_sample *sample) +{ + pid_t pid, tid; + int cpu; + + if (!(event->header.misc & PERF_RECORD_MISC_SWITCH_OUT)) + return 0; + + pid = event->context_switch.next_prev_pid; + tid = event->context_switch.next_prev_tid; + cpu = sample->cpu; + + if (tid == -1) + pr_warning("context_switch event has no tid\n"); + + return machine__set_current_tid(spe->machine, cpu, pid, tid); +} + static int arm_spe_process_event(struct perf_session *session, union perf_event *event, struct perf_sample *sample, @@ -718,6 +737,12 @@ static int arm_spe_process_event(struct perf_session *session, } } else if (timestamp) { err = arm_spe_process_queues(spe, timestamp); + if (err) + return err; + + if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE || + event->header.type == PERF_RECORD_SWITCH) + err = arm_spe_context_switch(spe, event, sample); } return err;
When perf report synthesize events from ARM SPE data, it refers to current cpu, pid and tid in the machine. But there's no place to set them in the ARM SPE decoder. I'm seeing all pid/tid is set to -1 and user symbols are not resolved in the output. # perf record -a -e arm_spe_0/ts_enable=1/ sleep 1 # perf report -q | head 8.77% 8.77% :-1 [kernel.kallsyms] [k] format_decode 7.02% 7.02% :-1 [kernel.kallsyms] [k] seq_printf 7.02% 7.02% :-1 [unknown] [.] 0x0000ffff9f687c34 5.26% 5.26% :-1 [kernel.kallsyms] [k] vsnprintf 3.51% 3.51% :-1 [kernel.kallsyms] [k] string 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f66ae20 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f670b3c 3.51% 3.51% :-1 [unknown] [.] 0x0000ffff9f67c040 1.75% 1.75% :-1 [kernel.kallsyms] [k] ___cache_free 1.75% 1.75% :-1 [kernel.kallsyms] [k] __count_memcg_events Like Intel PT, add context switch records to track task info. As ARM SPE support was added later than PERF_RECORD_SWITCH_CPU_WIDE, I think we can safely set the attr.context_switch bit and use it. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Signed-off-by: German Gomez <german.gomez@arm.com> --- tools/perf/arch/arm64/util/arm-spe.c | 6 +++++- tools/perf/util/arm-spe.c | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-)