Re: [PATCH 2/9] perf thread-stack: Report branch stack hw_idx as not available
James Clark <[email protected]>
| Newsgroups | org.kernel.vger.linux-perf-users,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-doc |
|---|---|
| Message-ID | <[email protected]> |
On 03/08/2026 10:06, Amir Ayupov wrote: > thread_stack__br_sample() and thread_stack__br_sample_late() fill a > caller-supplied branch_stack that is typically allocated with zalloc(), > leaving hw_idx as 0. Zero is a valid hardware index, so consumers that > honour PERF_SAMPLE_BRANCH_HW_INDEX see a reconstructed branch stack > claiming to start at LBR TOS entry 0. > > These branch stacks are reconstructed from instruction trace and have no > hardware index at all. Set hw_idx to -1ULL, which is the established way > to say "not available" and matches what intel-pt and cs-etm already put > in the branch stacks they synthesise directly. > This fix makes sense in case someone doesn't overwrite it, but I think "matches what cs-etm already puts in the branch stacks" isn't quite right. Based on the hw_id docs, cs-etm should be setting 0 shouldn't it? * For the architectures whose raw branch records are * already stored in age order, the hw_idx should be 0. -1 "unknown" isn't accurate as we do know what the order is because we generate them in order. If anyone is reading the field 0 is much more useful than -1. Can we change cs_etm__synth_branch_sample() to 0 and update this commit message to not say that -1 is right for cs-etm? > Signed-off-by: Amir Ayupov <[email protected]> > --- > tools/perf/util/thread-stack.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c > index c5ce741b07446..1a3dffa83bde2 100644 > --- a/tools/perf/util/thread-stack.c > +++ b/tools/perf/util/thread-stack.c > @@ -624,6 +624,7 @@ void thread_stack__br_sample(struct thread *thread, int cpu, > unsigned int nr; > > dst->nr = 0; > + dst->hw_idx = -1ULL; > > if (!ts) > return; > @@ -686,6 +687,7 @@ void thread_stack__br_sample_late(struct thread *thread, int cpu, > bool start = false; > > dst->nr = 0; > + dst->hw_idx = -1ULL; > > if (!ts) > return;