* [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
@ 2026-08-13 6:43 Dapeng Mi
2026-08-13 7:06 ` sashiko-bot
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Dapeng Mi @ 2026-08-13 6:43 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin,
Andi Kleen, Eranian Stephane
Cc: linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
Falcon Thomas, Xudong Hao, Dapeng Mi
The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
not be reentered. If so, 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(). Here is an example,
__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()
Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
use them in intel_pmu_drain_large_pebs() to disable the full PMU
around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
not disabled.
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
---
v2: Move __intel_pmu_quiesce()/__intel_pmu_resume() pair into
intel_pmu_drain_large_pebs() instead of guarding all drain_pebs()
callbacks. (Peter)
v1: https://lore.kernel.org/all/20260717080342.1879573-9-dapeng1.mi@linux.intel.com/
arch/x86/events/intel/core.c | 33 ++++++++++++++++++++++++---------
arch/x86/events/intel/ds.c | 8 +++++++-
arch/x86/events/perf_event.h | 3 +++
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index f6ee1819864e..1cdb52c0aad5 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(int 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 fa381110f7a7..84495279079a 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1638,6 +1638,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(int pmu_enabled);
+
int intel_pmu_save_and_restart(struct perf_event *event);
struct event_constraint *
base-commit: 917d558b151cad5b05991e5eaee22efab33525ca
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
2026-08-13 6:43 [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry Dapeng Mi
@ 2026-08-13 7:06 ` sashiko-bot
2026-08-17 19:28 ` Ian Rogers
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-13 7:06 UTC (permalink / raw)
To: Dapeng Mi; +Cc: linux-perf-users
> The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
> not be reentered. If so, 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(). Here is an example,
>
> __perf_addr_filters_adjust()
> perf_event_stop()
> [ ... ]
>
> Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
> not disabled.
>
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813064346.335458-1-dapeng1.mi@linux.intel.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
2026-08-13 6:43 [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry Dapeng Mi
2026-08-13 7:06 ` sashiko-bot
@ 2026-08-17 19:28 ` Ian Rogers
2026-08-18 1:35 ` Mi, Dapeng
2026-09-09 11:09 ` Mi, Dapeng
2026-09-10 9:01 ` [tip: perf/urgent] " tip-bot2 for Dapeng Mi
3 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2026-08-17 19:28 UTC (permalink / raw)
To: Dapeng Mi
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Adrian Hunter, Alexander Shishkin, Andi Kleen,
Eranian Stephane, linux-kernel, linux-perf-users, Dapeng Mi,
Zide Chen, Falcon Thomas, Xudong Hao
On Wed, Aug 12, 2026 at 11:50 PM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote:
>
> The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
> not be reentered. If so, 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(). Here is an example,
>
> __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()
>
> Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
> use them in intel_pmu_drain_large_pebs() to disable the full PMU
> around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
>
> Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
> not disabled.
>
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>
> v2: Move __intel_pmu_quiesce()/__intel_pmu_resume() pair into
> intel_pmu_drain_large_pebs() instead of guarding all drain_pebs()
> callbacks. (Peter)
>
> v1: https://lore.kernel.org/all/20260717080342.1879573-9-dapeng1.mi@linux.intel.com/
>
> arch/x86/events/intel/core.c | 33 ++++++++++++++++++++++++---------
> arch/x86/events/intel/ds.c | 8 +++++++-
> arch/x86/events/perf_event.h | 3 +++
> 3 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index f6ee1819864e..1cdb52c0aad5 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)
Thanks Dapeng. Rather than quiesce would
__intel_pmu_disable_if_enabled be a more intention revealing name?
> +{
> + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> + int pmu_enabled = cpuc->enabled;
> +
> + cpuc->enabled = 0;
Presumably the line above could also be inside the if.
> + if (pmu_enabled)
> + intel_pmu_disable_all();
> +
> + return pmu_enabled;
> +}
> +
> +void __intel_pmu_resume(int 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;
nit: Maybe pmu_enabled would be better as pmu_was_enabled or pmu_reenable?
>
> /* 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();
Similarly, may be was_enabled rather than enabled.
Thanks,
Ian
> 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 fa381110f7a7..84495279079a 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -1638,6 +1638,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(int pmu_enabled);
> +
> int intel_pmu_save_and_restart(struct perf_event *event);
>
> struct event_constraint *
>
> base-commit: 917d558b151cad5b05991e5eaee22efab33525ca
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
2026-08-17 19:28 ` Ian Rogers
@ 2026-08-18 1:35 ` Mi, Dapeng
0 siblings, 0 replies; 8+ messages in thread
From: Mi, Dapeng @ 2026-08-18 1:35 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Adrian Hunter, Alexander Shishkin, Andi Kleen,
Eranian Stephane, linux-kernel, linux-perf-users, Dapeng Mi,
Zide Chen, Falcon Thomas, Xudong Hao
On 8/18/2026 3:28 AM, Ian Rogers wrote:
> On Wed, Aug 12, 2026 at 11:50 PM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote:
>> The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
>> not be reentered. If so, 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(). Here is an example,
>>
>> __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()
>>
>> Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
>> use them in intel_pmu_drain_large_pebs() to disable the full PMU
>> around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
>>
>> Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
>> not disabled.
>>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>> ---
>>
>> v2: Move __intel_pmu_quiesce()/__intel_pmu_resume() pair into
>> intel_pmu_drain_large_pebs() instead of guarding all drain_pebs()
>> callbacks. (Peter)
>>
>> v1: https://lore.kernel.org/all/20260717080342.1879573-9-dapeng1.mi@linux.intel.com/
>>
>> arch/x86/events/intel/core.c | 33 ++++++++++++++++++++++++---------
>> arch/x86/events/intel/ds.c | 8 +++++++-
>> arch/x86/events/perf_event.h | 3 +++
>> 3 files changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index f6ee1819864e..1cdb52c0aad5 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)
> Thanks Dapeng. Rather than quiesce would
> __intel_pmu_disable_if_enabled be a more intention revealing name?
Thanks Ian for reviewing this patch.
Yeah, I was hesitated about the name for a while. Currently I still prefer
the name __intel_pmu_quiesce() which is simplified enough than other names.
>
>> +{
>> + struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>> + int pmu_enabled = cpuc->enabled;
>> +
>> + cpuc->enabled = 0;
> Presumably the line above could also be inside the if.
Yes. I'm not sure if it's worthy to post a new version for this minor
change. But If there is a new version, I would change it.
>
>> + if (pmu_enabled)
>> + intel_pmu_disable_all();
>> +
>> + return pmu_enabled;
>> +}
>> +
>> +void __intel_pmu_resume(int 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;
> nit: Maybe pmu_enabled would be better as pmu_was_enabled or pmu_reenable?
pmu_enabled seems better for me. pmu_enabled is simplified enough comparing
with other names.
Thanks.
>
>> /* 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();
> Similarly, may be was_enabled rather than enabled.
>
> Thanks,
> Ian
>
>> 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 fa381110f7a7..84495279079a 100644
>> --- a/arch/x86/events/perf_event.h
>> +++ b/arch/x86/events/perf_event.h
>> @@ -1638,6 +1638,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(int pmu_enabled);
>> +
>> int intel_pmu_save_and_restart(struct perf_event *event);
>>
>> struct event_constraint *
>>
>> base-commit: 917d558b151cad5b05991e5eaee22efab33525ca
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
2026-08-13 6:43 [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry Dapeng Mi
2026-08-13 7:06 ` sashiko-bot
2026-08-17 19:28 ` Ian Rogers
@ 2026-09-09 11:09 ` Mi, Dapeng
2026-09-09 13:03 ` Peter Zijlstra
2026-09-10 9:01 ` [tip: perf/urgent] " tip-bot2 for Dapeng Mi
3 siblings, 1 reply; 8+ messages in thread
From: Mi, Dapeng @ 2026-09-09 11:09 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Ian Rogers, Adrian Hunter, Alexander Shishkin,
Andi Kleen, Eranian Stephane
Cc: linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
Falcon Thomas, Xudong Hao
Hi Peter,
Could you please review and queue this patch if it's good enough? This
version addresses your comments. Thanks.
On 8/13/2026 2:43 PM, Dapeng Mi wrote:
> The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
> not be reentered. If so, 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(). Here is an example,
>
> __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()
>
> Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
> use them in intel_pmu_drain_large_pebs() to disable the full PMU
> around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
>
> Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
> not disabled.
>
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> ---
>
> v2: Move __intel_pmu_quiesce()/__intel_pmu_resume() pair into
> intel_pmu_drain_large_pebs() instead of guarding all drain_pebs()
> callbacks. (Peter)
>
> v1: https://lore.kernel.org/all/20260717080342.1879573-9-dapeng1.mi@linux.intel.com/
>
> arch/x86/events/intel/core.c | 33 ++++++++++++++++++++++++---------
> arch/x86/events/intel/ds.c | 8 +++++++-
> arch/x86/events/perf_event.h | 3 +++
> 3 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index f6ee1819864e..1cdb52c0aad5 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(int 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 fa381110f7a7..84495279079a 100644
> --- a/arch/x86/events/perf_event.h
> +++ b/arch/x86/events/perf_event.h
> @@ -1638,6 +1638,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(int pmu_enabled);
> +
> int intel_pmu_save_and_restart(struct perf_event *event);
>
> struct event_constraint *
>
> base-commit: 917d558b151cad5b05991e5eaee22efab33525ca
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
2026-09-09 11:09 ` Mi, Dapeng
@ 2026-09-09 13:03 ` Peter Zijlstra
2026-09-10 0:02 ` Mi, Dapeng
0 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2026-09-09 13:03 UTC (permalink / raw)
To: Mi, Dapeng
Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
Falcon Thomas, Xudong Hao
On Wed, Sep 09, 2026 at 07:09:11PM +0800, Mi, Dapeng wrote:
> Hi Peter,
>
> Could you please review and queue this patch if it's good enough? This
> version addresses your comments. Thanks.
>
>
> On 8/13/2026 2:43 PM, Dapeng Mi wrote:
> > The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
> > not be reentered. If so, 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(). Here is an example,
> >
> > __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()
> >
> > Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
> > use them in intel_pmu_drain_large_pebs() to disable the full PMU
> > around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
> >
> > Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
> > not disabled.
> >
> > Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Ah yes. Can you get me a Fixes tag to go with this?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry
2026-09-09 13:03 ` Peter Zijlstra
@ 2026-09-10 0:02 ` Mi, Dapeng
0 siblings, 0 replies; 8+ messages in thread
From: Mi, Dapeng @ 2026-09-10 0:02 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Ian Rogers,
Adrian Hunter, Alexander Shishkin, Andi Kleen, Eranian Stephane,
linux-kernel, linux-perf-users, Dapeng Mi, Zide Chen,
Falcon Thomas, Xudong Hao
On 9/9/2026 9:03 PM, Peter Zijlstra wrote:
> On Wed, Sep 09, 2026 at 07:09:11PM +0800, Mi, Dapeng wrote:
>> Hi Peter,
>>
>> Could you please review and queue this patch if it's good enough? This
>> version addresses your comments. Thanks.
>>
>>
>> On 8/13/2026 2:43 PM, Dapeng Mi wrote:
>>> The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
>>> not be reentered. If so, 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(). Here is an example,
>>>
>>> __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()
>>>
>>> Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
>>> use them in intel_pmu_drain_large_pebs() to disable the full PMU
>>> around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
>>>
>>> Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
>>> not disabled.
>>>
>>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Ah yes. Can you get me a Fixes tag to go with this?
Sure. Here it is. Thanks.
Fixes: b752ea0c28e3 ("perf/x86/intel/ds: Flush PEBS DS when changing
PEBS_DATA_CFG")
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip: perf/urgent] perf/x86/intel: Prevent drain_pebs() reentry
2026-08-13 6:43 [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry Dapeng Mi
` (2 preceding siblings ...)
2026-09-09 11:09 ` Mi, Dapeng
@ 2026-09-10 9:01 ` tip-bot2 for Dapeng Mi
3 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Dapeng Mi @ 2026-09-10 9:01 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Dapeng Mi, Peter Zijlstra (Intel), x86, linux-kernel
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: a56c03a397e2cd0c4cf8da96dcd6214f7d0e7d8c
Gitweb: https://git.kernel.org/tip/a56c03a397e2cd0c4cf8da96dcd6214f7d0e7d8c
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
AuthorDate: Thu, 13 Aug 2026 14:43:46 +08:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 10 Sep 2026 10:22:50 +02:00
perf/x86/intel: Prevent drain_pebs() reentry
The PEBS buffer is shared by all events on a CPU, so drain_pebs() must
not be reentered. If so, 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(). Here is an example,
__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()
Introduce __intel_pmu_quiesce() and __intel_pmu_resume() helpers and
use them in intel_pmu_drain_large_pebs() to disable the full PMU
around the intel_pmu_drain_pebs_buffer() call, preventing reentry.
Also add a warning in intel_pmu_drain_pebs_buffer() when the full PMU is
not disabled.
Fixes: b752ea0c28e3 ("perf/x86/intel/ds: Flush PEBS DS when changing PEBS_DATA_CFG")
Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20260813064346.335458-1-dapeng1.mi@linux.intel.com
---
arch/x86/events/intel/core.c | 33 ++++++++++++++++++++++++---------
arch/x86/events/intel/ds.c | 8 +++++++-
arch/x86/events/perf_event.h | 3 +++
3 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index cc13164..1ac2ca3 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(int 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 d0bb767..b98029b 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1242,8 +1242,11 @@ unlock:
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 71ed5b2..4680cba 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1638,6 +1638,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(int pmu_enabled);
+
int intel_pmu_save_and_restart(struct perf_event *event);
struct event_constraint *
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-10 9:02 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 6:43 [Patch v2] perf/x86/intel: Prevent drain_pebs() reentry Dapeng Mi
2026-08-13 7:06 ` sashiko-bot
2026-08-17 19:28 ` Ian Rogers
2026-08-18 1:35 ` Mi, Dapeng
2026-09-09 11:09 ` Mi, Dapeng
2026-09-09 13:03 ` Peter Zijlstra
2026-09-10 0:02 ` Mi, Dapeng
2026-09-10 9:01 ` [tip: perf/urgent] " tip-bot2 for Dapeng Mi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.