Re: [PATCH v5 2/4] perf/core: Clear the whole branch entry in perf_clear_branch_entry()

[James Clark <james.clark@linaro.org>]

On 16/06/2026 16:57, Puranjay Mohan wrote:
> perf_clear_branch_entry_bitfields() resets the bitfields of struct
> perf_branch_entry one at a time and deliberately leaves from/to alone,
> since callers overwrite those immediately. The list of assignments has to
> be kept in sync with the struct by hand, and it has already drifted:
> new_type and priv were added to perf_branch_entry but never cleared here,
> so stale values can leak into the records handed to userspace.
> 
> Clear the entry with a single struct assignment instead:
> 
> 	*br = (struct perf_branch_entry){ };
> 
> Every caller writes from/to right after the clear, so zeroing them as well
> is harmless and the dead stores are elided on the x86 LBR read paths.
> There is no longer anything to keep in sync when a field is added.
> 
> The helper no longer touches only the bitfields, so rename it to
> perf_clear_branch_entry() and update the callers, fixing up the
> br+nr/br+out spacing on the touched lines while at it.
> 
> Fixes: b190bc4ac9e6 ("perf: Extend branch type classification")
> Fixes: 5402d25aa571 ("perf: Capture branch privilege information")
> Suggested-by: James Clark <james.clark@linaro.org>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
>   arch/x86/events/amd/brs.c   |  2 +-
>   arch/x86/events/amd/lbr.c   |  2 +-
>   arch/x86/events/intel/lbr.c |  6 +++---
>   drivers/perf/arm_brbe.c     |  2 +-
>   include/linux/perf_event.h  | 16 ++--------------
>   5 files changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/events/amd/brs.c b/arch/x86/events/amd/brs.c
> index 06f35a6b58a5..68c5f42965e9 100644
> --- a/arch/x86/events/amd/brs.c
> +++ b/arch/x86/events/amd/brs.c
> @@ -343,7 +343,7 @@ void amd_brs_drain(void)
>   
>   		rdmsrq(brs_from(brs_idx), from);
>   
> -		perf_clear_branch_entry_bitfields(br+nr);
> +		perf_clear_branch_entry(br + nr);
>   
>   		br[nr].from = from;
>   		br[nr].to   = to;
> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> index d24da377df77..08401fd60585 100644
> --- a/arch/x86/events/amd/lbr.c
> +++ b/arch/x86/events/amd/lbr.c
> @@ -181,7 +181,7 @@ void amd_pmu_lbr_read(void)
>   		    entry.to.split.reserved)
>   			continue;
>   
> -		perf_clear_branch_entry_bitfields(br + out);
> +		perf_clear_branch_entry(br + out);
>   
>   		br[out].from	= sign_ext_branch_ip(entry.from.split.ip);
>   		br[out].to	= sign_ext_branch_ip(entry.to.split.ip);
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 72f2adcda7c6..295da179fa74 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -755,7 +755,7 @@ void intel_pmu_lbr_read_32(struct cpu_hw_events *cpuc)
>   
>   		rdmsrq(x86_pmu.lbr_from + lbr_idx, msr_lastbranch.lbr);
>   
> -		perf_clear_branch_entry_bitfields(br);
> +		perf_clear_branch_entry(br);
>   
>   		br->from	= msr_lastbranch.from;
>   		br->to		= msr_lastbranch.to;
> @@ -846,7 +846,7 @@ void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
>   		if (abort && x86_pmu.lbr_double_abort && out > 0)
>   			out--;
>   
> -		perf_clear_branch_entry_bitfields(br+out);
> +		perf_clear_branch_entry(br + out);
>   		br[out].from	 = from;
>   		br[out].to	 = to;
>   		br[out].mispred	 = mis;
> @@ -920,7 +920,7 @@ static void intel_pmu_store_lbr(struct cpu_hw_events *cpuc,
>   		to = rdlbr_to(i, lbr);
>   		info = rdlbr_info(i, lbr);
>   
> -		perf_clear_branch_entry_bitfields(e);
> +		perf_clear_branch_entry(e);
>   
>   		e->from		= from;
>   		e->to		= to;
> diff --git a/drivers/perf/arm_brbe.c b/drivers/perf/arm_brbe.c
> index ba554e0c846c..effbdeacfcbb 100644
> --- a/drivers/perf/arm_brbe.c
> +++ b/drivers/perf/arm_brbe.c
> @@ -604,7 +604,7 @@ static bool perf_entry_from_brbe_regset(int index, struct perf_branch_entry *ent
>   		return false;
>   
>   	brbinf = bregs.brbinf;
> -	perf_clear_branch_entry_bitfields(entry);
> +	perf_clear_branch_entry(entry);
>   	if (brbe_record_is_complete(brbinf)) {
>   		entry->from = bregs.brbsrc;
>   		entry->to = bregs.brbtgt;
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 48d851fbd8ea..e034be4a473a 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1467,21 +1467,9 @@ static inline u32 perf_sample_data_size(struct perf_sample_data *data,
>   	return size;
>   }
>   
> -/*
> - * Clear all bitfields in the perf_branch_entry.
> - * The to and from fields are not cleared because they are
> - * systematically modified by caller.
> - */
> -static inline void perf_clear_branch_entry_bitfields(struct perf_branch_entry *br)
> +static inline void perf_clear_branch_entry(struct perf_branch_entry *br)
>   {
> -	br->mispred	= 0;
> -	br->predicted	= 0;
> -	br->in_tx	= 0;
> -	br->abort	= 0;
> -	br->cycles	= 0;
> -	br->type	= 0;
> -	br->spec	= PERF_BR_SPEC_NA;
> -	br->reserved	= 0;
> +	*br = (struct perf_branch_entry){ };
>   }
>   
>   extern void perf_output_sample(struct perf_output_handle *handle,

Reviewed-by: James Clark <james.clark@linaro.org>