diff mbox series

coresight: tmc-etr: Remove perf_data check.

Message ID 20190808193122.76679-1-yabinc@google.com (mailing list archive)
State New, archived
Headers show
Series coresight: tmc-etr: Remove perf_data check. | expand

Commit Message

Yabin Cui Aug. 8, 2019, 7:31 p.m. UTC
When tracing etm data of multiple threads on multiple cpus through
perf interface, each cpu has a unique etr_perf_buffer while sharing
the same etr device. There is no guarantee that the last cpu starts
etm tracing also stops last. So the perf_data check is no longer valid.

Signed-off-by: Yabin Cui <yabinc@google.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 ---------
 drivers/hwtracing/coresight/coresight-tmc.h     | 2 --
 2 files changed, 11 deletions(-)

Comments

Suzuki K Poulose Aug. 9, 2019, 9:22 a.m. UTC | #1
Hi Yabin,


Thank you for the analysis and the patch.

On 08/08/2019 20:31, Yabin Cui wrote:
> When tracing etm data of multiple threads on multiple cpus through
> perf interface, each cpu has a unique etr_perf_buffer while sharing
> the same etr device. There is no guarantee that the last cpu starts
> etm tracing also stops last. So the perf_data check is no longer valid.
> 
> Signed-off-by: Yabin Cui <yabinc@google.com>
> ---
>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 ---------
>   drivers/hwtracing/coresight/coresight-tmc.h     | 2 --
>   2 files changed, 11 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 17006705287a..0418440e0141 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1484,20 +1484,12 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>   		goto out;
>   	}
>   
> -	if (WARN_ON(drvdata->perf_data != etr_perf)) {
> -		lost = true;
> -		spin_unlock_irqrestore(&drvdata->spinlock, flags);
> -		goto out;
> -	}
> -

I think some sort of sanity check is a good idea to make sure we don't loose the
context. Even when different CPUs have different etr_perf buffer, the underlying
etr_buf should be the same. So, we should be able to simply convert the check
to, something like :

	struct etr_perf_buffer *perf_buf = drvdata->perf_data;

	...

	if (WARN_ON(perf_buf->etr_buf != etr_perf->buf)) {
		....
	}

Does that solve the problem for you ?

Suzuki
Yabin Cui Aug. 9, 2019, 8:16 p.m. UTC | #2
I totally agree that checking etr_buf is a better way.
It solves my problem.
I will upload a new patch soon.
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 17006705287a..0418440e0141 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1484,20 +1484,12 @@  tmc_update_etr_buffer(struct coresight_device *csdev,
 		goto out;
 	}
 
-	if (WARN_ON(drvdata->perf_data != etr_perf)) {
-		lost = true;
-		spin_unlock_irqrestore(&drvdata->spinlock, flags);
-		goto out;
-	}
-
 	CS_UNLOCK(drvdata->base);
 
 	tmc_flush_and_stop(drvdata);
 	tmc_sync_etr_buf(drvdata);
 
 	CS_LOCK(drvdata->base);
-	/* Reset perf specific data */
-	drvdata->perf_data = NULL;
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	size = etr_buf->len;
@@ -1556,7 +1548,6 @@  static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
 	}
 
 	etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf);
-	drvdata->perf_data = etr_perf;
 
 	/*
 	 * No HW configuration is needed if the sink is already in
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 1ed50411cc3c..3881a9ee565a 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -178,7 +178,6 @@  struct etr_buf {
  *		device configuration register (DEVID)
  * @idr:	Holds etr_bufs allocated for this ETR.
  * @idr_mutex:	Access serialisation for idr.
- * @perf_data:	PERF buffer for ETR.
  * @sysfs_data:	SYSFS buffer for ETR.
  */
 struct tmc_drvdata {
@@ -202,7 +201,6 @@  struct tmc_drvdata {
 	struct idr		idr;
 	struct mutex		idr_mutex;
 	struct etr_buf		*sysfs_buf;
-	void			*perf_data;
 };
 
 struct etr_buf_operations {