[RFC,15/20] coresight: etm-perf: implementing 'setup_aux()' API

Message ID 1442593594-10665-16-git-send-email-mathieu.poirier@linaro.org
State New
Headers show

Commit Message

Mathieu Poirier Sept. 18, 2015, 4:26 p.m.
Before trace can be collected the PMU needs to get a handle
on the mmpap'ed memory that was granted.  Since the collection
of traces can be done by sink buffers of various types,
representation of the memory layout is done at the sink level
rather than the tracer PMU driver.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Alexander Shishkin Sept. 30, 2015, 11:50 a.m. | #1
Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> +static void *etm_setup_aux(int cpu, void **pages,
> +			      int nr_pages, bool overwrite)
> +{
> +	struct coresight_device *csdev;
> +
> +	if (cpu == -1)
> +		cpu = smp_processor_id();
> +
> +	csdev = per_cpu(csdev_sink, cpu);
> +	if (!csdev)
> +		return NULL;
> +
> +	return sink_ops(csdev)->setup_aux(csdev, cpu, pages,
> +					  nr_pages, overwrite);

Is it guaranteed that this sink would always have .setup_aux()?

Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Mathieu Poirier Oct. 1, 2015, 10:49 p.m. | #2
On 30 September 2015 at 05:50, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> +static void *etm_setup_aux(int cpu, void **pages,
>> +                           int nr_pages, bool overwrite)
>> +{
>> +     struct coresight_device *csdev;
>> +
>> +     if (cpu == -1)
>> +             cpu = smp_processor_id();
>> +
>> +     csdev = per_cpu(csdev_sink, cpu);
>> +     if (!csdev)
>> +             return NULL;
>> +
>> +     return sink_ops(csdev)->setup_aux(csdev, cpu, pages,
>> +                                       nr_pages, overwrite);
>
> Is it guaranteed that this sink would always have .setup_aux()?

A setup_aux() is vital to the design, both on Intel and ARM.  I really
don't see how one could go without it.  I can return NULL if it hasn't
been provided - that way the setup will fail without triggering a core
dump.

>
> Regards,
> --
> Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Alexander Shishkin Oct. 2, 2015, 4:50 a.m. | #3
Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> On 30 September 2015 at 05:50, Alexander Shishkin
> <alexander.shishkin@linux.intel.com> wrote:
>> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>>
>>> +static void *etm_setup_aux(int cpu, void **pages,
>>> +                           int nr_pages, bool overwrite)
>>> +{
>>> +     struct coresight_device *csdev;
>>> +
>>> +     if (cpu == -1)
>>> +             cpu = smp_processor_id();
>>> +
>>> +     csdev = per_cpu(csdev_sink, cpu);
>>> +     if (!csdev)
>>> +             return NULL;
>>> +
>>> +     return sink_ops(csdev)->setup_aux(csdev, cpu, pages,
>>> +                                       nr_pages, overwrite);
>>
>> Is it guaranteed that this sink would always have .setup_aux()?
>
> A setup_aux() is vital to the design, both on Intel and ARM.  I really
> don't see how one could go without it.  I can return NULL if it hasn't
> been provided - that way the setup will fail without triggering a core
> dump.

It wasn't clear to me that the sink that ends up in csdev_sink will
always be the one that does have .setup_aux(). And if it indeed doesn't,
it's better to refuse to setup a buffer than crash.

Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index a21171a3e929..3aeb4215bb22 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -227,6 +227,27 @@  out:
 	return ret;
 }
 
+static void *etm_setup_aux(int cpu, void **pages,
+			      int nr_pages, bool overwrite)
+{
+	struct coresight_device *csdev;
+
+	if (cpu == -1)
+		cpu = smp_processor_id();
+
+	csdev = per_cpu(csdev_sink, cpu);
+	if (!csdev)
+		return NULL;
+
+	return sink_ops(csdev)->setup_aux(csdev, cpu, pages,
+					  nr_pages, overwrite);
+}
+
+static void etm_free_aux(void *data)
+{
+	kfree(data);
+}
+
 static int __init etm_perf_init(void)
 {
 	etm_pmu.capabilities	= PERF_PMU_CAP_EXCLUSIVE;
@@ -235,6 +256,8 @@  static int __init etm_perf_init(void)
 	etm_pmu.task_ctx_nr	= perf_sw_context;
 	etm_pmu.read		= etm_event_read;
 	etm_pmu.event_init	= etm_event_init;
+	etm_pmu.setup_aux	= etm_setup_aux;
+	etm_pmu.free_aux	= etm_free_aux;
 
 	return perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1);
 }