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

On 24/08/2026 13:21, Adrian Hunter wrote:
> 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.
> 

Hmm yeah I suppose that makes sense. I can give it a go.

>>   	}
>>   
>>   	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;
>>   };
>>   
>>   /**
>>
>