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

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 8/11/2026 4:22 PM, Peter Zijlstra wrote:
> On Tue, Aug 11, 2026 at 09:39:23AM +0800, Mi, Dapeng wrote:
>> On 8/10/2026 9:01 PM, Peter Zijlstra wrote:
>>> On Fri, Jul 17, 2026 at 04:03:42PM +0800, Dapeng Mi wrote:
>>>> The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
>>>> not run concurrently. If it is reentered, one instance may observe stale
>>>> buffer state and potentially access out-of-bound memory.
>>>>
>>>> Most invocations happen in NMI context, which naturally prevents reentry.
>>>> However, drain_pebs() is also reachable from process context via
>>>> intel_pmu_drain_pebs_buffer().
>>>>
>>>> In those paths, the PMU is often already disabled, but not guaranteed.
>>>> For example, __intel_pmu_pebs_disable() only disables the target counter,
>>>> so other active counters can still raise a PMI and interrupt an in-flight
>>>> drain_pebs().
>>>>
>>>> Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
>>>> use them in intel_pmu_drain_pebs_buffer() to disable the full PMU
>>>> around the drain_pebs() call, preventing reentry.
>>>>
>>> It is not at all clear to me where the exact recursion happens. (The
>>> word you're looking for was recursion, not concurrent).
>> Yes, the word "concurrently" is not accurate, reentry is the more accurate
>> word.
>>
>> Currently drain_pebs() would be called in two places, one is the in the PMI
>> handler, like handle_pmi_common(). The other place is
>> intel_pmu_drain_pebs_buffer() which is from process context.
>>
>> So when intel_pmu_drain_pebs_buffer() is calling drain_pebs(), if there is
>> an active PEBS event triggering PMI, it would interrupt current in-flight
>> drain_pebs() and lead to drain_pebs() reentry.
> What is the actual callchain here?
>
> Because the one I'm thinking off is:
>
>   perf_ctx_disable();
>   perf_ctx_sched_task_cb()
>     x86_pmu_sched_task()
>       intel_pmu_sched_task()
> 	intel_pmu_pebs_sched_task()
> 	  intel_pmu_drain_pebs_buffer();
>   perf_ctx_enable();
>
> And that one has the full pmu disabled, because of context switch etc.
> And then there is one in perf_read():
>
>   pmu->read()
>     intel_pmu_read_event()
>       pmu_enabled = cpuc->enabled;
>       cpuc->enabled = 0;
>       if (pmu_enabled)
>         intel_pmu_disable_all();
>       intel_pmu_drain_pebs_buffer();
>       cpuc->enabled = pmu_enabled;
>       if (pmu_enabled)
>         intel_pmu_enable_all();
>
> So what specific callchain is going sideways?

Yes, the whole PMU would be disabled for these 2 call-chains.

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()


>