2021-11-11 13:36:46

by German Gomez

[permalink] [raw]
Subject: [PATCH v3 0/4] perf arm-spe: Track pid/tid for Arm SPE samples

The following patchset is an iteration on RFC [1] where pid/tid info is
assigned to the Arm SPE synthesized samples. Two methods of tracking
pids are considered: hardware-based (using Arm SPE CONTEXT packets), and
context-switch events (from perf) as fallback.

- Patch #1 enables pid tracking using RECORD_SWITCH* events from perf.
- Patch #2 updates perf-record documentation and arm-spe recording so
that they are consistent.
- Patch #3 saves the value of SPE CONTEXT packet to the arm_spe_record
struct.
- Patch #4 enables hardware-based pid tracking using SPE CONTEXT
packets.

---

Changes since v2:

- [PATCH 4/4] Set pid to '-1' in hardware-based pid&tid tracking.

Changes since v1:

- [PATCH 1/4] Fix authorship of commit.
- [PATCH 2/4] (New patch) Updated perf-record docs to reflect the
behavior of Arm SPE introduced by the previous patch.
- [PATCH 3/4] update initialization of context_id field to (u64)-1.
- [PATCH 4/4] Update handling of pid/tid tracking fallback following
Leo Yan's suggestion. Don't consider per-thread mode on this patch.

[1] https://lore.kernel.org/lkml/[email protected]/T/#m8a9890e929d2eab54cd51296837ece5d1a473349

German Gomez (3):
perf arm-spe: Update --switch-events docs in perf-record
perf arm-spe: Save context ID in record
perf arm-spe: Support hardware-based PID tracing

Namhyung Kim (1):
perf arm-spe: Track task context switch for cpu-mode events

tools/perf/Documentation/perf-record.txt | 2 +-
tools/perf/arch/arm64/util/arm-spe.c | 8 +-
.../util/arm-spe-decoder/arm-spe-decoder.c | 2 +
.../util/arm-spe-decoder/arm-spe-decoder.h | 1 +
tools/perf/util/arm-spe.c | 120 ++++++++++++++----
5 files changed, 104 insertions(+), 29 deletions(-)

--
2.25.1



2021-11-11 13:36:52

by German Gomez

[permalink] [raw]
Subject: [PATCH v3 1/4] perf arm-spe: Track task context switch for cpu-mode events

From: Namhyung Kim <[email protected]>

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 <[email protected]>
Signed-off-by: German Gomez <[email protected]>
Reviewed-by: Leo Yan <[email protected]>
---
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


2021-11-11 13:36:53

by German Gomez

[permalink] [raw]
Subject: [PATCH v3 2/4] perf arm-spe: Update --switch-events docs in perf-record

Update perf-record docs and Arm SPE recording options so that they are
consistent. This includes supporting the --no-switch-events flag in Arm
SPE as well.

Signed-off-by: German Gomez <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Reviewed-by: Leo Yan <[email protected]>
---
tools/perf/Documentation/perf-record.txt | 2 +-
tools/perf/arch/arm64/util/arm-spe.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 2d7df8703..3cf7bac67 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -469,7 +469,7 @@ This option sets the time out limit. The default value is 500 ms.

--switch-events::
Record context switch events i.e. events of type PERF_RECORD_SWITCH or
-PERF_RECORD_SWITCH_CPU_WIDE. In some cases (e.g. Intel PT or CoreSight)
+PERF_RECORD_SWITCH_CPU_WIDE. In some cases (e.g. Intel PT, CoreSight or Arm SPE)
switch events will be enabled automatically, which can be suppressed by
by the option --no-switch-events.

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 58ba8d15c..725a06cd2 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -169,8 +169,10 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
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;
+ if (!record_opts__no_switch_events(opts))
+ tracking_evsel->core.attr.context_switch = 1;
}

return 0;
--
2.25.1


2021-11-11 13:36:59

by German Gomez

[permalink] [raw]
Subject: [PATCH v3 3/4] perf arm-spe: Save context ID in record

This patch is to save context ID in record, this will be used to set TID
for samples.

Signed-off-by: German Gomez <[email protected]>
Reviewed-by: Leo Yan <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
---
tools/perf/util/arm-spe-decoder/arm-spe-decoder.c | 2 ++
tools/perf/util/arm-spe-decoder/arm-spe-decoder.h | 1 +
2 files changed, 3 insertions(+)

diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
index 32fe41835..3fc528c92 100644
--- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
+++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
@@ -151,6 +151,7 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder)
u64 payload, ip;

memset(&decoder->record, 0x0, sizeof(decoder->record));
+ decoder->record.context_id = (u64)-1;

while (1) {
err = arm_spe_get_next_packet(decoder);
@@ -180,6 +181,7 @@ static int arm_spe_read_record(struct arm_spe_decoder *decoder)
case ARM_SPE_COUNTER:
break;
case ARM_SPE_CONTEXT:
+ decoder->record.context_id = payload;
break;
case ARM_SPE_OP_TYPE:
if (idx == SPE_OP_PKT_HDR_CLASS_LD_ST_ATOMIC) {
diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
index 59bdb7309..46a8556a9 100644
--- a/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
+++ b/tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
@@ -38,6 +38,7 @@ struct arm_spe_record {
u64 timestamp;
u64 virt_addr;
u64 phys_addr;
+ u64 context_id;
};

struct arm_spe_insn;
--
2.25.1


2021-11-11 13:37:09

by German Gomez

[permalink] [raw]
Subject: [PATCH v3 4/4] perf arm-spe: Support hardware-based PID tracing

If Arm SPE traces contain CONTEXT packets with TID info, use these
values for tracking tid of samples. Otherwise fall back to using context
switch events and display a message warning the user of possible timing
inaccuracies [1].

[1] https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: German Gomez <[email protected]>
---
tools/perf/util/arm-spe.c | 99 +++++++++++++++++++++++++++------------
1 file changed, 70 insertions(+), 29 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 230bc7ab2..6bb545ba0 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -71,6 +71,7 @@ struct arm_spe {
u64 kernel_start;

unsigned long num_events;
+ u8 use_ctx_pkt_for_pid;
};

struct arm_spe_queue {
@@ -226,6 +227,44 @@ static inline u8 arm_spe_cpumode(struct arm_spe *spe, u64 ip)
PERF_RECORD_MISC_USER;
}

+static void arm_spe_set_pid_tid_cpu(struct arm_spe *spe,
+ struct auxtrace_queue *queue)
+{
+ struct arm_spe_queue *speq = queue->priv;
+ pid_t tid;
+
+ tid = machine__get_current_tid(spe->machine, speq->cpu);
+ if (tid != -1) {
+ speq->tid = tid;
+ thread__zput(speq->thread);
+ } else
+ speq->tid = queue->tid;
+
+ if ((!speq->thread) && (speq->tid != -1)) {
+ speq->thread = machine__find_thread(spe->machine, -1,
+ speq->tid);
+ }
+
+ if (speq->thread) {
+ speq->pid = speq->thread->pid_;
+ if (queue->cpu == -1)
+ speq->cpu = speq->thread->cpu;
+ }
+}
+
+static int arm_spe_set_tid(struct arm_spe_queue *speq, pid_t tid)
+{
+ struct arm_spe *spe = speq->spe;
+ int err = machine__set_current_tid(spe->machine, speq->cpu, -1, tid);
+
+ if (err)
+ return err;
+
+ arm_spe_set_pid_tid_cpu(spe, &spe->queues.queue_array[speq->queue_nr]);
+
+ return 0;
+}
+
static void arm_spe_prep_sample(struct arm_spe *spe,
struct arm_spe_queue *speq,
union perf_event *event,
@@ -460,6 +499,19 @@ static int arm_spe_run_decoder(struct arm_spe_queue *speq, u64 *timestamp)
* can correlate samples between Arm SPE trace data and other
* perf events with correct time ordering.
*/
+
+ /*
+ * Update pid/tid info.
+ */
+ record = &speq->decoder->record;
+ if (!spe->timeless_decoding && record->context_id != (u64)-1) {
+ ret = arm_spe_set_tid(speq, record->context_id);
+ if (ret)
+ return ret;
+
+ spe->use_ctx_pkt_for_pid = true;
+ }
+
ret = arm_spe_sample(speq);
if (ret)
return ret;
@@ -586,31 +638,6 @@ static bool arm_spe__is_timeless_decoding(struct arm_spe *spe)
return timeless_decoding;
}

-static void arm_spe_set_pid_tid_cpu(struct arm_spe *spe,
- struct auxtrace_queue *queue)
-{
- struct arm_spe_queue *speq = queue->priv;
- pid_t tid;
-
- tid = machine__get_current_tid(spe->machine, speq->cpu);
- if (tid != -1) {
- speq->tid = tid;
- thread__zput(speq->thread);
- } else
- speq->tid = queue->tid;
-
- if ((!speq->thread) && (speq->tid != -1)) {
- speq->thread = machine__find_thread(spe->machine, -1,
- speq->tid);
- }
-
- if (speq->thread) {
- speq->pid = speq->thread->pid_;
- if (queue->cpu == -1)
- speq->cpu = speq->thread->cpu;
- }
-}
-
static int arm_spe_process_queues(struct arm_spe *spe, u64 timestamp)
{
unsigned int queue_nr;
@@ -641,7 +668,12 @@ static int arm_spe_process_queues(struct arm_spe *spe, u64 timestamp)
ts = timestamp;
}

- arm_spe_set_pid_tid_cpu(spe, queue);
+ /*
+ * A previous context-switch event has set pid/tid in the machine's context, so
+ * here we need to update the pid/tid in the thread and SPE queue.
+ */
+ if (!spe->use_ctx_pkt_for_pid)
+ arm_spe_set_pid_tid_cpu(spe, queue);

ret = arm_spe_run_decoder(speq, &ts);
if (ret < 0) {
@@ -740,8 +772,9 @@ static int arm_spe_process_event(struct perf_session *session,
if (err)
return err;

- if (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE ||
- event->header.type == PERF_RECORD_SWITCH)
+ if (!spe->use_ctx_pkt_for_pid &&
+ (event->header.type == PERF_RECORD_SWITCH_CPU_WIDE ||
+ event->header.type == PERF_RECORD_SWITCH))
err = arm_spe_context_switch(spe, event, sample);
}

@@ -808,7 +841,15 @@ static int arm_spe_flush(struct perf_session *session __maybe_unused,
return arm_spe_process_timeless_queues(spe, -1,
MAX_TIMESTAMP - 1);

- return arm_spe_process_queues(spe, MAX_TIMESTAMP);
+ ret = arm_spe_process_queues(spe, MAX_TIMESTAMP);
+ if (ret)
+ return ret;
+
+ if (!spe->use_ctx_pkt_for_pid)
+ ui__warning("Arm SPE CONTEXT packets not found in the traces.\n"
+ "Matching of TIDs to SPE events could be inaccurate.\n");
+
+ return 0;
}

static void arm_spe_free_queue(void *priv)
--
2.25.1


2021-11-11 17:23:40

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] perf arm-spe: Support hardware-based PID tracing

On Thu, Nov 11, 2021 at 5:36 AM German Gomez <[email protected]> wrote:
>
> If Arm SPE traces contain CONTEXT packets with TID info, use these
> values for tracking tid of samples. Otherwise fall back to using context
> switch events and display a message warning the user of possible timing
> inaccuracies [1].
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> Signed-off-by: German Gomez <[email protected]>

Acked-by: Namhyung Kim <[email protected]>

Thanks,
Namhyung

2021-11-11 18:27:52

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] perf arm-spe: Support hardware-based PID tracing

Em Thu, Nov 11, 2021 at 09:23:22AM -0800, Namhyung Kim escreveu:
> On Thu, Nov 11, 2021 at 5:36 AM German Gomez <[email protected]> wrote:
> >
> > If Arm SPE traces contain CONTEXT packets with TID info, use these
> > values for tracking tid of samples. Otherwise fall back to using context
> > switch events and display a message warning the user of possible timing
> > inaccuracies [1].
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> >
> > Signed-off-by: German Gomez <[email protected]>
>
> Acked-by: Namhyung Kim <[email protected]>

Thanks, applied.

- Arnaldo