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

[Liang, Kan]

> 
> 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.

The task_ctx_data will be checked before use. So it wouldn't crash the
system if it's NULL.
The problem is that LBR stack info will not be save/store on context switch
anymore. The user probably get wrong call stack information.
May I know why you want to do that?

Thanks,
Kan


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/