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

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

On Tue, Mar 26, 2019 at 04:46:06PM +0000, Suzuki K Poulose wrote:
> 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.

I wasted a lot of time thinking about this... Theoretically it should be split
in two but the changes would be so trivial it would hardly be worth the trouble.
I'll rework 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