Re: [PATCH v2 12/16] coresight: tmc-etr: Introduce the notion of process ID to ETR devices

From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: mathieu.poirier@linaro.org, linux-arm-kernel@lists.infradead.org
Cc: alexander.shishkin@linux.intel.com, peterz@infradead.org,
	coresight@lists.linaro.org, Mike.Leach@arm.com,
	leo.yan@linaro.org
Subject: Re: [PATCH v2 12/16] coresight: tmc-etr: Introduce the notion of process ID to ETR devices
Date: Tue, 26 Mar 2019 16:46:06 +0000	[thread overview]
Message-ID: <08b9ad38-403a-9e0a-7ad3-5e9a4b4f6bf9@arm.com> (raw)
In-Reply-To: <20190325215632.17013-13-mathieu.poirier@linaro.org>

Mathieu,

On 03/25/2019 09:56 PM, Mathieu Poirier wrote:
> In preparation to support CPU-wide trace scenarios, add the notion of
> process ID to ETR devices so that memory buffers can be shared between
> events.

This patch looks fine to me. However it seems to do more than what the 
commit describes. e.g, refcounting. It may be worth fixing the
description.

Either way:

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   .../hwtracing/coresight/coresight-tmc-etr.c   | 22 +++++++++++++++++--
>   drivers/hwtracing/coresight/coresight-tmc.h   |  3 +++
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 79fee9341446..7254fafdf1c2 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -26,6 +26,7 @@ struct etr_flat_buf {
>   /*
>    * etr_perf_buffer - Perf buffer used for ETR
>    * @etr_buf		- Actual buffer used by the ETR
> + * @pid			- The PID this etr_perf_buffer belongs to.
>    * @snaphost		- Perf session mode
>    * @head		- handle->head at the beginning of the session.
>    * @nr_pages		- Number of pages in the ring buffer.
> @@ -33,6 +34,7 @@ struct etr_flat_buf {
>    */
>   struct etr_perf_buffer {
>   	struct etr_buf		*etr_buf;
> +	pid_t			pid;
>   	bool			snapshot;
>   	unsigned long		head;
>   	int			nr_pages;
> @@ -1160,8 +1162,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>   }
>   
>   static struct etr_buf *
> -tmc_etr_get_etr_buf(struct tmc_drvdata *drvdata, int node,
> -		    int nr_pages, void **pages)
> +alloc_etr_buf(struct tmc_drvdata *drvdata, int node,
> +	      int nr_pages, void **pages)
>   {
>   	struct etr_buf *etr_buf;
>   	unsigned long size;
> @@ -1195,6 +1197,20 @@ tmc_etr_get_etr_buf(struct tmc_drvdata *drvdata, int node,
>   	return etr_buf;
>   }
>   
> +static struct etr_buf *
> +tmc_etr_get_etr_buf(struct tmc_drvdata *drvdata, int node,
> +		    int nr_pages, void **pages)
> +{
> +	struct etr_buf *etr_buf;
> +
> +	etr_buf = alloc_etr_buf(drvdata, node, nr_pages, pages);
> +	if (IS_ERR(etr_buf))
> +		return etr_buf;
> +
> +	refcount_set(&etr_buf->refcount, 1);
> +	return etr_buf;
> +}
> +
>   /*
>    * tmc_etr_setup_perf_buf: Allocate ETR buffer for use by perf.
>    * The size of the hardware buffer is dependent on the size configured
> @@ -1231,6 +1247,7 @@ static void *tmc_alloc_etr_buffer(struct coresight_device *csdev,
>   				  int nr_pages, bool snapshot)
>   {
>   	int cpu = event->cpu;
> +	pid_t pid = task_pid_nr(event->owner);
>   	struct etr_perf_buffer *etr_perf;
>   	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>   
> @@ -1244,6 +1261,7 @@ static void *tmc_alloc_etr_buffer(struct coresight_device *csdev,
>   		return NULL;
>   	}
>   
> +	etr_perf->pid = pid;
>   	etr_perf->snapshot = snapshot;
>   	etr_perf->nr_pages = nr_pages;
>   	etr_perf->pages = pages;
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 487c53701e9c..7675138ff9fc 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -9,6 +9,7 @@
>   
>   #include <linux/dma-mapping.h>
>   #include <linux/miscdevice.h>
> +#include <linux/refcount.h>
>   
>   #define TMC_RSZ			0x004
>   #define TMC_STS			0x00c
> @@ -133,6 +134,7 @@ struct etr_buf_operations;
>   
>   /**
>    * struct etr_buf - Details of the buffer used by ETR
> + * @refcount	: Number of sources currently using this etr_buf.
>    * @mode	: Mode of the ETR buffer, contiguous, Scatter Gather etc.
>    * @full	: Trace data overflow
>    * @size	: Size of the buffer.
> @@ -143,6 +145,7 @@ struct etr_buf_operations;
>    * @private	: Backend specific information for the buf
>    */
>   struct etr_buf {
> +	refcount_t			refcount;
>   	enum etr_mode			mode;
>   	bool				full;
>   	ssize_t				size;
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel