Re: [Patch v5 13/18] perf/x86/intel: Refresh LBR stack for non-PEBS LBR events if needed

[Mi, Dapeng]

On 10/29/2026 7:00 PM, Peter Zijlstra wrote:
> On Thu, Oct 29, 2026 at 12:39:09PM +0800, Dapeng Mi wrote:
>> PEBS and non-PEBS events currently share the same LBR stack,
>> cpuc->lbr_stack. In the PMI handler, the non-PEBS path refreshes the
>> LBR state before the PEBS buffer is drained.
>>
>> When PEBS and non-PEBS LBR events overflow in the same PMI, draining the
>> PEBS buffer can overwrite the shared LBR stack with the PEBS snapshot.
>> This leaves stale LBR data from the PEBS record in place, and userspace
>> may observe the wrong branch stack instead of the live LBR state at PMI
>> entry.
>>
>> Add a lbr_need_update flag to indicate when the LBR state must be
>> refreshed. If PEBS events and non-PEBS LBR events overflow in the same
>> PMI, set this flag so the non-PEBS LBR event refreshes the live LBR
>> state. Since the LBR state is frozen for the duration of PMI handling, it
>> only needs to be read once even when multiple non-PEBS LBR events
>> overflow in the same PMI.
>>
>> Fixes: c22497f5838c ("perf/x86/intel: Support adaptive PEBS v4")
>> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>> ---
>>  arch/x86/events/intel/core.c | 23 ++++++++++++++++++++---
>>  1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 110fff65bd50..37f304a1b48c 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -3863,8 +3863,9 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  {
>>  	struct perf_sample_data data;
>>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>> -	int bit;
>> +	bool lbr_need_update = false;
>>  	int handled = 0;
>> +	int bit;
>>  
>>  	inc_perf_irq_stat();
>>  
>> @@ -3929,6 +3930,8 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  		if (cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS] &&
>>  		    is_pebs_counter_event_group(cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS]))
>>  			status &= ~GLOBAL_STATUS_PERF_METRICS_OVF_BIT;
>> +
>> +		lbr_need_update = true;
>>  	}
>>  
>>  	/*
>> @@ -3944,6 +3947,8 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  		if (cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS] &&
>>  		    is_pebs_counter_event_group(cpuc->events[INTEL_PMC_IDX_FIXED_SLOTS]))
>>  			status &= ~GLOBAL_STATUS_PERF_METRICS_OVF_BIT;
>> +
>> +		lbr_need_update = true;
>>  	}
>>  
>>  	/*
>> @@ -4002,8 +4007,14 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  		 *
>>  		 * The PEBS buffer has to be drained before handling the A-PMI
>>  		 */
>> -		if (is_pebs_counter_event_group(event))
>> +		if (is_pebs_counter_event_group(event)) {
>>  			static_call(x86_pmu_drain_pebs)(regs, &data);
>> +			/*
>> +			 * Draining the PEBS buffer can overwrite the shared
>> +			 * LBR stack, so mark lbr_need_update as true.
>> +			 */
>> +			lbr_need_update = true;
>> +		}
>>  
>>  		last_period = event->hw.last_period;
>>  
>> @@ -4012,8 +4023,14 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>>  
>>  		perf_sample_data_init(&data, 0, last_period);
>>  
>> -		if (has_branch_stack(event))
>> +		if (has_branch_stack(event)) {
>> +			if (lbr_need_update) {
>> +				intel_pmu_lbr_read(&cpuc->lbr_stack,
>> +					cpuc->lbr_entries, cpuc->lbr_counters);
>> +				lbr_need_update = false;
>> +			}
>>  			intel_pmu_lbr_save_brstack(&data, cpuc, event);
>> +		}
>>  
>>  		x86_pmu_clear_perf_regs(regs);
>>  		x86_pmu_update_perf_regs(event, &data, regs, false);
> Or... you move intel_pmu_lbr_read() from handle_irq() into pmi_common()
> after we done the PEBS things?

Yes, I ever tried to do that, but I found it would break the PEBS LBR
sampling on the platforms prior to ICE LAKE. The PEBS doesn't support
snapshot LBR stack on these old platforms, so PMI handler has to snapshot
the LBR stack before draining the PEBS buffer.

See below code in setup_pebs_fixed_sample_data().

    /*
     * v3 supplies an accurate time stamp, so we use that
     * for the time stamp.
     *
     * We can only do this for the default trace clock.
     */
    if (x86_pmu.intel_cap.pebs_format >= 3)
        setup_pebs_time(event, data, pebs->tsc);

    perf_sample_save_brstack(data, event, &cpuc->lbr_stack, NULL);


Thanks.