Re: [PATCH v3 05/13] coresight: perf: Allow tracing on hotplugged CPUs

From: Suzuki K Poulose
Date: Tue Jul 31 2018 - 19:07:40 EST

On 07/31/2018 06:46 PM, Mathieu Poirier wrote:
On Thu, Jul 26, 2018 at 01:54:43PM +0100, Suzuki K Poulose wrote:
At the moment, if there is no CPU specified for a given
event, we use cpu_online_mask and try to build path for
each of the CPUs in the mask. This could prevent any CPU
that is turned online later to be used for the tracing.

This patch changes to use the cpu_present_mask and tries
to build path for as much CPUs as possible ignoring the
failures in building path for some of the CPUs. If ever
we try to trace on those CPUs, we fail the operation.

Based on a patch from Mathieu Poirier.

Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
drivers/hwtracing/coresight/coresight-etm-perf.c | 46 +++++++++++++++---------
1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 6beb662..afe7e7f 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -127,11 +127,9 @@ static void free_event_data(struct work_struct *work)
event_data = container_of(work, struct etm_event_data, work);
mask = &event_data->mask;
- /*
- * First deal with the sink configuration. See comment in
- * etm_setup_aux() about why we take the first available path.
- */
- if (event_data->snk_config) {
+ /* Free the sink buffers, if there are any */
+ if (event_data->snk_config && !WARN_ON(cpumask_empty(mask))) {

I'm not sure the WARN_ON() is required.

Here @mask can't be empty since you've added a check in setup_aux(). So if
@mask does end up being empty something has trampled the memory and we have
far bigger problems.

And thats exactly why I have a WARN_ON(). The issue is, we can't free the sink_config unless we are able to find the sink device. So, if we
ever end up with a sink_config with an empty mask, there is something
seriously wrong and hence the WARN_ON().

cpu = cpumask_first(mask);
sink = coresight_get_sink(etm_event_cpu_path(event_data, cpu));
if (sink_ops(sink)->free_buffer)
@@ -166,7 +164,7 @@ static void *alloc_event_data(int cpu)


- if (!sink_ops(sink)->alloc_buffer)
- goto err;
+ /* If we don't have any CPUs ready for tracing, abort */
cpu = cpumask_first(mask);
- /* Get the AUX specific data from the sink buffer */
+ if (cpu >= nr_cpu_ids)
+ goto err;
+ /* Allocate the sink buffer for this session */
event_data->snk_config =
sink_ops(sink)->alloc_buffer(sink, cpu, pages,
nr_pages, overwrite);