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

[Peter Zijlstra]

On Fri, Mar 14, 2025 at 10:26:56AM -0700, kan.liang@xxxxxxxxxxxxxxx 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)