diff mbox series

[v2,2/3] coresight: tmc-etr: Decouple buffer sync and barrier packet insertion

Message ID 20190826194605.3791-3-mathieu.poirier@linaro.org (mailing list archive)
State Mainlined
Commit 3507d231a499e27c4bac8a47169b74ec7ef87292
Headers show
Series coresight: Add barrier packet when moving offset forward | expand

Commit Message

Mathieu Poirier Aug. 26, 2019, 7:46 p.m. UTC
If less space is available in the perf ring buffer than the ETR buffer,
barrier packets inserted in the trace stream by tmc_sync_etr_buf() are
skipped over when the head of the buffer is moved forward, resulting in
traces that can't be decoded.

This patch decouples the process of syncing ETR buffers and the addition
of barrier packets in order to perform the latter once the offset in the
trace buffer has been properly computed.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 .../hwtracing/coresight/coresight-tmc-etr.c    | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Leo Yan Aug. 27, 2019, 1:30 a.m. UTC | #1
On Mon, Aug 26, 2019 at 01:46:04PM -0600, Mathieu Poirier wrote:
> If less space is available in the perf ring buffer than the ETR buffer,
> barrier packets inserted in the trace stream by tmc_sync_etr_buf() are
> skipped over when the head of the buffer is moved forward, resulting in
> traces that can't be decoded.
> 
> This patch decouples the process of syncing ETR buffers and the addition
> of barrier packets in order to perform the latter once the offset in the
> trace buffer has been properly computed.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  .../hwtracing/coresight/coresight-tmc-etr.c    | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 4f000a03152e..bae47272de98 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -946,10 +946,6 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
>  	WARN_ON(!etr_buf->ops || !etr_buf->ops->sync);
>  
>  	etr_buf->ops->sync(etr_buf, rrp, rwp);
> -
> -	/* Insert barrier packets at the beginning, if there was an overflow */
> -	if (etr_buf->full)
> -		tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
>  }
>  
>  static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
> @@ -1086,6 +1082,13 @@ static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata)
>  		drvdata->sysfs_buf = NULL;
>  	} else {
>  		tmc_sync_etr_buf(drvdata);
> +		/*
> +		 * Insert barrier packets at the beginning, if there was
> +		 * an overflow.
> +		 */
> +		if (etr_buf->full)
> +			tmc_etr_buf_insert_barrier_packet(etr_buf,
> +							  etr_buf->offset);
>  	}
>  }
>  
> @@ -1502,11 +1505,16 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>  	CS_LOCK(drvdata->base);
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
> +	lost = etr_buf->full;

Comparing to the previous version, it drops '|' bitwise operator;
seems to me this is more neat :)

I think Yabin's testing is more convinced, so I skip to test.
FWIW, these three patches look good to me:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

>  	size = etr_buf->len;
>  	if (!etr_perf->snapshot && size > handle->size) {
>  		size = handle->size;
>  		lost = true;
>  	}
> +
> +	/* Insert barrier packets at the beginning, if there was an overflow */
> +	if (lost)
> +		tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
>  	tmc_etr_sync_perf_buffer(etr_perf, size);
>  
>  	/*
> @@ -1517,8 +1525,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
>  	 */
>  	if (etr_perf->snapshot)
>  		handle->head += size;
> -
> -	lost |= etr_buf->full;
>  out:
>  	/*
>  	 * Don't set the TRUNCATED flag in snapshot mode because 1) the
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 4f000a03152e..bae47272de98 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -946,10 +946,6 @@  static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
 	WARN_ON(!etr_buf->ops || !etr_buf->ops->sync);
 
 	etr_buf->ops->sync(etr_buf, rrp, rwp);
-
-	/* Insert barrier packets at the beginning, if there was an overflow */
-	if (etr_buf->full)
-		tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
 }
 
 static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
@@ -1086,6 +1082,13 @@  static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata)
 		drvdata->sysfs_buf = NULL;
 	} else {
 		tmc_sync_etr_buf(drvdata);
+		/*
+		 * Insert barrier packets at the beginning, if there was
+		 * an overflow.
+		 */
+		if (etr_buf->full)
+			tmc_etr_buf_insert_barrier_packet(etr_buf,
+							  etr_buf->offset);
 	}
 }
 
@@ -1502,11 +1505,16 @@  tmc_update_etr_buffer(struct coresight_device *csdev,
 	CS_LOCK(drvdata->base);
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
+	lost = etr_buf->full;
 	size = etr_buf->len;
 	if (!etr_perf->snapshot && size > handle->size) {
 		size = handle->size;
 		lost = true;
 	}
+
+	/* Insert barrier packets at the beginning, if there was an overflow */
+	if (lost)
+		tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
 	tmc_etr_sync_perf_buffer(etr_perf, size);
 
 	/*
@@ -1517,8 +1525,6 @@  tmc_update_etr_buffer(struct coresight_device *csdev,
 	 */
 	if (etr_perf->snapshot)
 		handle->head += size;
-
-	lost |= etr_buf->full;
 out:
 	/*
 	 * Don't set the TRUNCATED flag in snapshot mode because 1) the