Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1250183 > unrolled thread

[PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API Mathieu Poirier <mathieu.poirier@linaro.org> - 2015-10-18 20:30 +0200
    Re: [PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-10-20 12:00 +0200
      Re: [PATCH V2 20/30] coresight: etb10: implementing buffer  set/reset() API Mathieu Poirier <mathieu.poirier@linaro.org> - 2015-10-20 19:40 +0200

#1250183 — [PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API

Implementing perf related APIs to activate and terminate
a trace session.  More specifically dealing with the sink
buffer's internal mechanic along with perf's API to start
and stop interactions with the ring buffers.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etb10.c | 58 +++++++++++++++++++++++++++
 include/linux/coresight.h                     |  9 +++++
 2 files changed, 67 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index 7f34e7af465e..ca2c4b42464d 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -28,6 +28,7 @@
 #include <linux/amba/bus.h>
 #include <linux/clk.h>
 #include <linux/mm.h>
+#include <linux/perf_event.h>
 
 #include <asm/local.h>
 
@@ -306,10 +307,67 @@ static void *etb_setup_aux(struct coresight_device *csdev, int cpu,
 	return buf;
 }
 
+static int etb_set_buffer(struct coresight_device *csdev,
+			  struct perf_output_handle *handle,
+			  void *sink_config)
+{
+	int ret = 0;
+	unsigned long head;
+	struct cs_buffers *buf = sink_config;
+	struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	/* This sink can only be used by a single session */
+	if (local_xchg(&drvdata->in_use, 1))
+		return -EBUSY;
+
+	/* how much space do we have in this session */
+	buf->size = handle->size;
+
+	/* wrap head around to the amount of space we have */
+	head = handle->head & ((buf->nr_pages << PAGE_SHIFT) - 1);
+
+	/* find the page to write to */
+	buf->cur = head / PAGE_SIZE;
+
+	/* and offset within that page */
+	buf->offset = head % PAGE_SIZE;
+
+	local_set(&buf->head, head);
+	local_set(&buf->data_size, 0);
+
+	return ret;
+}
+
+static void etb_reset_buffer(struct coresight_device *csdev,
+			     struct perf_output_handle *handle,
+			     void *sink_config)
+{
+	struct cs_buffers *buf = sink_config;
+	struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	if (buf) {
+		/*
+		 * In snapshot mode ->data_size holds the new address of the
+		 * ring buffer's head.  The size itself is the whole address
+		 * range since we want the latest information.
+		 */
+		if (buf->snapshot)
+			handle->head = local_xchg(&buf->data_size,
+						  buf->nr_pages << PAGE_SHIFT);
+
+		perf_aux_output_end(handle, local_xchg(&buf->data_size, 0),
+				    local_xchg(&buf->lost, 0));
+	}
+
+	local_set(&drvdata->in_use, 0);
+}
+
 static const struct coresight_ops_sink etb_sink_ops = {
 	.enable		= etb_enable,
 	.disable	= etb_disable,
 	.setup_aux	= etb_setup_aux,
+	.set_buffer	= etb_set_buffer,
+	.reset_buffer	= etb_reset_buffer,
 };
 
 static const struct coresight_ops etb_cs_ops = {
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index b1c25eba83b4..78202d5ea58a 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -15,6 +15,7 @@
 
 #include <linux/device.h>
 #include <linux/sched.h>
+#include <linux/perf_event.h>
 
 /* Peripheral id registers (0xFD0-0xFEC) */
 #define CORESIGHT_PERIPHIDR4	0xfd0
@@ -188,12 +189,20 @@ struct coresight_device {
  * @enable:	enables the sink.
  * @disable:	disables the sink.
  * @setup_aux:	initialises perf's ring buffer for trace collection.
+ * @set_buffer:	initialises buffer mechanic before a trace session.
+ * @reset_buffer: finalises buffer mechanic after a trace session.
  */
 struct coresight_ops_sink {
 	int (*enable)(struct coresight_device *csdev);
 	void (*disable)(struct coresight_device *csdev);
 	void *(*setup_aux)(struct coresight_device *csdev, int cpu,
 			   void **pages, int nr_pages, bool overwrite);
+	int (*set_buffer)(struct coresight_device *csdev,
+			  struct perf_output_handle *handle,
+			  void *sink_config);
+	void (*reset_buffer)(struct coresight_device *csdev,
+			     struct perf_output_handle *handle,
+			     void *sink_config);
 };
 
 /**
-- 
1.9.1

--
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/

[toc] | [next] | [standalone]

#1251549

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> Implementing perf related APIs to activate and terminate
> a trace session.  More specifically dealing with the sink
> buffer's internal mechanic along with perf's API to start
> and stop interactions with the ring buffers.

A matter of preference, but I'd say that it would be easier to review
this part if you merged all the buffer related patches together.

> +static void etb_reset_buffer(struct coresight_device *csdev,
> +			     struct perf_output_handle *handle,
> +			     void *sink_config)
> +{
> +	struct cs_buffers *buf = sink_config;
> +	struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	if (buf) {
> +		/*
> +		 * In snapshot mode ->data_size holds the new address of the
> +		 * ring buffer's head.  The size itself is the whole address
> +		 * range since we want the latest information.
> +		 */
> +		if (buf->snapshot)
> +			handle->head = local_xchg(&buf->data_size,
> +						  buf->nr_pages << PAGE_SHIFT);

Does it make sense to do this in etb_update_buffer() instead?

> +		perf_aux_output_end(handle, local_xchg(&buf->data_size, 0),
> +				    local_xchg(&buf->lost, 0));

The corresponding perf_aux_output_begin() is done in etm_event_add(),
I'd suggest that you do this in etm_event_del(),
unconditionally. Otherwise you're risking ending up with a refcount leak
and all sorts of horror.

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/

[toc] | [prev] | [next] | [standalone]

#1251918 — Re: [PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API

On 20 October 2015 at 03:56, Alexander Shishkin
<alexander.shishkin@linux.intel.com> wrote:
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>
>> Implementing perf related APIs to activate and terminate
>> a trace session.  More specifically dealing with the sink
>> buffer's internal mechanic along with perf's API to start
>> and stop interactions with the ring buffers.
>
> A matter of preference, but I'd say that it would be easier to review
> this part if you merged all the buffer related patches together.
>
>> +static void etb_reset_buffer(struct coresight_device *csdev,
>> +                          struct perf_output_handle *handle,
>> +                          void *sink_config)
>> +{
>> +     struct cs_buffers *buf = sink_config;
>> +     struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +     if (buf) {
>> +             /*
>> +              * In snapshot mode ->data_size holds the new address of the
>> +              * ring buffer's head.  The size itself is the whole address
>> +              * range since we want the latest information.
>> +              */
>> +             if (buf->snapshot)
>> +                     handle->head = local_xchg(&buf->data_size,
>> +                                               buf->nr_pages << PAGE_SHIFT);
>
> Does it make sense to do this in etb_update_buffer() instead?

I toyed with that idea for a while.  I would make sense if the ETB was
generating an interrupt when it is full but since cross-triggers
aren't implemented yet I didn't want to introduce code I can't test.

>
>> +             perf_aux_output_end(handle, local_xchg(&buf->data_size, 0),
>> +                                 local_xchg(&buf->lost, 0));
>
> The corresponding perf_aux_output_begin() is done in etm_event_add(),
> I'd suggest that you do this in etm_event_del(),

I'm in total agreement.  Since perf_aux_output_begin() is called in
etm_event_add(), perf_aux_output_end() should really be called in
etm_event_del().  The problem is that "buf->data_size" and "buf->lost"
are specific to the sink buffer and shouldn't be made public outside
of it.  Let me think about this further.

> unconditionally. Otherwise you're risking ending up with a refcount leak
> and all sorts of horror.
>
> 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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web