kernel/events/core.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 3172fb986666dfb71bf483b6d3539e1e587fa197
Gitweb: https://git.kernel.org/tip/3172fb986666dfb71bf483b6d3539e1e587fa197
Author: Luo Gengkun <luogengkun@huaweicloud.com>
AuthorDate: Wed, 04 Jun 2025 03:39:24
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Thu, 05 Jun 2025 14:37:52 +02:00
perf/core: Fix WARN in perf_cgroup_switch()
There may be concurrency between perf_cgroup_switch and
perf_cgroup_event_disable. Consider the following scenario: after a new
perf cgroup event is created on CPU0, the new event may not trigger
a reprogramming, causing ctx->is_active to be 0. In this case, when CPU1
disables this perf event, it executes __perf_remove_from_context->
list _del_event->perf_cgroup_event_disable on CPU1, which causes a race
with perf_cgroup_switch running on CPU0.
The following describes the details of this concurrency scenario:
CPU0 CPU1
perf_cgroup_switch:
...
# cpuctx->cgrp is not NULL here
if (READ_ONCE(cpuctx->cgrp) == NULL)
return;
perf_remove_from_context:
...
raw_spin_lock_irq(&ctx->lock);
...
# ctx->is_active == 0 because reprogramm is not
# tigger, so CPU1 can do __perf_remove_from_context
# for CPU0
__perf_remove_from_context:
perf_cgroup_event_disable:
...
if (--ctx->nr_cgroups)
...
# this warning will happened because CPU1 changed
# ctx.nr_cgroups to 0.
WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
[peterz: use guard instead of goto unlock]
Fixes: db4a835601b7 ("perf/core: Set cgroup in CPU contexts for new cgroup events")
Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20250604033924.3914647-3-luogengkun@huaweicloud.com
---
kernel/events/core.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d786083..d7cf008 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -207,6 +207,19 @@ static void perf_ctx_unlock(struct perf_cpu_context *cpuctx,
__perf_ctx_unlock(&cpuctx->ctx);
}
+typedef struct {
+ struct perf_cpu_context *cpuctx;
+ struct perf_event_context *ctx;
+} class_perf_ctx_lock_t;
+
+static inline void class_perf_ctx_lock_destructor(class_perf_ctx_lock_t *_T)
+{ perf_ctx_unlock(_T->cpuctx, _T->ctx); }
+
+static inline class_perf_ctx_lock_t
+class_perf_ctx_lock_constructor(struct perf_cpu_context *cpuctx,
+ struct perf_event_context *ctx)
+{ perf_ctx_lock(cpuctx, ctx); return (class_perf_ctx_lock_t){ cpuctx, ctx }; }
+
#define TASK_TOMBSTONE ((void *)-1L)
static bool is_kernel_event(struct perf_event *event)
@@ -944,7 +957,13 @@ static void perf_cgroup_switch(struct task_struct *task)
if (READ_ONCE(cpuctx->cgrp) == cgrp)
return;
- perf_ctx_lock(cpuctx, cpuctx->task_ctx);
+ guard(perf_ctx_lock)(cpuctx, cpuctx->task_ctx);
+ /*
+ * Re-check, could've raced vs perf_remove_from_context().
+ */
+ if (READ_ONCE(cpuctx->cgrp) == NULL)
+ return;
+
perf_ctx_disable(&cpuctx->ctx, true);
ctx_sched_out(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP);
@@ -962,7 +981,6 @@ static void perf_cgroup_switch(struct task_struct *task)
ctx_sched_in(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP);
perf_ctx_enable(&cpuctx->ctx, true);
- perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
}
static int perf_cgroup_ensure_storage(struct perf_event *event,
On 2025/6/11 17:29, tip-bot2 for Luo Gengkun wrote:
> The following commit has been merged into the perf/urgent branch of tip:
>
> Commit-ID: 3172fb986666dfb71bf483b6d3539e1e587fa197
> Gitweb: https://git.kernel.org/tip/3172fb986666dfb71bf483b6d3539e1e587fa197
> Author: Luo Gengkun <luogengkun@huaweicloud.com>
> AuthorDate: Wed, 04 Jun 2025 03:39:24
> Committer: Peter Zijlstra <peterz@infradead.org>
> CommitterDate: Thu, 05 Jun 2025 14:37:52 +02:00
>
> perf/core: Fix WARN in perf_cgroup_switch()
>
> There may be concurrency between perf_cgroup_switch and
> perf_cgroup_event_disable. Consider the following scenario: after a new
> perf cgroup event is created on CPU0, the new event may not trigger
> a reprogramming, causing ctx->is_active to be 0. In this case, when CPU1
> disables this perf event, it executes __perf_remove_from_context->
> list _del_event->perf_cgroup_event_disable on CPU1, which causes a race
> with perf_cgroup_switch running on CPU0.
>
> The following describes the details of this concurrency scenario:
>
> CPU0 CPU1
>
> perf_cgroup_switch:
> ...
> # cpuctx->cgrp is not NULL here
> if (READ_ONCE(cpuctx->cgrp) == NULL)
> return;
>
> perf_remove_from_context:
> ...
> raw_spin_lock_irq(&ctx->lock);
> ...
> # ctx->is_active == 0 because reprogramm is not
> # tigger, so CPU1 can do __perf_remove_from_context
> # for CPU0
> __perf_remove_from_context:
> perf_cgroup_event_disable:
> ...
> if (--ctx->nr_cgroups)
> ...
>
> # this warning will happened because CPU1 changed
> # ctx.nr_cgroups to 0.
> WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
>
> [peterz: use guard instead of goto unlock]
> Fixes: db4a835601b7 ("perf/core: Set cgroup in CPU contexts for new cgroup events")
> Signed-off-by: Luo Gengkun <luogengkun@huaweicloud.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20250604033924.3914647-3-luogengkun@huaweicloud.com
Sorry for the late reply, I found that the link is v2 instead of v3.
This v3 link is:
https://lore.kernel.org/all/20250609035316.250557-1-luogengkun@huaweicloud.com/
v2 attempts to fix a concurrency problem between perf_cgroup_switch
and perf_cgroup_event_disable. But it does not to move WARN_ON_ONCE
into lock-protected region, so the warning is still triggered.
The following patches have been tested and fix this issue.
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1f746469fda5..a35784d42c66 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -951,8 +951,6 @@ static void perf_cgroup_switch(struct task_struct *task)
if (READ_ONCE(cpuctx->cgrp) == NULL)
return;
- WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
-
cgrp = perf_cgroup_from_task(task, NULL);
if (READ_ONCE(cpuctx->cgrp) == cgrp)
return;
@@ -964,6 +962,8 @@ static void perf_cgroup_switch(struct task_struct *task)
if (READ_ONCE(cpuctx->cgrp) == NULL)
return;
+ WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
+
perf_ctx_disable(&cpuctx->ctx, true);
ctx_sched_out(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP);
> ---
> kernel/events/core.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index d786083..d7cf008 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -207,6 +207,19 @@ static void perf_ctx_unlock(struct perf_cpu_context *cpuctx,
> __perf_ctx_unlock(&cpuctx->ctx);
> }
>
> +typedef struct {
> + struct perf_cpu_context *cpuctx;
> + struct perf_event_context *ctx;
> +} class_perf_ctx_lock_t;
> +
> +static inline void class_perf_ctx_lock_destructor(class_perf_ctx_lock_t *_T)
> +{ perf_ctx_unlock(_T->cpuctx, _T->ctx); }
> +
> +static inline class_perf_ctx_lock_t
> +class_perf_ctx_lock_constructor(struct perf_cpu_context *cpuctx,
> + struct perf_event_context *ctx)
> +{ perf_ctx_lock(cpuctx, ctx); return (class_perf_ctx_lock_t){ cpuctx, ctx }; }
> +
> #define TASK_TOMBSTONE ((void *)-1L)
>
> static bool is_kernel_event(struct perf_event *event)
> @@ -944,7 +957,13 @@ static void perf_cgroup_switch(struct task_struct *task)
> if (READ_ONCE(cpuctx->cgrp) == cgrp)
> return;
>
> - perf_ctx_lock(cpuctx, cpuctx->task_ctx);
> + guard(perf_ctx_lock)(cpuctx, cpuctx->task_ctx);
> + /*
> + * Re-check, could've raced vs perf_remove_from_context().
> + */
> + if (READ_ONCE(cpuctx->cgrp) == NULL)
> + return;
> +
> perf_ctx_disable(&cpuctx->ctx, true);
>
> ctx_sched_out(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP);
> @@ -962,7 +981,6 @@ static void perf_cgroup_switch(struct task_struct *task)
> ctx_sched_in(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP);
>
> perf_ctx_enable(&cpuctx->ctx, true);
> - perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
> }
>
> static int perf_cgroup_ensure_storage(struct perf_event *event,
On Thu, Jun 26, 2025 at 09:08:38PM +0800, Luo Gengkun wrote: > Sorry for the late reply, I found that the link is v2 instead of v3. > This v3 link is: > > https://lore.kernel.org/all/20250609035316.250557-1-luogengkun@huaweicloud.com/ > > v2 attempts to fix a concurrency problem between perf_cgroup_switch > and perf_cgroup_event_disable. But it does not to move WARN_ON_ONCE > into lock-protected region, so the warning is still triggered. > > The following patches have been tested and fix this issue. > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 1f746469fda5..a35784d42c66 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -951,8 +951,6 @@ static void perf_cgroup_switch(struct task_struct *task) > if (READ_ONCE(cpuctx->cgrp) == NULL) > return; > > - WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0); > - > cgrp = perf_cgroup_from_task(task, NULL); > if (READ_ONCE(cpuctx->cgrp) == cgrp) > return; > @@ -964,6 +962,8 @@ static void perf_cgroup_switch(struct task_struct *task) > if (READ_ONCE(cpuctx->cgrp) == NULL) > return; > > + WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0); > + > perf_ctx_disable(&cpuctx->ctx, true); > > ctx_sched_out(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP); Can you send as a full new patch, the thing is already in Linus' tree.
© 2016 - 2025 Red Hat, Inc.