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

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 <[email protected]>
> 
> Reviewed-by: Anshuman Khandual <[email protected]>
> 

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 <[email protected]>
>>
>