Re: [Patch v3 8/8] perf/x86/intel: Prevent drain_pebs() reentry

[Mi, Dapeng]

On 8/12/2026 11:18 PM, Peter Zijlstra wrote:
> On Tue, Aug 11, 2026 at 06:00:43PM +0800, Mi, Dapeng wrote:
>
>>> So what specific callchain is going sideways?
>> But for the helper __intel_pmu_pebs_disable(), it seems the whole PMU is
>> not disabled, only the target counter has been stopped. Here is one call-chain.
>>
>> __perf_addr_filters_adjust()
>>   perf_event_stop()
>>     __perf_event_stop()
>>       x86_pmu_stop() (event->pmu->stop)
>>         intel_pmu_disable_event()
>>           intel_pmu_pebs_disable()
>>             __intel_pmu_pebs_disable()
>>               intel_pmu_drain_large_pebs()
>>                 intel_pmu_drain_pebs_buffer()
>>
> Right, so that needs to be included in the changelog. Also, lets target
> that more specifically.

Sure.

>
> How about something like so?

It looks good to me. Would post a new version. Thanks.


>
> ---
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index db92ded774dd..e092ef6e37f4 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -3125,6 +3125,27 @@ static void intel_pmu_del_event(struct perf_event *event)
>  		this_cpu_ptr(&cpu_hw_events)->n_late_setup--;
>  }
>  
> +int __intel_pmu_quiesce(void)
> +{
> +	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +	int pmu_enabled = cpuc->enabled;
> +
> +	cpuc->enabled = 0;
> +	if (pmu_enabled)
> +		intel_pmu_disable_all();
> +
> +	return pmu_enabled;
> +}
> +
> +void __intel_pmu_resume(bool pmu_enabled)
> +{
> +	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +
> +	cpuc->enabled = pmu_enabled;
> +	if (pmu_enabled)
> +		intel_pmu_enable_all(0);
> +}
> +
>  static int icl_set_topdown_event_period(struct perf_event *event)
>  {
>  	struct hw_perf_event *hwc = &event->hw;
> @@ -3316,16 +3337,13 @@ static void intel_pmu_read_event(struct perf_event *event)
>  	if (event->hw.flags & (PERF_X86_EVENT_AUTO_RELOAD | PERF_X86_EVENT_TOPDOWN) ||
>  	    is_pebs_counter_event_group(event)) {
>  		struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> -		bool pmu_enabled = cpuc->enabled;
> +		int pmu_enabled;
>  
>  		/* Only need to call update_topdown_event() once for group read. */
>  		if (is_metric_event(event) && (cpuc->txn_flags & PERF_PMU_TXN_READ))
>  			return;
>  
> -		cpuc->enabled = 0;
> -		if (pmu_enabled)
> -			intel_pmu_disable_all();
> -
> +		pmu_enabled = __intel_pmu_quiesce();
>  		/*
>  		 * If the PEBS counters snapshotting is enabled,
>  		 * the topdown event is available in PEBS records.
> @@ -3334,10 +3352,7 @@ static void intel_pmu_read_event(struct perf_event *event)
>  			static_call(intel_pmu_update_topdown_event)(event, NULL);
>  		else
>  			intel_pmu_drain_pebs_buffer();
> -
> -		cpuc->enabled = pmu_enabled;
> -		if (pmu_enabled)
> -			intel_pmu_enable_all(0);
> +		__intel_pmu_resume(pmu_enabled);
>  
>  		return;
>  	}
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index e86e4ba91e1b..54890dda0589 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -1242,8 +1242,11 @@ int intel_pmu_drain_bts_buffer(void)
>  
>  void intel_pmu_drain_pebs_buffer(void)
>  {
> +	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>  	struct perf_sample_data data;
>  
> +	WARN_ON_ONCE(cpuc->enabled);
> +
>  	static_call(x86_pmu_drain_pebs)(NULL, &data);
>  }
>  
> @@ -1864,8 +1867,11 @@ static void intel_pmu_pebs_via_pt_enable(struct perf_event *event)
>  static inline void intel_pmu_drain_large_pebs(struct cpu_hw_events *cpuc)
>  {
>  	if (cpuc->n_pebs == cpuc->n_large_pebs &&
> -	    cpuc->n_pebs != cpuc->n_pebs_via_pt)
> +	    cpuc->n_pebs != cpuc->n_pebs_via_pt) {
> +		int enabled = __intel_pmu_quiesce();
>  		intel_pmu_drain_pebs_buffer();
> +		__intel_pmu_resume(enabled);
> +	}
>  }
>  
>  static void __intel_pmu_pebs_enable(struct perf_event *event)
> diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
> index 8c0b32ffab35..c7848c4cabae 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -1644,6 +1644,9 @@ static __always_inline void __intel_pmu_lbr_disable(void)
>  	wrmsrq(MSR_IA32_DEBUGCTLMSR, debugctl);
>  }
>  
> +extern int __intel_pmu_quiesce(void);
> +extern void __intel_pmu_resume(bool pmu_enabled);
> +
>  int intel_pmu_save_and_restart(struct perf_event *event);
>  
>  struct event_constraint *
>