Re: [RFC v2] perf: Rewrite core context handling

[Ravi Bangoria <ravi.bangoria@amd.com>]

On 24-Aug-22 8:29 PM, Peter Zijlstra wrote:
> On Wed, Aug 24, 2022 at 02:15:13PM +0200, Peter Zijlstra wrote:
>> On Mon, Jun 13, 2022 at 04:35:11PM +0200, Peter Zijlstra wrote:
>>>  void x86_pmu_update_cpu_context(struct pmu *pmu, int cpu)
>>>  {
>>> -	struct perf_cpu_context *cpuctx;
>>> +	/* XXX: Don't need this quirk anymore */
>>> +	/*struct perf_cpu_context *cpuctx;
>>>  
>>>  	if (!pmu->pmu_cpu_context)
>>>  		return;
>>>  
>>>  	cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
>>> -	cpuctx->ctx.pmu = pmu;
>>> +	cpuctx->ctx.pmu = pmu;*/
>>>  }
>>
>> Confirmed; my ADL seems to work fine without all that.
> 
> Additionally; this doesn't insta crash.

While collating this I came across armv8pmu_start() which does:

	struct perf_event_context *task_ctx =
		this_cpu_ptr(cpu_pmu->pmu.pmu_cpu_context)->task_ctx;

	if (sysctl_perf_user_access && task_ctx && task_ctx->nr_user)

Not sure why it does not lock task_ctx. Should it be changed to
something like below? Untested:

---
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 016072a89f8f..747415a5f2b2 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -806,10 +806,19 @@ static void armv8pmu_disable_event(struct perf_event *event)
 
 static void armv8pmu_start(struct arm_pmu *cpu_pmu)
 {
-	struct perf_event_context *task_ctx =
-		this_cpu_ptr(cpu_pmu->pmu.pmu_cpu_context)->task_ctx;
+	struct perf_event_context *ctx;
+	int nr_user = 0;
+
+	rcu_read_lock();
+	ctx = rcu_dereference(current->perf_event_ctxp);
+	if (ctx) {
+		raw_spin_lock(&ctx->lock);
+		nr_user = ctx->nr_user;
+		raw_spin_unlock(&ctx->lock);
+	}
+	rcu_read_unlock();
 
-	if (sysctl_perf_user_access && task_ctx && task_ctx->nr_user)
+	if (sysctl_perf_user_access && nr_user)
 		armv8pmu_enable_user_access(cpu_pmu);
 	else
 		armv8pmu_disable_user_access();
---

Thanks,
Ravi