Re: [PATCH v6 3/3] perf/core: Clear the whole branch entry in perf_clear_branch_entry()

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Aug 06, 2026 at 06:52:23AM -0700, Puranjay Mohan wrote:
> perf_clear_branch_entry_bitfields() clears the bitfields of struct
> perf_branch_entry one by one and leaves from/to alone, since callers
> overwrite those straight away. The list has to be kept in sync with the
> struct by hand and has already fallen behind: new_type and priv were
> added to perf_branch_entry and never added here.
> 
> Only BRBE writes those two, and neither is written for every record.
> brbe_set_perf_entry_type() leaves new_type alone for a branch type it
> does not recognise, and priv is not set for source-only records.
> arm_pmuv3.c allocates the per-CPU branch stack with kmalloc(), so such a
> record carries whatever the slot held: uninitialised kmalloc() data on
> the first pass over the buffer, the previous record's values after that.
> Both reach userspace through the branch stack. Nothing under
> arch/x86/events/ writes either field, so x86 is unaffected.
> 
> Clear the entry with a single struct assignment instead:
> 
> 	*br = (struct perf_branch_entry){ };
> 
> The bitfields add up to exactly 64 bits, so there is no padding, and
> every caller assigns from/to immediately afterwards, so zeroing those as
> well changes nothing. PERF_BR_SPEC_NA is 0, so dropping the explicit
> spec assignment leaves the behaviour unchanged. Nothing needs keeping in
> sync when a field is added.
> 
> The helper no longer touches only bitfields, so rename it to
> perf_clear_branch_entry().

Fair enough I suppose, but then why not write it like so?

---
--- a/arch/x86/events/amd/brs.c
+++ b/arch/x86/events/amd/brs.c
@@ -343,11 +343,7 @@ void amd_brs_drain(void)
 		if (!amd_brs_match_plm(event, from, to))
 			continue;
 
-		perf_clear_branch_entry_bitfields(br+nr);
-
-		br[nr].from = from;
-		br[nr].to   = to;
-
+		br[nr] = (struct perf_branch_entry){ from, to };
 		nr++;
 	}
 empty:
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -184,12 +184,6 @@ void amd_pmu_lbr_read(void)
 		    entry.to.split.reserved)
 			continue;
 
-		perf_clear_branch_entry_bitfields(br + out);
-
-		br[out].from	= sign_ext_branch_ip(entry.from.split.ip);
-		br[out].to	= sign_ext_branch_ip(entry.to.split.ip);
-		br[out].mispred	= entry.from.split.mispredict;
-		br[out].predicted = !br[out].mispred;
 
 		/*
 		 * Set branch speculation information using the status of
@@ -208,7 +202,13 @@ void amd_pmu_lbr_read(void)
 		 * speculative and took the correct path
 		 */
 		idx = (entry.to.split.valid << 1) | entry.to.split.spec;
-		br[out].spec = lbr_spec_map[idx];
+		br[out] = (struct perf_branch_entry) {
+			.from      = sign_ext_branch_ip(entry.from.split.ip),
+			.to        = sign_ext_branch_ip(entry.to.split.ip),
+			.mispred   = entry.from.split.mispredict,
+			.predicted = !entry.from.split.mispredict,
+			.spec      = lbr_spec_map[idx],
+		};
 		out++;
 	}
 
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -756,10 +756,10 @@ void intel_pmu_lbr_read_32(struct cpu_hw
 
 		rdmsrq(x86_pmu.lbr_from + lbr_idx, msr_lastbranch.lbr);
 
-		perf_clear_branch_entry_bitfields(br);
-
-		br->from	= msr_lastbranch.from;
-		br->to		= msr_lastbranch.to;
+		*br = (struct perf_branch_entry){
+			.from = msr_lastbranch.from,
+			.to   = msr_lastbranch.to,
+		};
 		br++;
 	}
 	cpuc->lbr_stack.nr = i;
@@ -847,14 +847,15 @@ void intel_pmu_lbr_read_64(struct cpu_hw
 		if (abort && x86_pmu.lbr_double_abort && out > 0)
 			out--;
 
-		perf_clear_branch_entry_bitfields(br+out);
-		br[out].from	 = from;
-		br[out].to	 = to;
-		br[out].mispred	 = mis;
-		br[out].predicted = pred;
-		br[out].in_tx	 = in_tx;
-		br[out].abort	 = abort;
-		br[out].cycles	 = cycles;
+		br[out] = (struct perf_branch_entry) {
+			.from      = from,
+			.to        = to,
+			.mispred   = mis,
+			.predicted = pred,
+			.in_tx     = in_tx,
+			.abort     = abort,
+			.cycles    = cycles,
+		};
 		out++;
 	}
 	cpuc->lbr_stack.nr = out;
@@ -921,24 +922,25 @@ static void intel_pmu_store_lbr(struct c
 		to = rdlbr_to(i, lbr);
 		info = rdlbr_info(i, lbr);
 
-		perf_clear_branch_entry_bitfields(e);
-
-		e->from		= from;
-		e->to		= to;
-		e->mispred	= get_lbr_mispred(info);
-		e->predicted	= !e->mispred;
-		e->in_tx	= !!(info & LBR_INFO_IN_TX);
-		e->abort	= !!(info & LBR_INFO_ABORT);
-		e->cycles	= get_lbr_cycles(info);
-		e->type		= get_lbr_br_type(info);
-
-		/*
-		 * Leverage the reserved field of cpuc->lbr_entries[i] to
-		 * temporarily store the branch counters information.
-		 * The later code will decide what content can be disclosed
-		 * to the perf tool. Pleae see intel_pmu_lbr_counters_reorder().
-		 */
-		e->reserved	= (info >> LBR_INFO_BR_CNTR_OFFSET) & LBR_INFO_BR_CNTR_FULL_MASK;
+		*e = (struct perf_branch_entry){
+			.from      = from,
+			.to        = to,
+			.mispred   = get_lbr_mispred(info),
+			.predicted = !get_lbr_mispred(info),
+			.in_tx     = !!(info & LBR_INFO_IN_TX),
+			.abort     = !!(info & LBR_INFO_ABORT),
+			.cycles    = get_lbr_cycles(info),
+			.type      = get_lbr_br_type(info),
+
+			/*
+			 * Leverage the reserved field of cpuc->lbr_entries[i]
+			 * to temporarily store the branch counters
+			 * information. The later code will decide what
+			 * content can be disclosed to the perf tool. Pleae
+			 * see intel_pmu_lbr_counters_reorder().
+			 */
+			.reserved  = (info >> LBR_INFO_BR_CNTR_OFFSET) & LBR_INFO_BR_CNTR_FULL_MASK,
+		};
 	}
 
 	cpuc->lbr_stack.nr = i;
--- a/drivers/perf/arm_brbe.c
+++ b/drivers/perf/arm_brbe.c
@@ -604,7 +604,7 @@ static bool perf_entry_from_brbe_regset(
 		return false;
 
 	brbinf = bregs.brbinf;
-	perf_clear_branch_entry_bitfields(entry);
+	*entry = (struct perf_branch_entry) { };
 	if (brbe_record_is_complete(brbinf)) {
 		entry->from = bregs.brbsrc;
 		entry->to = bregs.brbtgt;
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1467,23 +1467,6 @@ static inline u32 perf_sample_data_size(
 	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)
-{
-	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;
-}
-
 extern void perf_output_sample(struct perf_output_handle *handle,
 			       struct perf_event_header *header,
 			       struct perf_sample_data *data,