ETM perf callbacks currently use the per-CPU csdev_src pointer, which can race with updates during device registration and unregistration.
The AUX setup already builds and stores the path in the event data. Use this path to retrieve the source instead of csdev_src to avoid the race.
Export coresight_get_source() and add etm_event_get_ctxt_path() to retrieve the context's path and its source with READ_ONCE() / WRITE_ONCE() accessors. Give the comments to explain why this approach is safe when pause or resume callbacks preempt the disable callback (e.g. via NMI).
Reviewed-by: Yeoreum Yun yeoreum.yun@arm.com Reviewed-by: James Clark james.clark@linaro.org Tested-by: James Clark james.clark@linaro.org Tested-by: Jie Gan jie.gan@oss.qualcomm.com Signed-off-by: Leo Yan leo.yan@arm.com --- drivers/hwtracing/coresight/coresight-core.c | 2 +- drivers/hwtracing/coresight/coresight-etm-perf.c | 114 ++++++++++++++--------- drivers/hwtracing/coresight/coresight-priv.h | 1 + 3 files changed, 74 insertions(+), 43 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index 703cc4e349f7703a4b38ba897909e03dc9c617a6..5519d323f21f38d719b9030c7d77a9a61948ba1d 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -82,7 +82,7 @@ struct coresight_device *coresight_get_percpu_sink(int cpu) } EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
-static struct coresight_device *coresight_get_source(struct coresight_path *path) +struct coresight_device *coresight_get_source(struct coresight_path *path) { struct coresight_device *csdev;
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index cae6738e9906c35d291e4b0f3feae37306b95c06..206a983239a0b1983b2b78c5e78a5fd988d308ce 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -315,6 +315,35 @@ static bool sinks_compatible(struct coresight_device *a, (sink_ops(a) == sink_ops(b)); }
+/* + * This helper is used for fetching the path pointer via the ctxt. + * + * Perf event callbacks run on the same CPU in atomic context, but AUX pause + * and resume may run in NMI context and preempt other callbacks. Since the + * event stop callback clears ctxt->event_data before the data is released, + * AUX pause/resume will either observe a NULL pointer and stop fetching the + * path pointer, or safely access event_data and the path, as the data has + * not yet been freed. + */ +static struct coresight_path *etm_event_get_ctxt_path(struct etm_ctxt *ctxt) +{ + struct etm_event_data *event_data; + struct coresight_path *path; + + if (!ctxt) + return NULL; + + event_data = READ_ONCE(ctxt->event_data); + if (!event_data) + return NULL; + + path = etm_event_cpu_path(event_data, smp_processor_id()); + if (!path) + return NULL; + + return path; +} + static void *etm_setup_aux(struct perf_event *event, void **pages, int nr_pages, bool overwrite) { @@ -464,13 +493,23 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, goto out; }
-static int etm_event_resume(struct coresight_device *csdev, - struct etm_ctxt *ctxt) +static int etm_event_resume(struct coresight_path *path) { - if (!ctxt->event_data) + struct coresight_device *source; + int ret; + + if (!path) return 0;
- return coresight_resume_source(csdev); + source = coresight_get_source(path); + if (!source) + return 0; + + ret = coresight_resume_source(source); + if (ret < 0) + dev_err(&source->dev, "Failed to resume ETM event.\n"); + + return ret; }
static void etm_event_start(struct perf_event *event, int flags) @@ -479,23 +518,19 @@ static void etm_event_start(struct perf_event *event, int flags) struct etm_event_data *event_data; struct etm_ctxt *ctxt = this_cpu_ptr(&etm_ctxt); struct perf_output_handle *handle = &ctxt->handle; - struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); + struct coresight_device *source, *sink; struct coresight_path *path; u64 hw_id;
- if (!csdev) - goto fail; - if (flags & PERF_EF_RESUME) { - if (etm_event_resume(csdev, ctxt) < 0) { - dev_err(&csdev->dev, "Failed to resume ETM event.\n"); + path = etm_event_get_ctxt_path(ctxt); + if (etm_event_resume(path) < 0) goto fail; - } return; }
/* Have we messed up our tracking ? */ - if (WARN_ON(ctxt->event_data)) + if (WARN_ON(READ_ONCE(ctxt->event_data))) goto fail;
/* @@ -523,9 +558,10 @@ static void etm_event_start(struct perf_event *event, int flags)
path = etm_event_cpu_path(event_data, cpu); path->handle = handle; - /* We need a sink, no need to continue without one */ + /* We need source and sink, no need to continue if any is not set */ + source = coresight_get_source(path); sink = coresight_get_sink(path); - if (WARN_ON_ONCE(!sink)) + if (WARN_ON_ONCE(!source || !sink)) goto fail_end_stop;
/* Nothing will happen without a path */ @@ -533,7 +569,7 @@ static void etm_event_start(struct perf_event *event, int flags) goto fail_end_stop;
/* Finally enable the tracer */ - if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF, path)) + if (source_ops(source)->enable(source, event, CS_MODE_PERF, path)) goto fail_disable_path;
/* @@ -557,7 +593,7 @@ static void etm_event_start(struct perf_event *event, int flags) /* Tell the perf core the event is alive */ event->hw.state = 0; /* Save the event_data for this ETM */ - ctxt->event_data = event_data; + WRITE_ONCE(ctxt->event_data, event_data); return;
fail_disable_path: @@ -577,27 +613,26 @@ static void etm_event_start(struct perf_event *event, int flags) return; }
-static void etm_event_pause(struct perf_event *event, - struct coresight_device *csdev, +static void etm_event_pause(struct coresight_path *path, + struct perf_event *event, struct etm_ctxt *ctxt) { - int cpu = smp_processor_id(); - struct coresight_device *sink; struct perf_output_handle *handle = &ctxt->handle; - struct coresight_path *path; + struct coresight_device *source, *sink; + struct etm_event_data *event_data; unsigned long size;
- if (!ctxt->event_data) + if (!path) return;
- /* Stop tracer */ - coresight_pause_source(csdev); - - path = etm_event_cpu_path(ctxt->event_data, cpu); + source = coresight_get_source(path); sink = coresight_get_sink(path); - if (WARN_ON_ONCE(!sink)) + if (WARN_ON_ONCE(!source || !sink)) return;
+ /* Stop tracer */ + coresight_pause_source(source); + /* * The per CPU sink has own interrupt handling, it might have * race condition with updating buffer on AUX trace pause if @@ -613,8 +648,9 @@ static void etm_event_pause(struct perf_event *event, if (!sink_ops(sink)->update_buffer) return;
+ event_data = READ_ONCE(ctxt->event_data); size = sink_ops(sink)->update_buffer(sink, handle, - ctxt->event_data->snk_config); + event_data->snk_config); if (READ_ONCE(handle->event)) { if (!size) return; @@ -630,14 +666,14 @@ static void etm_event_stop(struct perf_event *event, int mode) { int cpu = smp_processor_id(); unsigned long size; - struct coresight_device *sink, *csdev = per_cpu(csdev_src, cpu); + struct coresight_device *source, *sink; struct etm_ctxt *ctxt = this_cpu_ptr(&etm_ctxt); struct perf_output_handle *handle = &ctxt->handle; + struct coresight_path *path = etm_event_get_ctxt_path(ctxt); struct etm_event_data *event_data; - struct coresight_path *path;
if (mode & PERF_EF_PAUSE) - return etm_event_pause(event, csdev, ctxt); + return etm_event_pause(path, event, ctxt);
/* * If we still have access to the event_data via handle, @@ -647,9 +683,9 @@ static void etm_event_stop(struct perf_event *event, int mode) WARN_ON(perf_get_aux(handle) != ctxt->event_data)) return;
- event_data = ctxt->event_data; + event_data = READ_ONCE(ctxt->event_data); /* Clear the event_data as this ETM is stopping the trace. */ - ctxt->event_data = NULL; + WRITE_ONCE(ctxt->event_data, NULL);
if (event->hw.state == PERF_HES_STOPPED) return; @@ -671,19 +707,13 @@ static void etm_event_stop(struct perf_event *event, int mode) return; }
- if (!csdev) - return; - - path = etm_event_cpu_path(event_data, cpu); - if (!path) - return; - + source = coresight_get_source(path); sink = coresight_get_sink(path); - if (!sink) + if (!source || !sink) return;
/* stop tracer */ - coresight_disable_source(csdev, event); + coresight_disable_source(source, event);
/* tell the core */ event->hw.state = PERF_HES_STOPPED; diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 1ea882dffd703b2873e41b4ce0c2564d2ce9bbad..a0cd1a0ad2a182f762cc9721fd93725f2d4ef688 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -248,6 +248,7 @@ void coresight_add_helper(struct coresight_device *csdev,
void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev); struct coresight_device *coresight_get_percpu_sink(int cpu); +struct coresight_device *coresight_get_source(struct coresight_path *path); void coresight_disable_source(struct coresight_device *csdev, void *data); void coresight_pause_source(struct coresight_device *csdev); int coresight_resume_source(struct coresight_device *csdev);