Re: [tip:perf/core] perf: Add pmu specific data for perf task context

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Feb 18, 2015 at 09:15:06AM -0800, tip-bot for Yan, Zheng wrote:
> +find_get_context(struct pmu *pmu, struct task_struct *task,
> +		struct perf_event *event)
>  {
>  	struct perf_event_context *ctx, *clone_ctx = NULL;
>  	struct perf_cpu_context *cpuctx;
> +	void *task_ctx_data = NULL;
>  	unsigned long flags;
>  	int ctxn, err;
> +	int cpu = event->cpu;
>  
>  	if (!task) {
>  		/* Must be root to operate on a CPU event: */
> @@ -3342,11 +3354,24 @@ find_get_context(struct pmu *pmu, struct task_struct *task, int cpu)
>  	if (ctxn < 0)
>  		goto errout;
>  
> +	if (event->attach_state & PERF_ATTACH_TASK_DATA) {
> +		task_ctx_data = kzalloc(pmu->task_ctx_size, GFP_KERNEL);
> +		if (!task_ctx_data) {
> +			err = -ENOMEM;
> +			goto errout;
> +		}
> +	}
> +
>  retry:
>  	ctx = perf_lock_task_context(task, ctxn, &flags);
>  	if (ctx) {
>  		clone_ctx = unclone_ctx(ctx);
>  		++ctx->pin_count;
> +
> +		if (task_ctx_data && !ctx->task_ctx_data) {
> +			ctx->task_ctx_data = task_ctx_data;
> +			task_ctx_data = NULL;
> +		}
>  		raw_spin_unlock_irqrestore(&ctx->lock, flags);
>  
>  		if (clone_ctx)
> @@ -3357,6 +3382,11 @@ retry:
>  		if (!ctx)
>  			goto errout;
>  
> +		if (task_ctx_data) {
> +			ctx->task_ctx_data = task_ctx_data;
> +			task_ctx_data = NULL;
> +		}
> +
>  		err = 0;
>  		mutex_lock(&task->perf_event_mutex);
>  		/*
> @@ -3383,9 +3413,11 @@ retry:
>  		}
>  	}
>  
> +	kfree(task_ctx_data);
>  	return ctx;
>  
>  errout:
> +	kfree(task_ctx_data);
>  	return ERR_PTR(err);
>  }


diff --git a/kernel/events/core.c b/kernel/events/core.c
index 36babfd..97aa610 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3508,11 +3515,6 @@ retry:
 		if (!ctx)
 			goto errout;
 
-		if (task_ctx_data) {
-			ctx->task_ctx_data = task_ctx_data;
-			task_ctx_data = NULL;
-		}
-
 		err = 0;
 		mutex_lock(&task->perf_event_mutex);
 		/*
@@ -3526,6 +3528,10 @@ retry:
 		else {
 			get_ctx(ctx);
 			++ctx->pin_count;
+			if (task_ctx_data) {
+				ctx->task_ctx_data = task_ctx_data;
+				task_ctx_data = NULL;
+			}
 			rcu_assign_pointer(task->perf_event_ctxp[ctxn], ctx);
 		}
 		mutex_unlock(&task->perf_event_mutex);


Does that make sense? No point in setting task_ctx_data if we're going
to free the ctx and try again.