Re: [PATCH v2 11/14] perf auxtrace: cs-etm: Capture wrapped snapshots

[Adrian Hunter]

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 <james.clark@xxxxxxxxxx>
> ---
>  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;
>  };
>  
>  /**
>