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 <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>
> ---
> 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.
> 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 (cpu != -1)
> cpumask_set_cpu(cpu, mask);
> else
> - cpumask_copy(mask, cpu_online_mask);
> + cpumask_copy(mask, cpu_present_mask);
>
> /*
> * Each CPU has a single path between source and destination. As such
> @@ -218,19 +216,32 @@ static void *etm_setup_aux(int event_cpu, void **pages,
> * on the cmd line. As such the "enable_sink" flag in sysFS is reset.
> */
> sink = coresight_get_enabled_sink(true);
> - if (!sink)
> + if (!sink || !sink_ops(sink)->alloc_buffer)
> goto err;
>
> mask = &event_data->mask;
>
> - /* Setup the path for each CPU in a trace session */
> + /*
> + * Setup the path for each CPU in a trace session. We try to build
> + * trace path for each CPU in the mask. If we don't find an ETM
> + * for the CPU or fail to build a path, we clear the CPU from the
> + * mask and continue with the rest. If ever we try to trace on those
> + * CPUs, we can handle it and fail the session.
> + */
> for_each_cpu(cpu, mask) {
> struct list_head *path;
> struct coresight_device *csdev;
>
> csdev = per_cpu(csdev_src, cpu);
> - if (!csdev)
> - goto err;
> + /*
> + * If there is no ETM associated with this CPU clear it from
> + * the mask and continue with the rest. If ever we try to trace
> + * on this CPU, we handle it accordingly.
> + */
> + if (!csdev) {
> + cpumask_clear_cpu(cpu, mask);
> + continue;
> + }
>
> /*
> * Building a path doesn't enable it, it simply builds a
> @@ -238,17 +249,20 @@ static void *etm_setup_aux(int event_cpu, void **pages,
> * referenced later when the path is actually needed.
> */
> path = coresight_build_path(csdev, sink);
> - if (IS_ERR(path))
> - goto err;
> + if (IS_ERR(path)) {
> + cpumask_clear_cpu(cpu, mask);
> + continue;
> + }
>
> *etm_event_cpu_path_ptr(event_data, cpu) = path;
> }
>
> - 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);
> --
> 2.7.4
>
