Re: [PATCH v2 11/14] perf auxtrace: cs-etm: Capture wrapped snapshots
Adrian Hunter <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-perf-users |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 21/08/2026 12:49, James Clark wrote: > 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 <[email protected]> > --- > 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; > + } > + } I feel like this is asking to keep the ->find_snapshot() callback. i.e. basically the same thing, but add a helper that BTS and SPE can call directly, like auxtrace_record__default_find_snapshot(). For PT, add/use auxtrace_record__snapshot_fixup() that does the common old/head adjustment. Then ->monotonic_snapshot_head is not needed. > } > > 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; > }; > > /** >