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.poir...@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....@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
>
