Messages in this thread Patch in this message | | | Date | Thu, 25 Aug 2022 16:33:51 +0530 | From | Ravi Bangoria <> | Subject | Re: [RFC v2] perf: Rewrite core context handling |
| |
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
| |