From: Leo Yan <leo.yan@linaro.org>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: suzuki.poulose@arm.com, mike.leach@arm.com, yabinc@google.com,
alexander.shishkin@linux.intel.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] coresight: tmc-etr: Decouple buffer sync and barrier packet insertion
Date: Tue, 27 Aug 2019 09:30:38 +0800 [thread overview]
Message-ID: <20190827013038.GA4596@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <20190826194605.3791-3-mathieu.poirier@linaro.org>
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
>
next prev parent reply other threads:[~2019-08-27 1:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-26 19:46 [PATCH v2 0/3] coresight: Add barrier packet when moving offset forward Mathieu Poirier
2019-08-26 19:46 ` [PATCH v2 1/3] coresight: tmc: Make memory width mask computation into a function Mathieu Poirier
2019-08-26 19:46 ` [PATCH v2 2/3] coresight: tmc-etr: Decouple buffer sync and barrier packet insertion Mathieu Poirier
2019-08-27 1:30 ` Leo Yan [this message]
2019-08-26 19:46 ` [PATCH v2 3/3] coresight: tmc-etr: Add barrier packets when moving offset forward Mathieu Poirier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190827013038.GA4596@leoy-ThinkPad-X240s \
--to=leo.yan@linaro.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@arm.com \
--cc=suzuki.poulose@arm.com \
--cc=yabinc@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).