Re: [PATCH] perf: arm_pmuv3: Zero initialize hw_id branch stack field

[James Clark]

On 07/08/2026 11:44, Anshuman Khandual wrote:
> On 07/08/26 2:44 PM, James Clark wrote:
> > PERF_SAMPLE_BRANCH_HW_INDEX is supported by BRBE so hw_id is passed to
> > userspace, but it's never set by the BRBE driver. Zero initialize it as
> > it should be according to the docs:
> >
> >     * For the architectures whose raw branch records are
> >     * already stored in age order, the hw_idx should be 0.
>
> The in code documentation while defining perf_branch_stack.
> Probably a good idea to specify the same above.
>
>   * For the architectures whose raw branch records are
>   * already stored in age order, the hw_idx should be 0.
>   */
> struct perf_branch_stack {
> 	u64				nr;
> 	u64				hw_idx;
> 	struct perf_branch_entry	entries[];
> };

I found it easily enough. I wouldn't want to put the same comment in two 
places and risk one of them going stale. And if I take it away from one 
place and move it to the struct then it's just missing from somewhere 
else instead. So I think I'd rather leave this one.

> > It's probably too risky to remove PERF_SAMPLE_BRANCH_HW_INDEX from BRBE
> > now in case anyone is setting it and reading the value, but zero
> > initializing the whole struct also protects against the same issue with
> > new fields that are added in the future.
>
> Agreed. Because PERF_SAMPLE_BRANCH_HW_INDEX is supported in BRBE,
> hw_idx pushed to the userspace should be zero if HW never updates.
> This is definitely better than dropping PERF_SAMPLE_BRANCH_HW_INDEX
> flag all together to avoid breaking current users (if any).
> > Fixes: 58074a0fce66 ("perf: arm_pmuv3: Add support for the Branch Record Buffer Extension (BRBE)")
> > Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>

Thanks

> > ---
> > Very small fix spotted by Sashiko. It was probably always zero during
> > testing or never looked at.
> > ---
> >   drivers/perf/arm_pmuv3.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> > index 8014ff766cff..b9a8592bf112 100644
> > --- a/drivers/perf/arm_pmuv3.c
> > +++ b/drivers/perf/arm_pmuv3.c
> > @@ -1361,7 +1361,7 @@ static int branch_records_alloc(struct arm_pmu *armpmu)
> >   		struct pmu_hw_events *events_cpu;
> >   
> >   		events_cpu = per_cpu_ptr(armpmu->hw_events, cpu);
> > -		events_cpu->branch_stack = kmalloc(size, GFP_KERNEL);
> > +		events_cpu->branch_stack = kzalloc(size, GFP_KERNEL);
> >   		if (!events_cpu->branch_stack)
> >   			return -ENOMEM;
> >   	}
> >
> > ---
> > base-commit: f9a2394a23482bfd330911e9c8295b71724feacd
> > change-id: 20260807-james-brbe-init-hw-idx-53dec54fe266
> >
> > Best regards,
> > --
> > James Clark <james.clark@xxxxxxxxxx>