Re: [PATCH V10 3/7] perf: attach/detach PMU specific data

Peter Zijlstra
2025-03-14T21:05:54Z

On Fri, Mar 14, 2025 at 10:26:56AM -0700, kan.liang@linux.intel.com wrote:

> @@ -5393,6 +5607,9 @@ static void __free_event(struct perf_event *event)
> if (is_cgroup_event(event))
> perf_detach_cgroup(event);
>
> + if (event->attach_state & PERF_ATTACH_TASK_DATA)
> + detach_perf_ctx_data(event);
> +
> if (event->destroy)
> event->destroy(event);
>

> @@ -12481,6 +12746,18 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
> if (IS_ERR(pmu))
> return (void*)pmu;
>
> + /*
> + * The PERF_ATTACH_TASK_DATA is set in the event_init()->hw_config().
> + * The attach should be right after the perf_init_event().
> + * Otherwise, the __free_event() would mistakenly detach the non-exist
> + * perf_ctx_data because of the other errors between them.
> + */
> + if (event->attach_state & PERF_ATTACH_TASK_DATA) {
> + err = attach_perf_ctx_data(event);
> + if (err)
> + return ERR_PTR(err);
> + }
> +
> /*
> * Disallow uncore-task events. Similarly, disallow uncore-cgroup
> * events (they don't make sense as the cgroup will be different

I've stuck this on top. Let me see if it all compiles and push out to
queue/perf/core.

--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -666,11 +666,12 @@ struct swevent_hlist {
#define PERF_ATTACH_GROUP 0x0002
#define PERF_ATTACH_TASK 0x0004
#define PERF_ATTACH_TASK_DATA 0x0008
-#define PERF_ATTACH_ITRACE 0x0010
+#define PERF_ATTACH_GLOBAL_DATA 0x0010
#define PERF_ATTACH_SCHED_CB 0x0020
#define PERF_ATTACH_CHILD 0x0040
#define PERF_ATTACH_EXCLUSIVE 0x0080
#define PERF_ATTACH_CALLCHAIN 0x0100
+#define PERF_ATTACH_ITRACE 0x0200

struct bpf_prog;
struct perf_cgroup;
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5278,14 +5278,19 @@ attach_perf_ctx_data(struct perf_event *
{
struct task_struct *task = event->hw.target;
struct kmem_cache *ctx_cache = event->pmu->task_ctx_cache;
+ int ret;

if (!ctx_cache)
return -ENOMEM;

if (task)
return attach_task_ctx_data(task, ctx_cache, false);
- else
- return attach_global_ctx_data(ctx_cache);
+
+ ret = attach_global_ctx_data(ctx_cache);
+ if (ret)
+ return ret;
+
+ event->attach_state |= PERF_ATTACH_GLOBAL_DATA;
}

static void
@@ -5348,13 +5353,15 @@ static void detach_perf_ctx_data(struct
{
struct task_struct *task = event->hw.target;

- if (!event->pmu->task_ctx_cache)
- return;
+ event->attach_state &= ~PERF_ATTACH_TASK_DATA;

if (task)
- detach_task_ctx_data(task);
- else
+ return detach_task_ctx_data(task);
+
+ if (event->attach_state & PERF_ATTACH_GLOBAL_DATA) {
detach_global_ctx_data();
+ event->attach_state &= ~PERF_ATTACH_GLOBAL_DATA;
+ }
}

static void unaccount_event(struct perf_event *event)