Re: [PATCH v2 07/14] perf arm-spe: Use generic snapshot search
From: James Clark
Date: Fri Oct 09 2026 - 05:17:42 EST
On 27/08/2026 17:16, Leo Yan wrote:
On Fri, Aug 21, 2026 at 10:49:05AM +0100, James Clark wrote:
[...]
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.
Here mentioned the "special fixup case" is:
if (head >= buffer_size)
return true
If the hardware pointer is exactly the end of buffer, it is a strong
indication for wrapping. So it might be worth adding an explicit
"head == buffer_size" check in the common code.
That said, if always checking the final 512 bytes, this case is very
likely to be detected anyway, so I am not concerned about dropping the
check.
I can't visualise why head == buffer_size indicates wrapping any more than head equaling any other index in the buffer. The heuristic seems to be impossible to make perfect, so I'm inclined to leave it as is.
Couldn't you also say "head > buffer - 512" indicates a wrap? But that could also just be the first time around with no wrap if there is no data after that point. Same way that head == buffer_size could also not be a wrap on the first iteration. But then you end up saving the whole buffer anyway when *old is still 0 or any of it isn't padding, so it doesn't make a difference.
Really we should move to the duplicate detection algorithm like IntelPT, or update the driver to use a monotonic head if we think it won't break anything. It's so much more usable.
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.
Wouldn't this be a trade-off between missing a wrap and reporting a
false positive wrap?
Yep, and false positives are basically harmless with this algorithm. It doesn't do anything do remove duplicate data, so you might as well mark it as wrapped as early as possible and save it all anyway.
Ignoring head makes the search overlap valid trace data when head is
close to the end of the buffer but has not wrapped yet
(e.g. mm->len - head < 512). That data can then be mistaken as evidence
of a wrap.
For a false positive wrap, [head..mm->len] contains zero data. This
should be fine, as zero data are treated as PAD packets and discarded
during decoding.
I'd suggest adding this info into the commit log, in case later we need
to understand these weird cases. With that:
The summaries are pretty good, will add to the commit message.
Reviewed-by: Leo Yan <leo.yan@xxxxxxx>