diff mbox series

[PATCHv2,2/4] coresight: tmc-etr: Track perf handler.

Message ID 20210713121532.836244-3-daniel.kiss@arm.com (mailing list archive)
State New, archived
Headers show
Series coresight: Add ETR-PERF polling. | expand

Commit Message

Daniel Kiss July 13, 2021, 12:15 p.m. UTC 
Keep track of the perf handler that is registred by the first tracer.
This will be used by the update call from polling.

Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
Signed-off-by: Branislav Rankov <Branislav.Rankov@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++--
 drivers/hwtracing/coresight/coresight-tmc.h     | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Mathieu Poirier Aug. 25, 2021, 5:02 p.m. UTC | #1 
Hi Daniel,

On Tue, Jul 13, 2021 at 02:15:30PM +0200, Daniel Kiss wrote:
> Keep track of the perf handler that is registred by the first tracer.
> This will be used by the update call from polling.
> 
> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> Signed-off-by: Branislav Rankov <Branislav.Rankov@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++--
>  drivers/hwtracing/coresight/coresight-tmc.h     | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 589bb2d56e802..55c9b5fd9f832 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1503,8 +1503,8 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>  
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
>  
> -	/* Don't do anything if another tracer is using this sink */
> -	if (atomic_read(csdev->refcnt) != 1) {
> +	/* Serve only the tracer with the leading perf handler */
> +	if (drvdata->perf_handle != handle) {

In CPU wide trace scenarios the first CPU to enable a sink is not
guaranteed to be the same as the last CPU to use it.  As far as I understand the
above assumes the first and last CPUs to use a sink are the same.

>  		spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  		goto out;
>  	}
> @@ -1619,6 +1619,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
>  		drvdata->pid = pid;
>  		drvdata->mode = CS_MODE_PERF;
>  		drvdata->perf_buf = etr_perf->etr_buf;
> +		drvdata->perf_handle = handle;
>  		atomic_inc(csdev->refcnt);
>  	}
>  
> @@ -1666,6 +1667,7 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
>  	drvdata->mode = CS_MODE_DISABLED;
>  	/* Reset perf specific data */
>  	drvdata->perf_buf = NULL;
> +	drvdata->perf_handle = NULL;
>  
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index b91ec7dde7bc9..81583ffb973dc 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -184,6 +184,7 @@ struct etr_buf {
>   * @idr_mutex:	Access serialisation for idr.
>   * @sysfs_buf:	SYSFS buffer for ETR.
>   * @perf_buf:	PERF buffer for ETR.
> + * @perf_handle: PERF handle for ETR.
>   */
>  struct tmc_drvdata {
>  	void __iomem		*base;
> @@ -207,6 +208,7 @@ struct tmc_drvdata {
>  	struct mutex		idr_mutex;
>  	struct etr_buf		*sysfs_buf;
>  	struct etr_buf		*perf_buf;
> +	struct perf_output_handle *perf_handle;
>  };
>  
>  struct etr_buf_operations {
> -- 
> 2.25.1
>
Mathieu Poirier Aug. 25, 2021, 7:09 p.m. UTC | #2 
On Wed, Aug 25, 2021 at 11:02:13AM -0600, Mathieu Poirier wrote:
> Hi Daniel,
> 
> On Tue, Jul 13, 2021 at 02:15:30PM +0200, Daniel Kiss wrote:
> > Keep track of the perf handler that is registred by the first tracer.
> > This will be used by the update call from polling.
> > 
> > Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
> > Signed-off-by: Branislav Rankov <Branislav.Rankov@arm.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++--
> >  drivers/hwtracing/coresight/coresight-tmc.h     | 2 ++
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > index 589bb2d56e802..55c9b5fd9f832 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > @@ -1503,8 +1503,8 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
> >  
> >  	spin_lock_irqsave(&drvdata->spinlock, flags);
> >  
> > -	/* Don't do anything if another tracer is using this sink */
> > -	if (atomic_read(csdev->refcnt) != 1) {
> > +	/* Serve only the tracer with the leading perf handler */
> > +	if (drvdata->perf_handle != handle) {
> 
> In CPU wide trace scenarios the first CPU to enable a sink is not
> guaranteed to be the same as the last CPU to use it.  As far as I understand the
> above assumes the first and last CPUs to use a sink are the same.

I want to take a little more time to ponder about the rest of this patchset.
More comments to come tomorrow.

Thanks,
Mathieu

> 
> >  		spin_unlock_irqrestore(&drvdata->spinlock, flags);
> >  		goto out;
> >  	}
> > @@ -1619,6 +1619,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
> >  		drvdata->pid = pid;
> >  		drvdata->mode = CS_MODE_PERF;
> >  		drvdata->perf_buf = etr_perf->etr_buf;
> > +		drvdata->perf_handle = handle;
> >  		atomic_inc(csdev->refcnt);
> >  	}
> >  
> > @@ -1666,6 +1667,7 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
> >  	drvdata->mode = CS_MODE_DISABLED;
> >  	/* Reset perf specific data */
> >  	drvdata->perf_buf = NULL;
> > +	drvdata->perf_handle = NULL;
> >  
> >  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
> >  
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> > index b91ec7dde7bc9..81583ffb973dc 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc.h
> > +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> > @@ -184,6 +184,7 @@ struct etr_buf {
> >   * @idr_mutex:	Access serialisation for idr.
> >   * @sysfs_buf:	SYSFS buffer for ETR.
> >   * @perf_buf:	PERF buffer for ETR.
> > + * @perf_handle: PERF handle for ETR.
> >   */
> >  struct tmc_drvdata {
> >  	void __iomem		*base;
> > @@ -207,6 +208,7 @@ struct tmc_drvdata {
> >  	struct mutex		idr_mutex;
> >  	struct etr_buf		*sysfs_buf;
> >  	struct etr_buf		*perf_buf;
> > +	struct perf_output_handle *perf_handle;
> >  };
> >  
> >  struct etr_buf_operations {
> > -- 
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 589bb2d56e802..55c9b5fd9f832 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1503,8 +1503,8 @@  tmc_update_etr_buffer(struct coresight_device *csdev,
 
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 
-	/* Don't do anything if another tracer is using this sink */
-	if (atomic_read(csdev->refcnt) != 1) {
+	/* Serve only the tracer with the leading perf handler */
+	if (drvdata->perf_handle != handle) {
 		spin_unlock_irqrestore(&drvdata->spinlock, flags);
 		goto out;
 	}
@@ -1619,6 +1619,7 @@  static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
 		drvdata->pid = pid;
 		drvdata->mode = CS_MODE_PERF;
 		drvdata->perf_buf = etr_perf->etr_buf;
+		drvdata->perf_handle = handle;
 		atomic_inc(csdev->refcnt);
 	}
 
@@ -1666,6 +1667,7 @@  static int tmc_disable_etr_sink(struct coresight_device *csdev)
 	drvdata->mode = CS_MODE_DISABLED;
 	/* Reset perf specific data */
 	drvdata->perf_buf = NULL;
+	drvdata->perf_handle = NULL;
 
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index b91ec7dde7bc9..81583ffb973dc 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -184,6 +184,7 @@  struct etr_buf {
  * @idr_mutex:	Access serialisation for idr.
  * @sysfs_buf:	SYSFS buffer for ETR.
  * @perf_buf:	PERF buffer for ETR.
+ * @perf_handle: PERF handle for ETR.
  */
 struct tmc_drvdata {
 	void __iomem		*base;
@@ -207,6 +208,7 @@  struct tmc_drvdata {
 	struct mutex		idr_mutex;
 	struct etr_buf		*sysfs_buf;
 	struct etr_buf		*perf_buf;
+	struct perf_output_handle *perf_handle;
 };
 
 struct etr_buf_operations {