Fix a few issues with per-thread mode: - With TRBE, thread migrations hit a queue without a decoder assert - On nVHE, everything looks like guest trace instead of host - Tracing multiple threads didn't really work
After adding a tests for those, it turns out there were some snapshot bugs not picked up by the other snapshot tests. That showed that the different snapshot searches for Intel BTS and Arm SPE were inconsistent and I think all tracers can benefit from using the same pointer fixup logic, so that gets refactored into the common auxtrace.c. Then SPE can just share the Intel BTS search as it was obviously copied from it originally. IntelPT keeps the more advanced duplicate data search, but in the future that could probably be made the common one and all tracers would benefit from using it (except maybe BTS if it always has a very small buffer, but I doubt the overhead of the duplicate search would be an issue). For now this is more of a refactor rather than behavioral change so don't do that yet.
Signed-off-by: James Clark james.clark@linaro.org --- James Clark (14): perf cs-etm: Fix nVHE per-thread decoding perf cs-etm: Warn for invalid timestamp option perf cs-etm: Turn on context packet timestamps in per-thread mode perf cs-etm: Use per-CPU queues for per-thread mode perf cs-etm: Increase default timestamp generation period perf auxtrace: Turn Intel BTS snapshot search into a generic one perf arm-spe: Use generic snapshot search perf auxtrace: intel-pt: Use new snapshot_has_wrapped callback perf cs-etm: Queue partial AUX records perf cs-etm: Don't print missing buffers in snapshot mode perf auxtrace: cs-etm: Capture wrapped snapshots perf test: Allow infinite named_thread loops perf test: Add test for per-thread mode perf cs-etm: Test multiple per-thread threads
Documentation/userspace-api/perf_ring_buffer.rst | 6 +- tools/perf/Documentation/perf-test.txt | 2 +- tools/perf/arch/arm/util/cs-etm.c | 30 ++++- tools/perf/arch/arm64/util/arm-spe.c | 147 +-------------------- tools/perf/arch/x86/util/intel-bts.c | 115 +--------------- tools/perf/arch/x86/util/intel-pt.c | 58 +++----- .../perf/tests/shell/coresight/per-thread-multi.sh | 78 +++++++++++ tools/perf/tests/shell/coresight/per-thread.sh | 48 +++++++ .../perf/tests/shell/coresight/raw_dump_stress.sh | 5 - tools/perf/tests/workloads/named_threads.c | 7 +- tools/perf/util/auxtrace.c | 136 +++++++++++++++++-- tools/perf/util/auxtrace.h | 20 ++- tools/perf/util/cs-etm.c | 98 ++++++++++---- 13 files changed, 382 insertions(+), 368 deletions(-) --- base-commit: bf10e6ee2ac3034c9068e03eed418fd16961984e change-id: 20260605-james-cs-unformatted-per-thread-fix-50e723aa7f0e
Best regards, -- James Clark james.clark@linaro.org
pid_fmt is unset in per-thread mode because we don't need PIDs, so we can't use it as a hint to determine which EL the host is running at. That results in falling though to the next part where EL1 is treated as guest, which is wrong for nVHE.
Fix it by only returning a guest machine when run from perf kvm where a guest image would be provided, which matches existing behavior in other parts of Perf.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 114b3cd2da49..d423f1cb408f 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1071,6 +1071,18 @@ static struct machine *cs_etm__get_machine(struct cs_etm_queue *etmq, if (pid_fmt == CS_ETM_PIDFMT_CTXTID) return &etmq->etm->session->machines.host;
+ /* + * If guest processing hasn't been enabled, also assume everything is + * the host. + * + * This matches Perf's guest handling in machines__find_for_cpumode() + * etc where guest machines are only created and used when perf_guest is + * set. This also guards against accidentally using guest machines when + * pid_fmt can't be used as a hint (per-thread mode). + */ + if (!perf_guest) + return &etmq->etm->session->machines.host; + /* * Not perfect, but otherwise assume anything in EL1 is the default * guest, and everything else is the host. Distinguishing between guest
Instead of silently returning an error, print a useful warning.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/arch/arm/util/cs-etm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index d2861d66a661..acfdb3f53821 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -176,8 +176,11 @@ static int cs_etm_validate_timestamp(struct perf_pmu *cs_etm_pmu, struct evsel * * 0b01000 Implementation supports a maximum timestamp of 64bits. */ trcidr0 &= GENMASK(28, 24); - if (!trcidr0) + if (!trcidr0) { + pr_err("%s: timestamp not supported by HW, disable with %s/timestamp=0/\n", + CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME); return -EINVAL; + }
return 0; }
Practically this costs nothing and preempts future changes that will be made to support decoding trace in order with Perf MMAP and JIT events. It also helps keep samples in order when we add support for multiple per-thread mode threads in the next commit.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/arch/arm/util/cs-etm.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index acfdb3f53821..d0486a164a70 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -453,13 +453,22 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, */ evsel__set_sample_bit(cs_etm_evsel, CPU);
- /* - * Also the case of per-cpu mmaps, need the contextID in order to be notified - * when a context switch happened. - */ if (!perf_cpu_map__is_any_cpu_or_is_empty(cpus)) { + /* + * Timestamps are required to interleave samples from different + * CPUs. + */ evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1); + /* Context IDs are required to associate trace to a process */ evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1); + } else { + /* + * Enable context packet timestamps only (no periodic + * timestamps). Even in per-thread mode with a single process we + * still need to correlate trace to a specific mmap around + * execs, which can be done with just context packet timestamps. + */ + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", UINT64_MAX); }
/*
This fixes two problems, firstly per-thread mode didn't support multiple threads well or at all because we always use queue 0 and assumed it was for a single thread. However, Perf attaches to all child threads of one process or allows multiple threads to be specified on the commandline. This meant that trace before a context packet could use the wrong MMAP for decoding, and threads running concurrently wouldn't be interleaved properly.
The second problem is that we'd put all HW_IDs into queue 0, so if a thread runs on multiple CPUs then it would receive multiple HW_IDs and trigger the assert that unformatted trace should only have 1 decoder per-CPU:
$ perf record --per-thread -e cs_etm//u -- taskset --cpu-list 0 \ taskset --cpu-list 1 true $ perf script
perf: util/cs-etm.c:3381: cs_etm__create_queue_decoders: Assertion `decoders == 1' failed.
Fixes: 9182f04a85b2 ("perf cs-etm: Pass unformatted flag to decoder") Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index d423f1cb408f..28f5ce63ba80 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -295,9 +295,6 @@ static int cs_etm__insert_trace_id_node(struct cs_etm_queue *etmq,
static struct cs_etm_queue *cs_etm__get_queue(struct cs_etm_auxtrace *etm, int cpu) { - if (etm->per_thread_decoding) - return etm->queues.queue_array[0].priv; - if (cpu < 0 || cpu >= (int)etm->queues.nr_queues) return NULL;
@@ -2160,6 +2157,7 @@ static void cs_etm__flush_all_stack(struct cs_etm_queue *etmq) */ static int cs_etm__get_data_block(struct cs_etm_queue *etmq) { + struct cs_etm_auxtrace *etm = etmq->etm; int ret;
/* The current block is not finished */ @@ -2188,6 +2186,27 @@ static int cs_etm__get_data_block(struct cs_etm_queue *etmq) */ cs_etm__flush_all_stack(etmq);
+ /* + * Per-thread mode still uses a queue for each CPU, but that CPU can run + * different threads. When the TID from the AUX record on a CPU changes, + * re-initialize the thread using the AUX record/buffer fragment TID so + * we can start decoding even if the context ID packet was cropped or + * they're disabled. + */ + if (etm->per_thread_decoding) { + struct cs_etm_traceid_queue *tidq = cs_etm__etmq_get_traceid_queue(etmq, + CS_ETM_PER_THREAD_TRACEID); + + if (thread__tid(tidq->decode_thread) != etmq->buffer->tid) { + thread__zput(tidq->frontend_thread); + thread__zput(tidq->decode_thread); + tidq->frontend_thread = machine__findnew_thread(&etm->session->machines.host, + -1, etmq->buffer->tid); + tidq->decode_thread = machine__findnew_thread(&etm->session->machines.host, + -1, etmq->buffer->tid); + } + } + return 1; }
@@ -3245,7 +3264,7 @@ static int cs_etm__queue_aux_fragment(struct perf_session *session, off_t file_o
if (aux_offset >= auxtrace_event->offset && aux_offset + aux_size <= auxtrace_event->offset + auxtrace_event->size) { - struct cs_etm_queue *etmq = cs_etm__get_queue(etm, auxtrace_event->cpu); + struct cs_etm_queue *etmq = cs_etm__get_queue(etm, sample->cpu);
if (!etmq) return -EINVAL;
The original purpose of timestamps in per-CPU mode was to roughly emit samples in the order they occurred across multiple CPUs, rather than emit all the trace from CPU 1 followed by all from CPU 2 etc. Previously there was no way to change the interval from the minimum of 1 cycle, but custom intervals are now supported by the driver.
Drastically reduce the number of timestamps emitted by increasing the interval, which is more than enough to interleave sample generation. This reduces the size of the trace generated.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/arch/arm/util/cs-etm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index d0486a164a70..d36b7e41399b 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -70,6 +70,9 @@ static const char * const metadata_ete_ro[] = {
enum cs_etm_version { CS_NOT_PRESENT, CS_ETMV3, CS_ETMV4, CS_ETE };
+/* Default timestamp period of 256 cycles (2 ^ (9 - 1) = 256). */ +#define DEFAULT_TS 9 + static bool cs_etm_is_ete(struct perf_pmu *cs_etm_pmu, struct perf_cpu cpu); static int cs_etm_get_ro(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path, __u64 *val); static bool cs_etm_pmu_path_exists(struct perf_pmu *pmu, struct perf_cpu cpu, const char *path); @@ -458,7 +461,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, * Timestamps are required to interleave samples from different * CPUs. */ - evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1); + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", DEFAULT_TS); /* Context IDs are required to associate trace to a process */ evsel__set_config_if_unset(cs_etm_evsel, "contextid", 1); } else { @@ -477,7 +480,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, * timestamp tracing. */ if (opts->sample_time_set) - evsel__set_config_if_unset(cs_etm_evsel, "timestamp", 1); + evsel__set_config_if_unset(cs_etm_evsel, "timestamp", DEFAULT_TS);
/* Add dummy event to keep tracking */ err = parse_event(evlist, "dummy:u");
Currently each auxtrace user has to implement both the snapshot search and the logic to fixup the 'head' and 'old' values after finding a wrap. IntelPT and BTS have the same fixup logic but different wrap searches. BTS and Arm SPE have the same search but SPE modifies head differently (although it probably shouldn’t), and Arm Coresight doesn't have a search at all, but later it will want its own search and same fixup logic.
Start moving towards a system where the pointers are always fixed up in the same way by auxtrace.c, and users only have to provide a search function or can use the generic one without reimplementing it.
The new auxtrace_find_snapshot() calls the search callback if it's implemented, handles pointer fixup in a consistent way and is based on intel_bts_find_snapshot(). auxtrace_record__has_wrapped() is the generic search function based on intel_bts_has_wrapped() and includes the caching from intel_bts_find_snapshot(). The old find_snapshot callback will eventually be removed.
No functional changes intended apart from changing the pr_debug3()s to print hex to match __auxtrace_mmap__read() and no longer leaking snapshot_refs when expanding the array length.
Signed-off-by: James Clark james.clark@linaro.org --- Documentation/userspace-api/perf_ring_buffer.rst | 6 +- tools/perf/arch/x86/util/intel-bts.c | 115 +------------------- tools/perf/util/auxtrace.c | 128 +++++++++++++++++++++-- tools/perf/util/auxtrace.h | 11 ++ 4 files changed, 133 insertions(+), 127 deletions(-)
diff --git a/Documentation/userspace-api/perf_ring_buffer.rst b/Documentation/userspace-api/perf_ring_buffer.rst index dc71544532ce..687c95a738f7 100644 --- a/Documentation/userspace-api/perf_ring_buffer.rst +++ b/Documentation/userspace-api/perf_ring_buffer.rst @@ -796,9 +796,9 @@ The perf only accesses the head pointer ``perf_event_mmap_page::aux_head`` in snapshot mode and doesn’t touch tail pointer ``aux_tail``, this is because the AUX ring buffer can overflow in free run mode, the tail pointer is useless in this case. Alternatively, the callback -``auxtrace_record::find_snapshot()`` is introduced for making the decision -of whether the AUX ring buffer has been wrapped around or not, at the -end it fixes up the AUX buffer's head which are used to calculate the +``auxtrace_record::snapshot_has_wrapped()`` is introduced for making the +decision of whether the AUX ring buffer has been wrapped around or not, at the +end Perf fixes up the AUX buffer's head which are used to calculate the trace data size.
As we know, the buffers' deployment can be per-thread mode, per-CPU diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c index d44d568a6d21..1b1df0003435 100644 --- a/tools/perf/arch/x86/util/intel-bts.c +++ b/tools/perf/arch/x86/util/intel-bts.c @@ -34,20 +34,12 @@ #define KiB_MASK(x) (KiB(x) - 1) #define MiB_MASK(x) (MiB(x) - 1)
-struct intel_bts_snapshot_ref { - void *ref_buf; - size_t ref_offset; - bool wrapped; -}; - struct intel_bts_recording { struct auxtrace_record itr; struct perf_pmu *intel_bts_pmu; struct evlist *evlist; bool snapshot_mode; size_t snapshot_size; - int snapshot_ref_cnt; - struct intel_bts_snapshot_ref *snapshot_refs; };
struct branch { @@ -280,46 +272,11 @@ static u64 intel_bts_reference(struct auxtrace_record *itr __maybe_unused) return rdtsc(); }
-static int intel_bts_alloc_snapshot_refs(struct intel_bts_recording *btsr, - int idx) -{ - const size_t sz = sizeof(struct intel_bts_snapshot_ref); - int cnt = btsr->snapshot_ref_cnt, new_cnt = cnt * 2; - struct intel_bts_snapshot_ref *refs; - - if (!new_cnt) - new_cnt = 16; - - while (new_cnt <= idx) - new_cnt *= 2; - - refs = calloc(new_cnt, sz); - if (!refs) - return -ENOMEM; - - memcpy(refs, btsr->snapshot_refs, cnt * sz); - - btsr->snapshot_refs = refs; - btsr->snapshot_ref_cnt = new_cnt; - - return 0; -} - -static void intel_bts_free_snapshot_refs(struct intel_bts_recording *btsr) -{ - int i; - - for (i = 0; i < btsr->snapshot_ref_cnt; i++) - zfree(&btsr->snapshot_refs[i].ref_buf); - zfree(&btsr->snapshot_refs); -} - static void intel_bts_recording_free(struct auxtrace_record *itr) { struct intel_bts_recording *btsr = container_of(itr, struct intel_bts_recording, itr);
- intel_bts_free_snapshot_refs(btsr); free(btsr); }
@@ -349,75 +306,6 @@ static int intel_bts_snapshot_finish(struct auxtrace_record *itr) return -EINVAL; }
-static bool intel_bts_first_wrap(u64 *data, size_t buf_size) -{ - int i, a, b; - - b = buf_size >> 3; - a = b - 512; - if (a < 0) - a = 0; - - for (i = a; i < b; i++) { - if (data[i]) - return true; - } - - return false; -} - -static int intel_bts_find_snapshot(struct auxtrace_record *itr, int idx, - struct auxtrace_mmap *mm, unsigned char *data, - u64 *head, u64 *old) -{ - struct intel_bts_recording *btsr = - container_of(itr, struct intel_bts_recording, itr); - bool wrapped; - int err; - - pr_debug3("%s: mmap index %d old head %zu new head %zu\n", - __func__, idx, (size_t)*old, (size_t)*head); - - if (idx >= btsr->snapshot_ref_cnt) { - err = intel_bts_alloc_snapshot_refs(btsr, idx); - if (err) - goto out_err; - } - - wrapped = btsr->snapshot_refs[idx].wrapped; - if (!wrapped && intel_bts_first_wrap((u64 *)data, mm->len)) { - btsr->snapshot_refs[idx].wrapped = true; - wrapped = true; - } - - /* - * In full trace mode 'head' continually increases. However in snapshot - * mode 'head' is an offset within the buffer. Here 'old' and 'head' - * are adjusted to match the full trace case which expects that 'old' is - * always less than 'head'. - */ - if (wrapped) { - *old = *head; - *head += mm->len; - } else { - if (mm->mask) - *old &= mm->mask; - else - *old %= mm->len; - if (*old > *head) - *head += mm->len; - } - - pr_debug3("%s: wrap-around %sdetected, adjusted old head %zu adjusted new head %zu\n", - __func__, wrapped ? "" : "not ", (size_t)*old, (size_t)*head); - - return 0; - -out_err: - pr_err("%s: failed, error %d\n", __func__, err); - return err; -} - struct auxtrace_record *intel_bts_recording_init(int *err) { struct perf_pmu *intel_bts_pmu = perf_pmus__find(INTEL_BTS_PMU_NAME); @@ -444,7 +332,8 @@ struct auxtrace_record *intel_bts_recording_init(int *err) btsr->itr.free = intel_bts_recording_free; btsr->itr.snapshot_start = intel_bts_snapshot_start; btsr->itr.snapshot_finish = intel_bts_snapshot_finish; - btsr->itr.find_snapshot = intel_bts_find_snapshot; + btsr->itr.snapshot_has_wrapped = auxtrace_record__has_wrapped; + btsr->itr.snapshot_search_bytes = 4096; btsr->itr.parse_snapshot_options = intel_bts_parse_snapshot_options; btsr->itr.reference = intel_bts_reference; btsr->itr.read_finish = auxtrace_record__read_finish; diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index aa749e1c3036..9586989e645e 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -603,8 +603,10 @@ int auxtrace_record__info_fill(struct auxtrace_record *itr,
void auxtrace_record__free(struct auxtrace_record *itr) { - if (itr) + if (itr) { + zfree(&itr->snapshot_wrapped); itr->free(itr); + } }
int auxtrace_record__snapshot_start(struct auxtrace_record *itr) @@ -621,12 +623,109 @@ int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit) return 0; }
-int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx, - struct auxtrace_mmap *mm, - unsigned char *data, u64 *head, u64 *old) +static int auxtrace_alloc_wrap_bitmap(struct auxtrace_record *itr, int idx) +{ + int len = itr->snapshot_wrapped_len; + unsigned long *new_bitmap; + int new_len = len * 2; + + if (!new_len) + new_len = BITS_PER_LONG; + + while (new_len <= idx) + new_len *= 2; + + new_bitmap = realloc(itr->snapshot_wrapped, bitmap_size(new_len)); + if (!new_bitmap) + return -ENOMEM; + + itr->snapshot_wrapped = new_bitmap; + bitmap_clear(itr->snapshot_wrapped, len, new_len - len); + itr->snapshot_wrapped_len = new_len; + + return 0; +} + +/* + * Generic auxtrace_record::has_wrapped() implementation that returns 1 if non + * zero data exists within auxtrace_record::snapshot_search_bytes of the end of + * the buffer. The result is cached for each buffer idx so the search is not + * repeated. + * + * Writes at the end mean a high chance that trace would have continued past + * this search area and wrapped to the beginning. It's not a perfect heuristic, + * but it's only to avoid saving mostly empty buffers into the file. A false + * positive results in saving up to snapshot_search_bytes zeros before the + * actual data, which a decoder should be able to skip over. + */ +int auxtrace_record__has_wrapped(struct auxtrace_record *itr, int idx, + unsigned char *data, size_t buf_size, + u64 head __maybe_unused) +{ + u64 *wide_data = (u64 *)data; + s64 i, a, b; + + if (idx >= itr->snapshot_wrapped_len) { + int err = auxtrace_alloc_wrap_bitmap(itr, idx); + + if (err) + return err; + } + + if (test_bit(idx, itr->snapshot_wrapped)) + return 1; + + b = buf_size / sizeof(u64); + a = b - (itr->snapshot_search_bytes / sizeof(u64)); + if (a < 0) + a = 0; + + for (i = a; i < b; i++) { + if (wide_data[i]) { + __set_bit(idx, itr->snapshot_wrapped); + return 1; + } + } + + return 0; +} + +static int auxtrace_find_snapshot(struct auxtrace_record *itr, int idx, + struct auxtrace_mmap *mm, unsigned char *data, + u64 *head, u64 *old) { - if (itr && itr->find_snapshot) - return itr->find_snapshot(itr, idx, mm, data, head, old); + int wrapped; + + pr_debug3("%s: mmap index %d old head 0x%"PRIx64" new head 0x%"PRIx64"\n", + __func__, idx, *old, *head); + + wrapped = itr->snapshot_has_wrapped(itr, idx, data, mm->len, *head); + if (wrapped < 0) { + pr_err("%s: failed, error %d\n", __func__, wrapped); + return wrapped; + } + + /* + * In full trace mode 'head' continually increases. However in snapshot + * mode 'head' is an offset within the buffer. Here 'old' and 'head' + * are adjusted to match the full trace case which expects that 'old' is + * always less than 'head'. + */ + if (wrapped) { + *old = *head; + *head += mm->len; + } else { + if (mm->mask) + *old &= mm->mask; + else + *old %= mm->len; + if (*old > *head) + *head += mm->len; + } + + pr_debug3("%s: wrap-around %sdetected, adjusted old head 0x%"PRIx64" adjusted new head 0x%"PRIx64"\n", + __func__, wrapped ? "" : "not ", *old, *head); + return 0; }
@@ -1958,12 +2057,21 @@ static int __auxtrace_mmap__read(struct mmap *map, union perf_event ev; void *data1, *data2; int kernel_is_64_bit = perf_env__kernel_is_64_bit(env); + int err;
head = auxtrace_mmap__read_head(mm, kernel_is_64_bit);
- if (snapshot && - auxtrace_record__find_snapshot(itr, mm->idx, mm, data, &head, &old)) - return -1; + if (snapshot) { + if (itr->find_snapshot) { + err = itr->find_snapshot(itr, mm->idx, mm, data, &head, &old); + if (err) + return err; + } else if (itr->snapshot_has_wrapped) { + err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); + if (err) + return err; + } + }
if (old == head) return 0; @@ -2042,8 +2150,6 @@ static int __auxtrace_mmap__read(struct mmap *map, mm->prev = head;
if (!snapshot) { - int err; - err = auxtrace_mmap__write_tail(mm, head, kernel_is_64_bit); if (err < 0) return err; diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index 6947f3f284c0..c2342d0f4afb 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -376,6 +376,7 @@ struct auxtrace_mmap_params { * @snapshot_start: starting a snapshot * @snapshot_finish: finishing a snapshot * @find_snapshot: find data to snapshot within auxtrace mmap + * @snapshot_has_wrapped: callback to check if the buffer has wrapped in snapshot mode * @parse_snapshot_options: parse snapshot options * @reference: provide a 64-bit reference number for auxtrace_event * @read_finish: called after reading from an auxtrace mmap @@ -383,6 +384,8 @@ struct auxtrace_mmap_params { * @default_aux_sample_size: default sample size for --aux sample option * @pmu: associated pmu * @evlist: selected events list + * @snapshot_wrapped_len: number of bits in @snapshot_wrapped + * @snapshot_wrapped: bitmap indicating if each aux buffer has wrapped */ struct auxtrace_record { int (*recording_options)(struct auxtrace_record *itr, @@ -400,6 +403,8 @@ struct auxtrace_record { int (*find_snapshot)(struct auxtrace_record *itr, int idx, struct auxtrace_mmap *mm, unsigned char *data, u64 *head, u64 *old); + int (*snapshot_has_wrapped)(struct auxtrace_record *itr, int idx, + unsigned char *data, size_t size, u64 head); int (*parse_snapshot_options)(struct auxtrace_record *itr, struct record_opts *opts, const char *str); @@ -408,6 +413,9 @@ struct auxtrace_record { unsigned int alignment; unsigned int default_aux_sample_size; struct evlist *evlist; + int snapshot_wrapped_len; + unsigned long *snapshot_wrapped; + int snapshot_search_bytes; };
/** @@ -590,6 +598,9 @@ int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit); int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx, struct auxtrace_mmap *mm, unsigned char *data, u64 *head, u64 *old); +int auxtrace_record__has_wrapped(struct auxtrace_record *itr, int idx, + unsigned char *data, size_t buf_size, + u64 head __maybe_unused); u64 auxtrace_record__reference(struct auxtrace_record *itr); int auxtrace_record__read_finish(struct auxtrace_record *itr, int idx);
The new generic snapshot search gives a similar result as the reimplemented version of it here. One slight difference is in the pointer fixup. SPE has:
*head += mm->len; *old = *head - mm->len;
Which gives exactly the same result but written in reverse order to the generic one:
*old = *head; *head += mm->len;
SPE also had a special fixup case for head pointers greater than the buffer length, which is not needed because the SPE driver always wraps them, and __auxtrace_mmap__read() handles that anyway. It also didn't have the special case for old > head for when the wrap heuristic fails but the pointers showed a wrap had happened.
The other feature lost is that this search only looked from head to the end of the buffer, rather than always at the last 512 bytes. This was flawed because once head is close to the end, it's likely it could contain zero padding from actual SPE data and a wrap would be missed. It's better to err on the side of caution and mark as a wrap, rather than trying to optimize by limiting the search from head onwards.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/arch/arm64/util/arm-spe.c | 147 +---------------------------------- 1 file changed, 2 insertions(+), 145 deletions(-)
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c index 1ba803a8d9b4..d8c4a1d6c621 100644 --- a/tools/perf/arch/arm64/util/arm-spe.c +++ b/tools/perf/arch/arm64/util/arm-spe.c @@ -26,7 +26,6 @@ #include "../../../util/record.h" #include "../../../util/header.h" #include "../../../util/arm-spe.h" -#include <tools/libc_compat.h> // reallocarray
#define ARM_SPE_CPU_MAGIC 0x1010101010101010ULL
@@ -37,8 +36,6 @@ struct arm_spe_recording { struct auxtrace_record itr; struct perf_pmu *arm_spe_pmu; struct evlist *evlist; - int wrapped_cnt; - bool *wrapped; };
/* Iterate config list to detect if the "freq" parameter is set */ @@ -497,146 +494,6 @@ static int arm_spe_snapshot_finish(struct auxtrace_record *itr) return ret; }
-static int arm_spe_alloc_wrapped_array(struct arm_spe_recording *ptr, int idx) -{ - bool *wrapped; - int cnt = ptr->wrapped_cnt, new_cnt, i; - - /* - * No need to allocate, so return early. - */ - if (idx < cnt) - return 0; - - /* - * Make ptr->wrapped as big as idx. - */ - new_cnt = idx + 1; - - /* - * Free'ed in arm_spe_recording_free(). - */ - wrapped = reallocarray(ptr->wrapped, new_cnt, sizeof(bool)); - if (!wrapped) - return -ENOMEM; - - /* - * init new allocated values. - */ - for (i = cnt; i < new_cnt; i++) - wrapped[i] = false; - - ptr->wrapped_cnt = new_cnt; - ptr->wrapped = wrapped; - - return 0; -} - -static bool arm_spe_buffer_has_wrapped(unsigned char *buffer, - size_t buffer_size, u64 head) -{ - u64 i, watermark; - u64 *buf = (u64 *)buffer; - size_t buf_size = buffer_size; - - /* - * Defensively handle the case where head might be continually increasing - if its value is - * equal or greater than the size of the ring buffer, then we can safely determine it has - * wrapped around. Otherwise, continue to detect if head might have wrapped. - */ - if (head >= buffer_size) - return true; - - /* - * We want to look the very last 512 byte (chosen arbitrarily) in the ring buffer. - */ - watermark = buf_size - 512; - - /* - * The value of head is somewhere within the size of the ring buffer. This can be that there - * hasn't been enough data to fill the ring buffer yet or the trace time was so long that - * head has numerically wrapped around. To find we need to check if we have data at the - * very end of the ring buffer. We can reliably do this because mmap'ed pages are zeroed - * out and there is a fresh mapping with every new session. - */ - - /* - * head is less than 512 byte from the end of the ring buffer. - */ - if (head > watermark) - watermark = head; - - /* - * Speed things up by using 64 bit transactions (see "u64 *buf" above) - */ - watermark /= sizeof(u64); - buf_size /= sizeof(u64); - - /* - * If we find trace data at the end of the ring buffer, head has been there and has - * numerically wrapped around at least once. - */ - for (i = watermark; i < buf_size; i++) - if (buf[i]) - return true; - - return false; -} - -static int arm_spe_find_snapshot(struct auxtrace_record *itr, int idx, - struct auxtrace_mmap *mm, unsigned char *data, - u64 *head, u64 *old) -{ - int err; - bool wrapped; - struct arm_spe_recording *ptr = - container_of(itr, struct arm_spe_recording, itr); - - /* - * Allocate memory to keep track of wrapping if this is the first - * time we deal with this *mm. - */ - if (idx >= ptr->wrapped_cnt) { - err = arm_spe_alloc_wrapped_array(ptr, idx); - if (err) - return err; - } - - /* - * Check to see if *head has wrapped around. If it hasn't only the - * amount of data between *head and *old is snapshot'ed to avoid - * bloating the perf.data file with zeros. But as soon as *head has - * wrapped around the entire size of the AUX ring buffer it taken. - */ - wrapped = ptr->wrapped[idx]; - if (!wrapped && arm_spe_buffer_has_wrapped(data, mm->len, *head)) { - wrapped = true; - ptr->wrapped[idx] = true; - } - - pr_debug3("%s: mmap index %d old head %zu new head %zu size %zu\n", - __func__, idx, (size_t)*old, (size_t)*head, mm->len); - - /* - * No wrap has occurred, we can just use *head and *old. - */ - if (!wrapped) - return 0; - - /* - * *head has wrapped around - adjust *head and *old to pickup the - * entire content of the AUX buffer. - */ - if (*head >= mm->len) { - *old = *head - mm->len; - } else { - *head += mm->len; - *old = *head - mm->len; - } - - return 0; -} - static u64 arm_spe_reference(struct auxtrace_record *itr __maybe_unused) { struct timespec ts; @@ -651,7 +508,6 @@ static void arm_spe_recording_free(struct auxtrace_record *itr) struct arm_spe_recording *sper = container_of(itr, struct arm_spe_recording, itr);
- zfree(&sper->wrapped); free(sper); }
@@ -674,7 +530,8 @@ struct auxtrace_record *arm_spe_recording_init(int *err, sper->arm_spe_pmu = arm_spe_pmu; sper->itr.snapshot_start = arm_spe_snapshot_start; sper->itr.snapshot_finish = arm_spe_snapshot_finish; - sper->itr.find_snapshot = arm_spe_find_snapshot; + sper->itr.snapshot_has_wrapped = auxtrace_record__has_wrapped; + sper->itr.snapshot_search_bytes = 512; sper->itr.parse_snapshot_options = arm_spe_parse_snapshot_options; sper->itr.recording_options = arm_spe_recording_options; sper->itr.info_priv_size = arm_spe_info_priv_size;
The new callback is only for the search, and auxtrace.c fixes up the pointers after a wrap is found. Update Intel PT to use the new callback and finish removal of the old one now that there are no more users.
No functional changes intended.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/arch/x86/util/intel-pt.c | 58 ++++++++++--------------------------- tools/perf/util/auxtrace.c | 14 +++------ tools/perf/util/auxtrace.h | 7 ----- 3 files changed, 19 insertions(+), 60 deletions(-)
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c index a533114c0048..b8de129a218e 100644 --- a/tools/perf/arch/x86/util/intel-pt.c +++ b/tools/perf/arch/x86/util/intel-pt.c @@ -1080,17 +1080,17 @@ static void intel_pt_copy_ref(void *ref_buf, size_t ref_size, size_t buf_size, }
static bool intel_pt_wrapped(struct intel_pt_recording *ptr, int idx, - struct auxtrace_mmap *mm, unsigned char *data, + size_t size, unsigned char *data, u64 head) { struct intel_pt_snapshot_ref *ref = &ptr->snapshot_refs[idx]; bool wrapped;
wrapped = intel_pt_compare_ref(ref->ref_buf, ref->ref_offset, - ptr->snapshot_ref_buf_size, mm->len, + ptr->snapshot_ref_buf_size, size, data, head);
- intel_pt_copy_ref(ref->ref_buf, ptr->snapshot_ref_buf_size, mm->len, + intel_pt_copy_ref(ref->ref_buf, ptr->snapshot_ref_buf_size, size, data, head);
return wrapped; @@ -1113,69 +1113,41 @@ static bool intel_pt_first_wrap(u64 *data, size_t buf_size) return false; }
-static int intel_pt_find_snapshot(struct auxtrace_record *itr, int idx, - struct auxtrace_mmap *mm, unsigned char *data, - u64 *head, u64 *old) +static int intel_pt_snapshot_has_wrapped(struct auxtrace_record *itr, int idx, + unsigned char *data, size_t size, + u64 head) { struct intel_pt_recording *ptr = container_of(itr, struct intel_pt_recording, itr); bool wrapped; int err;
- pr_debug3("%s: mmap index %d old head %zu new head %zu\n", - __func__, idx, (size_t)*old, (size_t)*head); - - err = intel_pt_snapshot_init(ptr, mm->len); + err = intel_pt_snapshot_init(ptr, size); if (err) - goto out_err; + return err;
if (idx >= ptr->snapshot_ref_cnt) { err = intel_pt_alloc_snapshot_refs(ptr, idx); if (err) - goto out_err; + return err; }
if (ptr->snapshot_ref_buf_size) { if (!ptr->snapshot_refs[idx].ref_buf) { - err = intel_pt_alloc_snapshot_ref(ptr, idx, mm->len); + err = intel_pt_alloc_snapshot_ref(ptr, idx, size); if (err) - goto out_err; + return err; } - wrapped = intel_pt_wrapped(ptr, idx, mm, data, *head); + wrapped = intel_pt_wrapped(ptr, idx, size, data, head); } else { wrapped = ptr->snapshot_refs[idx].wrapped; - if (!wrapped && intel_pt_first_wrap((u64 *)data, mm->len)) { + if (!wrapped && intel_pt_first_wrap((u64 *)data, size)) { ptr->snapshot_refs[idx].wrapped = true; wrapped = true; } }
- /* - * In full trace mode 'head' continually increases. However in snapshot - * mode 'head' is an offset within the buffer. Here 'old' and 'head' - * are adjusted to match the full trace case which expects that 'old' is - * always less than 'head'. - */ - if (wrapped) { - *old = *head; - *head += mm->len; - } else { - if (mm->mask) - *old &= mm->mask; - else - *old %= mm->len; - if (*old > *head) - *head += mm->len; - } - - pr_debug3("%s: wrap-around %sdetected, adjusted old head %zu adjusted new head %zu\n", - __func__, wrapped ? "" : "not ", (size_t)*old, (size_t)*head); - - return 0; - -out_err: - pr_err("%s: failed, error %d\n", __func__, err); - return err; + return wrapped; }
static u64 intel_pt_reference(struct auxtrace_record *itr __maybe_unused) @@ -1221,7 +1193,7 @@ struct auxtrace_record *intel_pt_recording_init(int *err) ptr->itr.free = intel_pt_recording_free; ptr->itr.snapshot_start = intel_pt_snapshot_start; ptr->itr.snapshot_finish = intel_pt_snapshot_finish; - ptr->itr.find_snapshot = intel_pt_find_snapshot; + ptr->itr.snapshot_has_wrapped = intel_pt_snapshot_has_wrapped; ptr->itr.parse_snapshot_options = intel_pt_parse_snapshot_options; ptr->itr.reference = intel_pt_reference; ptr->itr.read_finish = auxtrace_record__read_finish; diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index 9586989e645e..8ca872e6d465 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -2061,16 +2061,10 @@ static int __auxtrace_mmap__read(struct mmap *map,
head = auxtrace_mmap__read_head(mm, kernel_is_64_bit);
- if (snapshot) { - if (itr->find_snapshot) { - err = itr->find_snapshot(itr, mm->idx, mm, data, &head, &old); - if (err) - return err; - } else if (itr->snapshot_has_wrapped) { - err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); - if (err) - return err; - } + if (snapshot && itr->snapshot_has_wrapped) { + err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); + if (err) + return err; }
if (old == head) diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index c2342d0f4afb..a9d0c84184c5 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -375,7 +375,6 @@ struct auxtrace_mmap_params { * @free: free this auxtrace record structure * @snapshot_start: starting a snapshot * @snapshot_finish: finishing a snapshot - * @find_snapshot: find data to snapshot within auxtrace mmap * @snapshot_has_wrapped: callback to check if the buffer has wrapped in snapshot mode * @parse_snapshot_options: parse snapshot options * @reference: provide a 64-bit reference number for auxtrace_event @@ -400,9 +399,6 @@ struct auxtrace_record { void (*free)(struct auxtrace_record *itr); int (*snapshot_start)(struct auxtrace_record *itr); int (*snapshot_finish)(struct auxtrace_record *itr); - int (*find_snapshot)(struct auxtrace_record *itr, int idx, - struct auxtrace_mmap *mm, unsigned char *data, - u64 *head, u64 *old); int (*snapshot_has_wrapped)(struct auxtrace_record *itr, int idx, unsigned char *data, size_t size, u64 head); int (*parse_snapshot_options)(struct auxtrace_record *itr, @@ -595,9 +591,6 @@ int auxtrace_record__info_fill(struct auxtrace_record *itr, void auxtrace_record__free(struct auxtrace_record *itr); int auxtrace_record__snapshot_start(struct auxtrace_record *itr); int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit); -int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx, - struct auxtrace_mmap *mm, - unsigned char *data, u64 *head, u64 *old); int auxtrace_record__has_wrapped(struct auxtrace_record *itr, int idx, unsigned char *data, size_t buf_size, u64 head __maybe_unused);
In snapshot mode AUX records may straddle the boundary of multiple trace buffers. Currently we only queue AUX records whose range is contained wholly within a single buffer.
Fix it by queuing any partial AUX range that overlaps any buffer.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 54 +++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 28f5ce63ba80..5bafee31cf4b 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -3197,7 +3197,8 @@ static int cs_etm__queue_aux_fragment(struct perf_session *session, off_t file_o union perf_event *auxtrace_event_union; struct perf_record_auxtrace *auxtrace_event; union perf_event auxtrace_fragment; - __u64 aux_offset, aux_size; + __u64 aux_start, aux_end, auxtrace_start, auxtrace_end; + __u64 frag_start, frag_end, frag_size; enum cs_etm_format format;
struct cs_etm_auxtrace *etm = container_of(session->auxtrace, @@ -3245,42 +3246,41 @@ static int cs_etm__queue_aux_fragment(struct perf_session *session, off_t file_o }
if (aux_event->flags & PERF_AUX_FLAG_OVERWRITE) { - /* - * Clamp size in snapshot mode. The buffer size is clamped in - * __auxtrace_mmap__read() for snapshots, so the aux record size doesn't reflect - * the buffer size. - */ - aux_size = min(aux_event->aux_size, auxtrace_event->size); - /* * In this mode, the head also points to the end of the buffer so aux_offset * needs to have the size subtracted so it points to the beginning as in normal mode */ - aux_offset = aux_event->aux_offset - aux_size; + aux_start = aux_event->aux_offset - aux_event->aux_size; + aux_end = aux_event->aux_offset; } else { - aux_size = aux_event->aux_size; - aux_offset = aux_event->aux_offset; + aux_start = aux_event->aux_offset; + aux_end = aux_event->aux_offset + aux_event->aux_size; }
- if (aux_offset >= auxtrace_event->offset && - aux_offset + aux_size <= auxtrace_event->offset + auxtrace_event->size) { + auxtrace_start = auxtrace_event->offset; + auxtrace_end = auxtrace_start + auxtrace_event->size; + frag_start = max(aux_start, auxtrace_start); + frag_end = min(aux_end, auxtrace_end); + + if (frag_start < frag_end) { struct cs_etm_queue *etmq = cs_etm__get_queue(etm, sample->cpu);
if (!etmq) return -EINVAL;
/* - * If this AUX event was inside this buffer somewhere, create a new auxtrace event - * based on the sizes of the aux event, and queue that fragment. + * If this AUX event overlaps this buffer, create a new auxtrace event + * for the overlapping range and queue that fragment. */ + frag_size = frag_end - frag_start; auxtrace_fragment.auxtrace = *auxtrace_event; - auxtrace_fragment.auxtrace.size = aux_size; - auxtrace_fragment.auxtrace.offset = aux_offset; + auxtrace_fragment.auxtrace.size = frag_size; + auxtrace_fragment.auxtrace.offset = frag_start; auxtrace_fragment.auxtrace.idx = etmq->queue_nr; - file_offset += aux_offset - auxtrace_event->offset + auxtrace_event->header.size; + file_offset += frag_start - auxtrace_start + auxtrace_event->header.size;
pr_debug3("CS ETM: Queue buffer size: %#"PRI_lx64" offset: %#"PRI_lx64 - " tid: %d cpu: %d\n", aux_size, aux_offset, sample->tid, sample->cpu); + " tid: %d cpu: %d\n", frag_size, frag_start, sample->tid, sample->cpu); err = auxtrace_queues__add_event(&etm->queues, session, &auxtrace_fragment, file_offset, NULL); if (err) @@ -3320,6 +3320,7 @@ static int cs_etm__queue_aux_records_cb(struct perf_session *session, union perf struct auxtrace_index *auxtrace_index; struct evsel *evsel; size_t i; + bool found = false;
/* Don't care about any other events, we're only queuing buffers for AUX events */ if (event->header.type != PERF_RECORD_AUX) @@ -3353,11 +3354,14 @@ static int cs_etm__queue_aux_records_cb(struct perf_session *session, union perf ret = cs_etm__queue_aux_fragment(session, ent->file_offset, ent->sz, &event->aux, &sample); /* - * Stop search on error or successful values. Continue search on - * 1 ('not found') + * Stop search on error values. Continue search on + * 1 ('not found') or 0 ('queued') */ - if (ret != 1) + if (ret < 0) goto out; + + if (!ret) + found = true; } }
@@ -3365,8 +3369,10 @@ static int cs_etm__queue_aux_records_cb(struct perf_session *session, union perf * Couldn't find the buffer corresponding to this aux record, something went wrong. Warn but * don't exit with an error because it will still be possible to decode other aux records. */ - pr_err("CS ETM: Couldn't find auxtrace buffer for aux_offset: %#"PRI_lx64 - " tid: %d cpu: %d\n", event->aux.aux_offset, sample.tid, sample.cpu); + if (!found) { + pr_err("CS ETM: Couldn't find auxtrace buffer for aux_offset: %#"PRI_lx64 + " tid: %d cpu: %d\n", event->aux.aux_offset, sample.tid, sample.cpu); + } ret = 0; out: perf_sample__exit(&sample);
The driver produces lots of AUX records for trace data that was never captured in snapshot mode. Downgrade this to a pr_debug3 message as it's only an error in normal mode.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/util/cs-etm.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 5bafee31cf4b..a80f34530917 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -3317,6 +3317,9 @@ static int cs_etm__queue_aux_records_cb(struct perf_session *session, union perf struct perf_sample sample; int ret; struct auxtrace_index_entry *ent; + struct cs_etm_auxtrace *etm = container_of(session->auxtrace, + struct cs_etm_auxtrace, + auxtrace); struct auxtrace_index *auxtrace_index; struct evsel *evsel; size_t i; @@ -3370,8 +3373,10 @@ static int cs_etm__queue_aux_records_cb(struct perf_session *session, union perf * don't exit with an error because it will still be possible to decode other aux records. */ if (!found) { - pr_err("CS ETM: Couldn't find auxtrace buffer for aux_offset: %#"PRI_lx64 - " tid: %d cpu: %d\n", event->aux.aux_offset, sample.tid, sample.cpu); + int level = etm->snapshot_mode ? 3 : 0; + + pr_debugN(level, "CS ETM: Couldn't find auxtrace buffer for aux_offset: %#"PRI_lx64 + " tid: %d cpu: %d\n", event->aux.aux_offset, sample.tid, sample.cpu); } ret = 0; out:
snapshot_has_wrapped() isn't implemented for Coresight, so when a wrap occurs we only save from offset 0 to current head, modulo buffer size, rather than capturing the whole buffer. Coresight drivers currently use a monotonic head pointer even in snapshot mode, which gives a lot more information for taking snapshots than a wrapped pointer. It means we don't need to search for data at the end of the buffer to guess if a wrap has occurred, and we can go back to saving partial buffers after a wrap by checking if the next size to take is less than the buffer size.
The drivers will always be able to generate a monotonic head because TRBE has a bytes written counter, so even if we stop generating an AUX record on every overflow, we can check if the counter indicated an overwrite and offset head by a whole buffer size if it wrapped. This forces userspace to read the whole buffer rather than between last and current head. ETR and other sinks use software double buffering, so can continue as they are.
Add a much simpler 'monotonic_snapshot_head' path for tracers to use, and use it for Coresight.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/arch/arm/util/cs-etm.c | 1 + .../perf/tests/shell/coresight/raw_dump_stress.sh | 5 ----- tools/perf/util/auxtrace.c | 22 ++++++++++++++++------ tools/perf/util/auxtrace.h | 2 ++ 4 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index d36b7e41399b..dbf76dd3349e 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -918,6 +918,7 @@ struct auxtrace_record *cs_etm_record_init(int *err) ptr->itr.info_fill = cs_etm_info_fill; ptr->itr.snapshot_start = cs_etm_snapshot_start; ptr->itr.snapshot_finish = cs_etm_snapshot_finish; + ptr->itr.monotonic_snapshot_head = true; ptr->itr.reference = cs_etm_reference; ptr->itr.free = cs_etm_recording_free; ptr->itr.read_finish = auxtrace_record__read_finish; diff --git a/tools/perf/tests/shell/coresight/raw_dump_stress.sh b/tools/perf/tests/shell/coresight/raw_dump_stress.sh index bea70d825596..c9459ded4cbe 100755 --- a/tools/perf/tests/shell/coresight/raw_dump_stress.sh +++ b/tools/perf/tests/shell/coresight/raw_dump_stress.sh @@ -48,11 +48,6 @@ read -r size offset last_idx <<< "$(awk ' # everything was dumped. Allow 48 bytes difference to cover 3 frames: current # frame length, a partial frame and a final empty one, all of which aren't # dumped. -# -# TODO: for a single snapshot, offset should always be zero. However, we -# currently output AUX records in snapshot mode when we shouldn't, which -# increments the offset. Allow for that until it's fixed so we can test raw -# dumping. decode_size=$((1 + last_idx - offset)) if [ "$decode_size" -gt "$((size - 48))" ] && [ "$decode_size" -le "$((size))" ]; then echo "PASS: AUXTRACE buffer length matches dumped packet index" diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index 8ca872e6d465..e3c770b46e94 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -2051,9 +2051,9 @@ static int __auxtrace_mmap__read(struct mmap *map, bool snapshot, size_t snapshot_size) { struct auxtrace_mmap *mm = &map->auxtrace_mmap; - u64 head, old = mm->prev, offset, ref; + u64 head, old = mm->prev, offset, ref, size; unsigned char *data = mm->base; - size_t size, head_off, old_off, len1, len2, padding; + size_t head_off, old_off, len1, len2, padding; union perf_event ev; void *data1, *data2; int kernel_is_64_bit = perf_env__kernel_is_64_bit(env); @@ -2061,10 +2061,20 @@ static int __auxtrace_mmap__read(struct mmap *map,
head = auxtrace_mmap__read_head(mm, kernel_is_64_bit);
- if (snapshot && itr->snapshot_has_wrapped) { - err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); - if (err) - return err; + if (snapshot) { + if (itr->snapshot_has_wrapped) { + err = auxtrace_find_snapshot(itr, mm->idx, mm, data, &head, &old); + if (err) + return err; + } else if (itr->monotonic_snapshot_head) { + size = head - old; + /* Force a full buffer read if a wrap has occurred */ + if (size > mm->len) { + pr_debug3("%s: wrap detected, adjusting old from 0x%"PRIx64" to 0x%"PRIx64"\n", + __func__, old, head - mm->len); + old = head - mm->len; + } + } }
if (old == head) diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index a9d0c84184c5..971b817d3396 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -385,6 +385,7 @@ struct auxtrace_mmap_params { * @evlist: selected events list * @snapshot_wrapped_len: number of bits in @snapshot_wrapped * @snapshot_wrapped: bitmap indicating if each aux buffer has wrapped + * @monotonic_snapshot_head: driver doesn't wrap head pointer on overflow */ struct auxtrace_record { int (*recording_options)(struct auxtrace_record *itr, @@ -412,6 +413,7 @@ struct auxtrace_record { int snapshot_wrapped_len; unsigned long *snapshot_wrapped; int snapshot_search_bytes; + bool monotonic_snapshot_head; };
/**
The following test will want this to run forever, so add support by treating 0 loops as infinite.
Convert i and named_threads_work to be unsigned to avoid undefined signed overflow behavior in infinite loop mode.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/Documentation/perf-test.txt | 2 +- tools/perf/tests/workloads/named_threads.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/perf/Documentation/perf-test.txt b/tools/perf/Documentation/perf-test.txt index 859df74e62ef..dca8972fec8d 100644 --- a/tools/perf/Documentation/perf-test.txt +++ b/tools/perf/Documentation/perf-test.txt @@ -68,7 +68,7 @@ OPTIONS nrloops: brstack, context_switch_loop
'named_threads' accepts the number of threads and the number of loops to - do in each thread. + do in each thread. Use 0 for an infinite loop.
The datasym, landlock, deterministic and callchain workloads don't accept any. diff --git a/tools/perf/tests/workloads/named_threads.c b/tools/perf/tests/workloads/named_threads.c index d051d41a3cfe..6672c2f77684 100644 --- a/tools/perf/tests/workloads/named_threads.c +++ b/tools/perf/tests/workloads/named_threads.c @@ -11,7 +11,7 @@ #define MAX_THREADS 25
static int iterations = 500; -int named_threads_work = 1234; +unsigned int named_threads_work = 1234;
typedef void *(*thread_fn_t)(void *);
@@ -19,7 +19,8 @@ typedef void *(*thread_fn_t)(void *); noinline void *named_threads_thread##n(void *arg __maybe_unused) \ { \ pthread_setname_np(pthread_self(), "thread" #n); \ - for (int i = 0; i < iterations; i++) \ + for (unsigned int i = 0; \ + i < (unsigned int) iterations || iterations == 0; i++) \ named_threads_work += 3; \ \ return NULL; \ @@ -65,7 +66,7 @@ static thread_fn_t thread_fns[MAX_THREADS] = {
/* * Creates argv[0] threads that run a unique function named "thread[x]" which performs - * a multiplication in a loop for argv[1] loops. + * a multiplication in a loop for argv[1] loops (use 0 for infinite loop). */ static int named_threads(int argc, const char **argv) {
Test that per-thread mode works when the process migrates across CPUs and that the decode happens in the correct order.
A TODO is left in for when support for decoding in order with Perf records is added.
Signed-off-by: James Clark james.clark@linaro.org --- tools/perf/tests/shell/coresight/per-thread.sh | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/tools/perf/tests/shell/coresight/per-thread.sh b/tools/perf/tests/shell/coresight/per-thread.sh new file mode 100755 index 000000000000..77d1ed3405a9 --- /dev/null +++ b/tools/perf/tests/shell/coresight/per-thread.sh @@ -0,0 +1,48 @@ +#!/bin/bash -e +# CoreSight per-thread CPU attribution (exclusive) + +# SPDX-License-Identifier: GPL-2.0 + +# If CoreSight is not available, skip the test +perf list pmu | grep -q cs_etm || exit 2 + +if ! tmpdir=$(mktemp -d /tmp/perf-cs-callchain-test.XXXXXX); then + echo "mktemp failed" + exit 1 +fi + +cleanup_files() +{ + rm -rf "$tmpdir" + trap - EXIT TERM INT +} + +trap cleanup_files EXIT +trap 'cleanup_files; exit 1' TERM INT + +echo "Recording..." +perf record -o "$tmpdir/data" -e cs_etm//u --per-thread -- \ + taskset --cpu-list 0 taskset --cpu-list 1 taskset --cpu-list 2 true > /dev/null 2>&1 + +echo "Decoding..." +perf script -i "$tmpdir/data" --itrace=b -F comm,cpu 2> /dev/null | \ + grep -Eo '(taskset|true).*([[0-9]+])' | \ + uniq | tail -n 3 > "$tmpdir/script" 2>/dev/null + +# Check that the decode says it ran on CPU 0, 1, 2, in that order. TODO: The +# correct result should be "taskset [0,1,2], true 2" but we don't decode trace +# in order of the Perf events yet, so everything is associated with the last +# exec. +cat > "$tmpdir/expected" << EOF +true [000] +true [001] +true [002] +EOF + +if ! diff -q "$tmpdir/script" "$tmpdir/expected"; then + echo "FAIL: per-thread output doesn't match expected:" + cat "$tmpdir/script" + exit 1 +fi + +exit 0
Launch something that has multiple child threads, attach to it and make sure that all child threads are present in the samples
Signed-off-by: James Clark james.clark@linaro.org --- .../perf/tests/shell/coresight/per-thread-multi.sh | 78 ++++++++++++++++++++++ 1 file changed, 78 insertions(+)
diff --git a/tools/perf/tests/shell/coresight/per-thread-multi.sh b/tools/perf/tests/shell/coresight/per-thread-multi.sh new file mode 100755 index 000000000000..ea525ed5e18b --- /dev/null +++ b/tools/perf/tests/shell/coresight/per-thread-multi.sh @@ -0,0 +1,78 @@ +#!/bin/bash -e +# CoreSight per-thread multiple threads (exclusive) + +# SPDX-License-Identifier: GPL-2.0 + +# If CoreSight is not available, skip the test +perf list pmu | grep -q cs_etm || exit 2 + +if ! tmpdir=$(mktemp -d /tmp/perf-cs-callchain-test.XXXXXX); then + echo "mktemp failed" + exit 1 +fi + +cleanup_files() +{ + if [[ $parent ]]; then + kill -9 $parent + wait $parent || true + fi + if [[ $perf ]]; then + kill -9 $perf + wait $perf || true + fi + rm -rf "$tmpdir" + trap - EXIT TERM INT +} + +trap cleanup_files EXIT +trap 'cleanup_files; exit 1' TERM INT + +# Launch 2 threads to run indefinitely +nthreads=2 +perf test -w named_threads $nthreads 0 & +parent=$! + +# While parent still exists, wait for the 2 children to spawn +while kill -0 "$parent" 2>/dev/null; do + threads=(/proc/"$parent"/task/*) + + if (( ${#threads[@]} >= $(($nthreads + 1)))); then + break + fi + + sleep 0.1 +done + +echo "Recording..." +perf record -o "$tmpdir/data" -e cs_etm//u --per-thread -Se -m,64K --pid $parent > /dev/null 2>&1 & +perf=$! + +sleep 1 + +kill $parent +wait $parent || true +unset parent +wait $perf +unset perf + +echo "Decoding..." +perf script -i "$tmpdir/data" > "$tmpdir/script" 2>/dev/null + +# Exit early unless there is a dedicated sink per core which only TRBE +# guarantees. This is because shared sinks will report BUSY if two threads try +# to use them at the same time +if ! ls /sys/bus/coresight/devices/trbe* > /dev/null 2>&1; then + echo "No TRBE sinks, skipping output validation" + exit 0 +fi + +# Check all threads were traced and they have the correct thread name and symbol +for i in $(seq 1 $nthreads); do + if ! grep -q "thread${i} .* named_threads_thread${i}" "$tmpdir/script"; then + echo "Error: thread${i} missing" >&2 + exit 1 + fi +done + +exit 0